diff --git a/README.md b/README.md index 23bbe13..ba1a0c2 100644 --- a/README.md +++ b/README.md @@ -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). diff --git a/RouterOSAssistant/Core/Services/ConnectionService.swift b/RouterOSAssistant/Core/Services/ConnectionService.swift index 21449c0..823cf23 100644 --- a/RouterOSAssistant/Core/Services/ConnectionService.swift +++ b/RouterOSAssistant/Core/Services/ConnectionService.swift @@ -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 diff --git a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift index 5eb54e1..2a52d23 100644 --- a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift +++ b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectView.swift @@ -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 { Binding( get: { certificateFingerprint != nil }, - set: { _ in } + set: { isPresented in if !isPresented { viewModel.cancelCertificatePrompt() } } ) } @@ -203,7 +205,7 @@ struct ConnectView: View { private var sshHostKeyAlertBinding: Binding { Binding( get: { sshHostKeyFingerprint != nil }, - set: { _ in } + set: { isPresented in if !isPresented { viewModel.dismissSSHHostKeyPrompt() } } ) } diff --git a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift index f0a5c55..25ee2a4 100644 --- a/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift +++ b/RouterOSAssistant/Features/Wizard/Steps/Connect/ConnectViewModel.swift @@ -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) { diff --git a/RouterOSAssistant/Features/Wizard/Steps/Setup/ReviewApplyView.swift b/RouterOSAssistant/Features/Wizard/Steps/Setup/ReviewApplyView.swift index 9952edd..ada5b7b 100644 --- a/RouterOSAssistant/Features/Wizard/Steps/Setup/ReviewApplyView.swift +++ b/RouterOSAssistant/Features/Wizard/Steps/Setup/ReviewApplyView.swift @@ -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 ?? "") } diff --git a/RouterOSAssistant/Features/Wizard/Steps/Setup/SetupViewModel.swift b/RouterOSAssistant/Features/Wizard/Steps/Setup/SetupViewModel.swift index b9d5c58..7c0b552 100644 --- a/RouterOSAssistant/Features/Wizard/Steps/Setup/SetupViewModel.swift +++ b/RouterOSAssistant/Features/Wizard/Steps/Setup/SetupViewModel.swift @@ -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 diff --git a/RouterOSAssistantTests/ConnectionServiceTests.swift b/RouterOSAssistantTests/ConnectionServiceTests.swift index b339954..1f712d3 100644 --- a/RouterOSAssistantTests/ConnectionServiceTests.swift +++ b/RouterOSAssistantTests/ConnectionServiceTests.swift @@ -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() diff --git a/bugs.md b/bugs.md index 29091da..08f1d2a 100644 --- a/bugs.md +++ b/bugs.md @@ -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. diff --git a/found.md b/found.md index dc567c4..934fdf5 100644 --- a/found.md +++ b/found.md @@ -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.