Fix 3 nicht funktionierende Alert-Dismiss-Buttons (M34)
README-Milestone-Nachcheck (Port-Konflikt-Prüfung/"Fertig"-Button) deckte drei eigenständige Bugs derselben Klasse auf: Cancel/OK-Buttons bei ReviewApplyViews Apply-Fehler-Alert, ConnectViews Zertifikat-Alert und ConnectViews SSH-Hostkey-Alert taten nichts oder zu wenig - der jeweilige Verbindungs-/Fehlerzustand blieb hängen, der Dialog konnte nicht sauber verlassen werden. Neue ConnectionService.cancelPendingTrustConfirmation() und SetupViewModel.dismissApplyError(), alle drei Alerts korrekt verdrahtet (Button-Action + Bindings-Setter fuer Tap-Outside/Esc). Totes dismissPendingSSHTrust() entfernt. 1 neuer Regressionstest, alle 102 Unit-Tests gruen. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Kay
@@ -170,13 +170,14 @@ nur die zugehörigen Passwörter liegen weiterhin im macOS-Schlüsselbund.
|
||||
| M25 | Einrichten-Wizard-Politur: WAN-Zurück-Button, prominente Aktionsbuttons, leere Platzhalter-Felder (LAN/VLAN) | ✅ live verifiziert |
|
||||
| M26 | Bekannte Router: Seriennummer hinterlegt, trennt zwei Geräte mit identischem Host+Benutzername | ✅ live verifiziert |
|
||||
| M27 | Passwort-Anzeige-Button, Schlüsselbund nach Seriennummer getrennt, `terse`-Fallback verallgemeinert | ✅ live verifiziert |
|
||||
| — | LAN-Port-Konflikt-Prüfung + "Fertig"-Button (Einrichten) | 🔶 gebaut, Live-Test offen |
|
||||
| — | LAN-Port-Konflikt-Prüfung + "Fertig"-Button (Einrichten) | 🔶 gebaut, Code-Review bestätigt (dabei 3 echte Alert-Dismiss-Bugs gefunden+gefixt, siehe M34), Live-Klicktest durch Nutzer weiterhin offen |
|
||||
| M28 | Übersicht-Tab: Fokus-Modus (Klick auf Knoten → Kette im schwebenden Popup, Rest abgedunkelt); Close-Button-Layout in allen vier Popups vereinheitlicht | ✅ live verifiziert |
|
||||
| M29 | Einstellungen-Fenster (⌘,): Sprache, Update-Auto-Check, Farbschema, Textgröße, Bedienelement-Größe, LAN-Scanner-Refreshraten | ✅ live verifiziert |
|
||||
| M30 | Experte-Tab: "Mode-Taste"-Menü (`/system routerboard mode-button`), inkl. SSH-Zwangsweg für REST-Deckungslücken | ✅ live verifiziert |
|
||||
| M31 | Handbuch in der App (⌘? -Buttons, Textanker, Übersicht/Wizard/45 Experte-Menüs), DE+EN vollständig übersetzt | ✅ live verifiziert |
|
||||
| M32 | Automatisches Wiederverbinden bei Verbindungsabbruch (Herzschlag+Retry-Schleife, Versuchszähler+Countdown im Verbinden-Tab) | ✅ live verifiziert |
|
||||
| M33 | Dreifacher Tester-/Sicherheits-Deep-Dive (9 Funde: RouterOS-CLI-Injection live exploitiert+gefixt, Parser-Datenverlust, Race Condition, TOFU-Härtung u.a.) | ✅ Build+101 Tests grün, Details in [`bugs.md`](bugs.md) |
|
||||
| M34 | README-Nachcheck: 3 Alert-Dismiss-Bugs gefunden+gefixt ("Abbrechen"/OK-Buttons bei Zertifikat-/SSH-Hostkey-/Apply-Fehler-Dialogen ohne Wirkung), wizard_flow-Diagramm korrigiert | ✅ Build+102 Tests grün |
|
||||
|
||||
Ausführlicher Stand inkl. aller gefundenen Bugs, offener Punkte und
|
||||
Session-Verlauf: [`HANDOFF.md`](HANDOFF.md) / [`CHATLOG.md`](CHATLOG.md).
|
||||
|
||||
@@ -123,6 +123,27 @@ final class ConnectionService: ObservableObject {
|
||||
await connect(with: credentials)
|
||||
}
|
||||
|
||||
/// Backs out of a blocked connect attempt (`.needsCertificateConfirmation`/
|
||||
/// `.needsSSHHostKeyConfirmation`) without trusting anything — back to `.idle` so the
|
||||
/// Verbinden-tab form is usable again for a fresh attempt (e.g. different credentials, wrong
|
||||
/// host typo). Without this, `ConnectView`'s "Abbrechen" buttons on both trust alerts had no
|
||||
/// way to actually change `state`, so the alert's `isPresented` binding (derived from
|
||||
/// `state`/`pendingSSHTrustFingerprint`) stayed true and the dialog could never really be
|
||||
/// dismissed — found during the README milestone re-check, 2026-09-17 (same class of bug as
|
||||
/// `SetupViewModel.dismissApplyError()`, fixed moments earlier this session). Also clears
|
||||
/// `pendingSSHTrustFingerprint`, covering the background-trust-probe trigger path too, so one
|
||||
/// method correctly backs out of either of the two ways `ConnectView`'s SSH-host-key alert
|
||||
/// can be showing (see `pendingSSHTrustFingerprint`'s own doc comment for that distinction).
|
||||
func cancelPendingTrustConfirmation() {
|
||||
switch state {
|
||||
case .needsCertificateConfirmation, .needsSSHHostKeyConfirmation:
|
||||
state = .idle
|
||||
default:
|
||||
break
|
||||
}
|
||||
pendingSSHTrustFingerprint = nil
|
||||
}
|
||||
|
||||
func trustCurrentSSHHostKeyAndRetry(fingerprint: String) async {
|
||||
guard let credentials else { return }
|
||||
sshHostKeyTrust.trust(host: credentials.host, fingerprint: fingerprint)
|
||||
@@ -138,10 +159,6 @@ final class ConnectionService: ObservableObject {
|
||||
pendingSSHTrustFingerprint = nil
|
||||
}
|
||||
|
||||
func dismissPendingSSHTrust() {
|
||||
pendingSSHTrustFingerprint = nil
|
||||
}
|
||||
|
||||
/// Safety net for when the proactive `verifySSHTrust` check (above) didn't already cover a
|
||||
/// dedicated SSH service's host key — confirmed live: a session's first Expert-tab write (its
|
||||
/// pre-apply backup, over its own dedicated SSH connection) can still hit
|
||||
|
||||
@@ -134,7 +134,9 @@ struct ConnectView: View {
|
||||
Button(L10n.t("Vertrauen und verbinden", appLanguage)) {
|
||||
viewModel.trustAndRetry(fingerprint: fingerprint)
|
||||
}
|
||||
Button(L10n.t("Abbrechen", appLanguage), role: .cancel) {}
|
||||
Button(L10n.t("Abbrechen", appLanguage), role: .cancel) {
|
||||
viewModel.cancelCertificatePrompt()
|
||||
}
|
||||
} message: { fingerprint in
|
||||
Text(L10n.t("Der Router hat sich mit einem unbekannten Zertifikat gemeldet.", appLanguage)
|
||||
+ "\n" + L10n.t("Fingerabdruck:", appLanguage) + " \(fingerprint)\n\n"
|
||||
@@ -189,7 +191,7 @@ struct ConnectView: View {
|
||||
private var certificateAlertBinding: Binding<Bool> {
|
||||
Binding(
|
||||
get: { certificateFingerprint != nil },
|
||||
set: { _ in }
|
||||
set: { isPresented in if !isPresented { viewModel.cancelCertificatePrompt() } }
|
||||
)
|
||||
}
|
||||
|
||||
@@ -203,7 +205,7 @@ struct ConnectView: View {
|
||||
private var sshHostKeyAlertBinding: Binding<Bool> {
|
||||
Binding(
|
||||
get: { sshHostKeyFingerprint != nil },
|
||||
set: { _ in }
|
||||
set: { isPresented in if !isPresented { viewModel.dismissSSHHostKeyPrompt() } }
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -145,8 +145,21 @@ final class ConnectViewModel: ObservableObject {
|
||||
}
|
||||
}
|
||||
|
||||
/// Backs out of whichever of the two trust-confirmation states is currently showing the
|
||||
/// SSH-host-key alert (a blocked initial connect, or the background trust probe after a
|
||||
/// successful REST connect — see `ConnectionService.pendingSSHTrustFingerprint`'s doc
|
||||
/// comment). Previously only cleared `pendingSSHTrustFingerprint`, which left the alert stuck
|
||||
/// showing whenever it was triggered by the *other* path (`state ==
|
||||
/// .needsSSHHostKeyConfirmation`) — found during the README milestone re-check, 2026-09-17.
|
||||
func dismissSSHHostKeyPrompt() {
|
||||
connectionService.dismissPendingSSHTrust()
|
||||
connectionService.cancelPendingTrustConfirmation()
|
||||
}
|
||||
|
||||
/// Same "Abbrechen" fix as `dismissSSHHostKeyPrompt()`, for the unknown-TLS-certificate alert
|
||||
/// — its "Abbrechen" button previously had an empty action and could never actually clear
|
||||
/// `connectionService.state` back out of `.needsCertificateConfirmation`.
|
||||
func cancelCertificatePrompt() {
|
||||
connectionService.cancelPendingTrustConfirmation()
|
||||
}
|
||||
|
||||
func checkForUpdates(for credentials: RouterOSCredentials) {
|
||||
|
||||
@@ -79,10 +79,10 @@ struct ReviewApplyView: View {
|
||||
L10n.t("Anwenden fehlgeschlagen", appLanguage),
|
||||
isPresented: Binding(
|
||||
get: { viewModel.applyError != nil },
|
||||
set: { _ in }
|
||||
set: { isPresented in if !isPresented { viewModel.dismissApplyError() } }
|
||||
)
|
||||
) {
|
||||
Button(L10n.t("OK", appLanguage)) {}
|
||||
Button(L10n.t("OK", appLanguage)) { viewModel.dismissApplyError() }
|
||||
} message: {
|
||||
Text(viewModel.applyError ?? "")
|
||||
}
|
||||
|
||||
@@ -264,6 +264,17 @@ final class SetupViewModel: ObservableObject {
|
||||
setFirewallSectionEnabled(true)
|
||||
}
|
||||
|
||||
/// Dismisses the "Anwenden fehlgeschlagen" alert — `ReviewApplyView`'s `.alert(isPresented:)`
|
||||
/// binding derives its presented state from `applyError != nil`, so without this the alert's
|
||||
/// own OK button (whose action previously did nothing) could never actually clear the error:
|
||||
/// SwiftUI's next re-render would immediately re-evaluate the getter as still true and the
|
||||
/// alert would reappear or fail to dismiss. Found during the README milestone re-check,
|
||||
/// 2026-09-17 (bugs.md-style fix — see `SetupViewModel`'s port-conflict entry in bugs.md for
|
||||
/// the same-feature-area race-condition fix earlier this session).
|
||||
func dismissApplyError() {
|
||||
applyError = nil
|
||||
}
|
||||
|
||||
/// Ends the wizard after a successful apply — without this, the completed review screen just
|
||||
/// sits there with a disabled "Jetzt anwenden" and no way forward except "Zurück" (which would
|
||||
/// re-walk now-stale steps against the router state this apply just changed). Resets to a
|
||||
|
||||
@@ -70,6 +70,25 @@ final class ConnectionServiceTests: XCTestCase {
|
||||
XCTAssertEqual(service.state, .needsCertificateConfirmation(fingerprint: "AA:BB"))
|
||||
}
|
||||
|
||||
/// Regression test for the README milestone re-check, 2026-09-17: `ConnectView`'s
|
||||
/// "Abbrechen" button on the certificate-trust alert previously had an empty action and could
|
||||
/// never actually clear `state` back out of `.needsCertificateConfirmation`, so the alert
|
||||
/// (whose `isPresented` binding is derived from `state`) could never really be dismissed.
|
||||
func testCancelPendingTrustConfirmationResetsCertificateConfirmationToIdle() async {
|
||||
let service = ConnectionService()
|
||||
|
||||
await service.connect(
|
||||
with: credentials,
|
||||
makeRestTransport: { MockTransport(kind: .rest, connectError: RouterOSError.untrustedCertificate(fingerprint: "AA:BB")) },
|
||||
makeSSHTransport: { MockTransport(kind: .ssh) }
|
||||
)
|
||||
XCTAssertEqual(service.state, .needsCertificateConfirmation(fingerprint: "AA:BB"))
|
||||
|
||||
service.cancelPendingTrustConfirmation()
|
||||
|
||||
XCTAssertEqual(service.state, .idle)
|
||||
}
|
||||
|
||||
func testFailsWhenBothTransportsFail() async {
|
||||
let service = ConnectionService()
|
||||
|
||||
|
||||
@@ -245,3 +245,37 @@ Build + alle 101 Unit-Tests grün nach beiden Fixes.
|
||||
- Restliche Tabs/Bereiche (Übersicht-Diagramm-Interaktion, Sicherungen, Mode-Taste, Settings) wurden
|
||||
im Code überflogen, ohne konkreten neuen Fund über die bereits in `found.md`/`HANDOFF.md`
|
||||
dokumentierten Punkte hinaus.
|
||||
|
||||
---
|
||||
|
||||
## 2026-09-17 (Nachtrag 3: README-Milestone-Nachcheck)
|
||||
|
||||
Nutzer wies auf den noch offenen Milestone "LAN-Port-Konflikt-Prüfung + 'Fertig'-Button" in
|
||||
README.md hin. Beim erneuten Code-Review dieses Bereichs (u.a. wegen des in Runde 1 bereits
|
||||
gefixten Race-Bugs #1 in genau dieser Feature) drei weitere, eigenständige Funde derselben
|
||||
Bugklasse ("Alert-Dismiss tut nichts") entdeckt und gefixt.
|
||||
|
||||
### 10. Drei nicht funktionierende Alert-"Abbrechen"/"OK"-Buttons
|
||||
**Status:** fixed (Build + 102 Unit-Tests grün, 1 neuer Regressionstest)
|
||||
**Confidence:** hoch (Logikfehler direkt im Code nachvollzogen, nicht live reproduziert)
|
||||
|
||||
1. `ReviewApplyView`s "Anwenden fehlgeschlagen"-Alert: OK-Button-Action leer, `isPresented`-
|
||||
Bindings-Setter ebenfalls No-Op — `applyError` wurde nie zurückgesetzt.
|
||||
2. `ConnectView`s "Unbekanntes Zertifikat"-Alert: "Abbrechen"-Button-Action komplett leer —
|
||||
`connectionService.state` blieb für immer auf `.needsCertificateConfirmation` hängen, kein Weg
|
||||
zurück außer dem Zertifikat zu vertrauen.
|
||||
3. `ConnectView`s "Unbekannter SSH-Schlüssel"-Alert: "Abbrechen" rief nur
|
||||
`dismissPendingSSHTrust()` (löschte nur `pendingSSHTrustFingerprint`) — funktionierte nur für
|
||||
einen von zwei möglichen Auslöse-Pfaden, beim anderen (`state == .needsSSHHostKeyConfirmation`)
|
||||
blieb der Dialog hängen.
|
||||
|
||||
**Fix:** neue `ConnectionService.cancelPendingTrustConfirmation()` (setzt `state` bei beiden
|
||||
"needs...Confirmation"-Fällen auf `.idle` zurück, löscht zusätzlich `pendingSSHTrustFingerprint`),
|
||||
neue `SetupViewModel.dismissApplyError()`. Alle drei Alerts verdrahtet (Button-Action UND
|
||||
Bindings-Setter, deckt auch Tap-Outside/Esc-Dismiss ab). Totes `dismissPendingSSHTrust()`
|
||||
entfernt.
|
||||
|
||||
**Weiterhin offen:** die Port-Konflikt-Prüfung selbst (Warndialoge, "Weiter"-Sperre, "Fertig"-
|
||||
Button) ist nach diesem zweiten Code-Review-Durchgang funktional plausibel, aber noch nie
|
||||
tatsächlich in der App-UI durchgeklickt worden — dafür bräuchte es einen Live-Test durch den
|
||||
Nutzer, da UI-Automatisierung hier nicht verfügbar ist.
|
||||
|
||||
@@ -295,3 +295,34 @@ live exploitiert — anders als #5/#6 im vorigen Durchgang):
|
||||
hartem Verbindungsabbruch statt einem Vertrauens-Dialog mit unverifizierbarer Kennung.
|
||||
|
||||
Build grün, alle 101 Unit-Tests grün. Details in `bugs.md`.
|
||||
|
||||
### 14. README-Nachcheck: offener Milestone (Port-Konflikt/"Fertig") + 3 Alert-Dismiss-Bugs
|
||||
**Status:** fixed (Build + 102 Unit-Tests grün, Live-Klicktest weiterhin offen)
|
||||
|
||||
Nutzer wies auf einen noch offenen Milestone in der README hin (LAN-Port-Konflikt-Prüfung +
|
||||
"Fertig"-Button, nie live durchgeklickt). Beim erneuten Code-Review dieses Bereichs (u.a. wegen
|
||||
des in dieser Session bereits gefixten Race-Bugs in genau dieser Feature) drei echte,
|
||||
eigenständige Bugs derselben Klasse gefunden:
|
||||
|
||||
1. `ReviewApplyView`s "Anwenden fehlgeschlagen"-Alert: OK-Button-Action war leer, die
|
||||
`isPresented`-Bindings-Setter-Closure ebenfalls ein No-Op — `applyError` wurde nie
|
||||
zurückgesetzt, der Alert konnte sich nach dem Schließen theoretisch sofort wieder öffnen.
|
||||
2. `ConnectView`s "Unbekanntes Zertifikat"-Alert: "Abbrechen"-Button-Action war komplett leer —
|
||||
`connectionService.state` blieb für immer auf `.needsCertificateConfirmation` hängen, es gab
|
||||
keinen Weg zurück außer dem Zertifikat zu vertrauen.
|
||||
3. `ConnectView`s "Unbekannter SSH-Schlüssel"-Alert: "Abbrechen" rief nur
|
||||
`dismissPendingSSHTrust()` (löschte nur `pendingSSHTrustFingerprint`) — funktionierte nur für
|
||||
einen von zwei möglichen Auslöse-Pfaden dieses Alerts, beim anderen (`state ==
|
||||
.needsSSHHostKeyConfirmation`) blieb der Dialog hängen.
|
||||
|
||||
Fix: neue `ConnectionService.cancelPendingTrustConfirmation()` (setzt `state` bei beiden
|
||||
"needs...Confirmation"-Fällen auf `.idle` zurück, löscht zusätzlich
|
||||
`pendingSSHTrustFingerprint`), neue `SetupViewModel.dismissApplyError()`. Alle drei Alerts
|
||||
verdrahtet — sowohl der jeweilige Cancel/OK-Button als auch die Bindings-Setter-Closure (deckt
|
||||
auch Tap-Outside/Esc-Dismiss ab). Totes `dismissPendingSSHTrust()` entfernt. Ein neuer
|
||||
Regressionstest (`testCancelPendingTrustConfirmationResetsCertificateConfirmationToIdle`).
|
||||
|
||||
Build grün, alle 102 Unit-Tests grün. Der ursprünglich gemeldete Milestone (Port-Konflikt-Prüfung
|
||||
selbst, inkl. Warndialoge, "Weiter"-Sperre, "Fertig"-Button) bleibt beim Status "Code-Review
|
||||
bestätigt" — ein echter Live-Klicktest durch den Nutzer in der App-UI steht weiterhin aus, da
|
||||
UI-Automatisierung in dieser Session nicht verfügbar ist.
|
||||
|
||||
Reference in New Issue
Block a user