From dac5bbf71aa99681a57c7d54751d69bcd862a3ba Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 12 May 2026 16:11:39 -0500 Subject: [PATCH 1/3] =?UTF-8?q?fix(vision):=20release=20pendingFrame=20on?= =?UTF-8?q?=20rejected=20submit=20=E2=80=94=20closes=20#2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit 2026-05-12 F-V2: FrameAttachController.confirmSend returned an ImageBlock but did not clear pendingFrame; the slot was released via the broadcaster's .turnEnd watcher calling onAssistantTurnComplete(). When the orchestrator returned .rejected (no turn allocated), no .turnEnd ever flowed and the JPEG bytes sat in actor memory past the D-15 byte-release window until the next requestAttach overwrote them. Adds releaseAfterRejectedSubmit() — a dedicated entry point routed through the SOLE discardFrame() emission site so the D-15 single-emission-site invariant (FrameAttachDiscardSiteGrepTests) still holds with exactly one `pendingFrame = nil` assignment. AppDelegate's handleChatSubmit / handleChatCancelAndSubmit call it whenever they consumed a pending frame and the orchestrator returned .rejected. Co-Authored-By: Claude Opus 4.7 --- App/AppDelegate.swift | 18 ++++++++++ .../Vision/FrameAttachController.swift | 13 ++++++++ .../FrameAttachControllerTests.swift | 33 +++++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/App/AppDelegate.swift b/App/AppDelegate.swift index 471690c..93af23f 100644 --- a/App/AppDelegate.swift +++ b/App/AppDelegate.swift @@ -2357,11 +2357,21 @@ final class AppDelegate: NSObject, NSApplicationDelegate { } await self.tryPhraseAttachIfMatch(text) let outcome: SubmitOutcome + let hadImage: Bool if let imageBlock = await self.consumePendingFrameIfAny(userText: text) { + hadImage = true outcome = await orch.submit(.withImages(.text, text: text, images: [imageBlock])) } else { + hadImage = false outcome = await orch.submit(.text(text)) } + // Audit 2026-05-12 F-V2: on `.rejected` no turn is allocated, so the + // broadcaster's `.turnEnd` watcher will never fire + // `onAssistantTurnComplete()`. Release the pending frame explicitly so + // the JPEG bytes don't sit in actor memory past the D-15 window. + if hadImage, case .rejected = outcome { + await self.frameAttachController?.releaseAfterRejectedSubmit() + } await self.appendUserTextIfRunning(outcome, text: text) await self.handleTextOutcome(outcome) } @@ -2382,11 +2392,19 @@ final class AppDelegate: NSObject, NSApplicationDelegate { } await self.tryPhraseAttachIfMatch(text) let outcome: SubmitOutcome + let hadImage: Bool if let imageBlock = await self.consumePendingFrameIfAny(userText: text) { + hadImage = true outcome = await orch.cancelAndSubmit(.withImages(.text, text: text, images: [imageBlock])) } else { + hadImage = false outcome = await orch.cancelAndSubmit(.text(text)) } + // Audit 2026-05-12 F-V2: see handleChatSubmit — same release-on-rejected + // path applies to the barge-in handler. + if hadImage, case .rejected = outcome { + await self.frameAttachController?.releaseAfterRejectedSubmit() + } await self.appendUserTextIfRunning(outcome, text: text) await self.handleTextOutcome(outcome) } diff --git a/packages/Vision/Sources/Vision/FrameAttachController.swift b/packages/Vision/Sources/Vision/FrameAttachController.swift index a7f2758..d06ec75 100644 --- a/packages/Vision/Sources/Vision/FrameAttachController.swift +++ b/packages/Vision/Sources/Vision/FrameAttachController.swift @@ -134,6 +134,19 @@ public actor FrameAttachController { discardFrame() } + /// Release the pending frame when `confirmSend` returned a block but the + /// orchestrator rejected the submit (no turn was allocated, so the + /// broadcaster's `.turnEnd` watcher will never fire `onAssistantTurnComplete`). + /// Without this hook the JPEG bytes would sit in actor memory until the + /// next `requestAttach` overwrites the slot — a privacy regression on the + /// D-15 byte-release window. Routes through the SOLE emission site + /// (`discardFrame`) so the single-emission-site invariant is preserved. + /// Idempotent — safe to call when no frame is pending. + public func releaseAfterRejectedSubmit() { + pendingTimeoutTask?.cancel() + discardFrame() + } + // MARK: - SOLE EMISSION SITE — D-15 /// D-15 single emission site. The ONLY place in this file where the diff --git a/packages/Vision/Tests/VisionTests/FrameAttachControllerTests.swift b/packages/Vision/Tests/VisionTests/FrameAttachControllerTests.swift index 903cdcf..0d88c9d 100644 --- a/packages/Vision/Tests/VisionTests/FrameAttachControllerTests.swift +++ b/packages/Vision/Tests/VisionTests/FrameAttachControllerTests.swift @@ -117,6 +117,39 @@ final class FrameAttachControllerTests: XCTestCase { let stillFalse = await controller.hasPendingFrame XCTAssertFalse(stillFalse) } + + // MARK: - Test 6: rejected-submit release path (#2) + + /// Audit 2026-05-12 F-V2 — when `confirmSend` produced an ImageBlock but + /// the orchestrator returned `.rejected`, the controller must release the + /// frame on demand. Without `releaseAfterRejectedSubmit()` the JPEG bytes + /// would sit in actor memory until the next `requestAttach` overwrites + /// them — privacy regression on the D-15 byte-release window. + func testReleaseAfterRejectedSubmitClearsPendingFrame() async throws { + let capture = FakeCapture(frame: fixtureFrame) + let sink = FakeReplaySink() + let controller = FrameAttachController( + captureSession: capture, + replaySink: sink, + config: VisionRouterConfig.default + ) + await controller.requestAttach(reason: .hudButton) + let block = await controller.confirmSend(userText: "what is this") + XCTAssertNotNil(block, "confirmSend should have produced an ImageBlock") + // After confirmSend the slot is still populated by design — the + // assistant-turn-complete edge would normally release it. Simulate a + // rejected submit (no .turnEnd will ever fire) and assert the + // dedicated release entry point clears the slot. + let stillPending = await controller.hasPendingFrame + XCTAssertTrue(stillPending, "confirmSend keeps the slot populated until release") + await controller.releaseAfterRejectedSubmit() + let pending = await controller.hasPendingFrame + XCTAssertFalse(pending, "releaseAfterRejectedSubmit must clear pendingFrame") + // Idempotent. + await controller.releaseAfterRejectedSubmit() + let stillFalse = await controller.hasPendingFrame + XCTAssertFalse(stillFalse) + } } // MARK: - Test fakes -- 2.54.0 From aab046d942bd82e5e13f7be0e63375de0b113c09 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 12 May 2026 16:21:47 -0500 Subject: [PATCH 2/3] =?UTF-8?q?fix(vision):=20honor=20T2=20availability=20?= =?UTF-8?q?instead=20of=20lying=20=E2=80=94=20closes=20#1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit 2026-05-12 F-V1: AgentOrchestrator.runTurn computed `t2AvailableForThisTurn = (decision.tier != .t3Cloud)` and passed `true` on every local-tier vision turn. Production wires `MissingT2Provider` (the real T2 sidecar is v1.1 work, GH issue #84) whose stream throws `VisionError.t2ProviderUnavailable`. Combined with the router's escalate-on-low-confidence heuristic, every short / "I'm not sure" T1 response escalated into MissingT2Provider and killed the turn with a provider error instead of staying on T1. The audit's recommended fix (option a / option b) is option (b) modified: combine the local-tier flag with an honest `hasRealT2Provider` flag plumbed through `AgentOrchestrator.init`. Defaults `false`, so test sites and the production wiring both stay on T1 unless a real T2 provider is opted in. The pre-existing escalation test in `AgentOrchestratorVisionDispatchTests` flips `hasRealT2Provider: true` to exercise the swap path against a real mock T2; a new regression test wires `MissingT2Provider` + the default `false` flag + a low-confidence T1 response and asserts the turn finishes cleanly (no .error event, exactly one .turnEnd) — the exact shape that broke in production. `AppDelegate.installAgent` passes `hasRealT2Provider: false` explicitly alongside the `MissingT2Provider` wiring in `installVision`. Flip the flag when a real T2 provider replaces MissingT2Provider (#84). Co-Authored-By: Claude Opus 4.7 --- App/AppDelegate.swift | 7 ++ .../AgentOrchestrator/AgentOrchestrator.swift | 29 ++++++-- ...AgentOrchestratorVisionDispatchTests.swift | 66 +++++++++++++++++-- 3 files changed, 93 insertions(+), 9 deletions(-) diff --git a/App/AppDelegate.swift b/App/AppDelegate.swift index 93af23f..7f680a3 100644 --- a/App/AppDelegate.swift +++ b/App/AppDelegate.swift @@ -1895,6 +1895,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate { availableTools: [], availableToolsResolver: toolCatalogResolver, visionRouter: self.visionRouter, + // Audit 2026-05-12 F-V1: `hasRealT2Provider: false` because + // `installVision` wires `MissingT2Provider` (the real T2 sidecar + // is v1.1 work, GH issue #84). Without this gate, every low- + // confidence local T1 response escalated into MissingT2Provider + // and died with `t2ProviderUnavailable`. Flip to `true` only when + // a real T2 LLMProvider replaces `MissingT2Provider` above. + hasRealT2Provider: false, presenceSnapshot: PresenceStateSnapshot.shared, sessionHistoryLookup: sessionHistoryLookup ) diff --git a/packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift b/packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift index f813524..9060612 100644 --- a/packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift +++ b/packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift @@ -91,6 +91,18 @@ public actor AgentOrchestrator { /// who care about that pre-route through their own dispatcher). private let visionRouter: VisionRouter? + /// Audit 2026-05-12 F-V1 — honest T2 availability flag. The orchestrator + /// previously derived `t2AvailableForThisTurn = (decision.tier != .t3Cloud)`, + /// passing `true` on every local-tier vision turn. In production T2 is + /// `MissingT2Provider` (no real T2 sidecar exists yet — that's v1.1, GH + /// issue #84). Combined with the router's escalate-on-low-confidence + /// heuristic, every short / "I'm not sure" T1 response escalated into + /// `MissingT2Provider.stream(...)` which throws `t2ProviderUnavailable`, + /// killing the turn. Until a real T2 provider is wired, production passes + /// `false`; tests that exercise T1→T2 escalation pass `true` and supply a + /// real mock T2 provider. + private let hasRealT2Provider: Bool + /// Plan 09-03 / D-13 + D-14 — read-only ambient presence snapshot. /// `runTurn` queries `currentEnrichment()` during system-prompt /// composition and appends a plain sentence (e.g., "User is at the @@ -168,6 +180,7 @@ public actor AgentOrchestrator { availableTools: [ToolSchema] = [], availableToolsResolver: (@Sendable () async -> [ToolSchema])? = nil, visionRouter: VisionRouter? = nil, + hasRealT2Provider: Bool = false, presenceSnapshot: PresenceStateSnapshot? = nil, sessionHistoryLookup: @escaping SessionHistoryLookup = AgentOrchestrator.emptySessionHistoryLookup, degradationSummary: String? = nil @@ -181,6 +194,7 @@ public actor AgentOrchestrator { self.availableToolsList = availableTools self.availableToolsResolver = availableToolsResolver self.visionRouter = visionRouter + self.hasRealT2Provider = hasRealT2Provider self.presenceSnapshot = presenceSnapshot self.sessionHistoryLookup = sessionHistoryLookup self.degradationSummary = degradationSummary @@ -314,11 +328,16 @@ public actor AgentOrchestrator { explicitCloudOptIn: cloudOptIn ) provider = await router.providerForTier(decision.tier) - // T2 is available iff we routed local — D-04 ships with - // t2Provider == t1Provider, but evaluatePostResponse correctly - // stays-on-T1 when t2Available == false, so passing the actual - // tier here keeps the contract honest. - t2AvailableForThisTurn = (decision.tier != .t3Cloud) + // Audit 2026-05-12 F-V1: T2 is available iff we routed local AND + // a real T2 provider is wired. Production passes + // `hasRealT2Provider: false` because AppDelegate wires + // `MissingT2Provider` (the real T2 sidecar is v1.1 work, GH + // issue #84). Without the AND-gate, every low-confidence local + // T1 response escalated into `MissingT2Provider.stream(...)` + // which throws `t2ProviderUnavailable`, killing the turn. The + // router's `evaluatePostResponse(..., t2Available: false)` + // correctly stays-on-T1 in that case. + t2AvailableForThisTurn = (decision.tier != .t3Cloud) && hasRealT2Provider imageBearingTurns.insert(turnId) } diff --git a/packages/AgentCore/Tests/AgentOrchestratorTests/AgentOrchestratorVisionDispatchTests.swift b/packages/AgentCore/Tests/AgentOrchestratorTests/AgentOrchestratorVisionDispatchTests.swift index 42e14ee..cf0a4d1 100644 --- a/packages/AgentCore/Tests/AgentOrchestratorTests/AgentOrchestratorVisionDispatchTests.swift +++ b/packages/AgentCore/Tests/AgentOrchestratorTests/AgentOrchestratorVisionDispatchTests.swift @@ -40,6 +40,7 @@ final class AgentOrchestratorVisionDispatchTests: XCTestCase { private func buildOrchestrator( provider mock: any LLMProvider, visionRouter: VisionRouter? = nil, + hasRealT2Provider: Bool = false, dispatcher: any ToolDispatcher = StubToolDispatcher() ) async throws -> (AgentOrchestrator, ReplayLog, SessionID) { let replay = try ReplayLog(databaseURL: tempHome.dbURL) @@ -52,7 +53,8 @@ final class AgentOrchestratorVisionDispatchTests: XCTestCase { sessionId: session, systemPrompt: "you are jarvis", availableTools: [], - visionRouter: visionRouter + visionRouter: visionRouter, + hasRealT2Provider: hasRealT2Provider ) return (orch, replay, session) } @@ -163,9 +165,14 @@ final class AgentOrchestratorVisionDispatchTests: XCTestCase { .messageStop, ])) let router = makeRouter(t1: t1Mock, t2: t2Mock) - // D-04 ships with t1Provider == t2Provider in production; this test - // uses distinct providers to verify the swap actually occurs. - let (orch, _, _) = try await buildOrchestrator(provider: t1Mock, visionRouter: router) + // Production today wires `MissingT2Provider` (no real T2 sidecar + // yet); this test passes `hasRealT2Provider: true` and a real mock + // T2 to exercise the escalation path. + let (orch, _, _) = try await buildOrchestrator( + provider: t1Mock, + visionRouter: router, + hasRealT2Provider: true + ) let input = TurnInput.withImages(.text, text: "what is this", images: [Self.fakeImage]) let outcome = await orch.submit(input) @@ -245,6 +252,57 @@ final class AgentOrchestratorVisionDispatchTests: XCTestCase { XCTAssertFalse(textHad, "text-only turn should NOT be tracked") } + /// Audit 2026-05-12 F-V1 regression — when only `MissingT2Provider` is + /// wired (production today), a low-confidence T1 response must NOT + /// escalate to T2. Without the `hasRealT2Provider: false` gate, the + /// orchestrator would swap to `MissingT2Provider`, whose stream throws + /// `t2ProviderUnavailable`, and the turn would die with an error event. + func testLowConfidenceT1StaysOnT1WhenT2ProviderIsMissing() async throws { + // T1 returns a low-confidence answer that would trigger escalation if + // T2 were available. + let t1Mock = MockLLMProvider(script: .init(events: [ + .messageStart(LLMMessageStart(messageId: "m_t1", model: "gemma4:31b", usagePrefix: nil)), + .textDelta("I'm not sure"), + .stopReason(.endTurn), + .messageStop, + ])) + // Production wiring: T2 is `MissingT2Provider`. If reached its stream + // throws `VisionError.t2ProviderUnavailable`. + let router = VisionRouter( + t1Provider: t1Mock, + t2Provider: MissingT2Provider(), + t3Provider: t1Mock + ) + // Default `hasRealT2Provider: false` mirrors AppDelegate.installVision. + let (orch, _, _) = try await buildOrchestrator(provider: t1Mock, visionRouter: router) + + let input = TurnInput.withImages(.text, text: "what is this", images: [Self.fakeImage]) + let outcome = await orch.submit(input) + guard case .ran(let turnId) = outcome else { return XCTFail("expected .ran") } + + let events = await collectUntilIdle(orch: orch) + + // T1 was called exactly once — escalation did NOT fire, so + // MissingT2Provider was never streamed. + let t1Calls = await t1Mock.getRecordedCalls() + XCTAssertEqual(t1Calls.count, 1, "T1 should be the only provider invoked when hasRealT2Provider is false") + + // Exactly one .turnEnd — no error event. + let turnEnds = events.compactMap { event -> TurnID? in + if case .turnEnd(let id, _) = event { return id } + return nil + } + XCTAssertEqual(turnEnds.count, 1, "exactly one turnEnd; the turn must not die with a provider error") + XCTAssertEqual(turnEnds.first, turnId) + + // No error event surfaced. + let errored = events.contains { event in + if case .error = event { return true } + return false + } + XCTAssertFalse(errored, "no error event should surface — MissingT2Provider must never be reached") + } + /// Test 6 — escalationAttempt is a distinct ReplayEvent case from /// streamTruncatedRetry (the AGENT-09 retry path). Pattern matching on /// each is mutually exclusive. -- 2.54.0 From 8a3881ffc2a0389867620964d46584a55e7f1982 Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 12 May 2026 16:38:48 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix(vision):=20observe=20TCC=20revoke=20+?= =?UTF-8?q?=20wire=20becameAuthorized=20re-grant=20=E2=80=94=20closes=20#3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit 2026-05-12 F-V3: two related defects in the mid-session TCC lifecycle. (1) `CameraCapture.startWatchers()` only observed `AVCaptureSessionRuntimeError`. macOS Sequoia / 26 Tahoe surface mid-session Camera TCC revoke and device-stolen-by-other-process events through `AVCaptureSessionWasInterruptedNotification`, not through RuntimeError. The watcher therefore sat waiting for a yield that never came, and the `cameraRevoked` HUD banner never fired. Fix: add an `AVCaptureSessionWasInterruptedNotification` observer to `startWatchers()` that yields `.midSessionRevoked` on the degradation stream. macOS quirk: `AVCaptureSessionInterruptionReasonKey` and the `InterruptionReason` enum are `API_UNAVAILABLE(macos)` per AVFoundation headers (iOS / macCatalyst / tvOS / visionOS only), so the observer cannot discriminate by reason — any macOS interruption means the session can't continue capturing, and the user should see the revoked banner. A new `simulateInterruption()` test seam posts a synthetic notification through the same code path to validate observer wiring. (2) `CameraCapture.becameAuthorized()` had zero non-doc callsites outside its own declaration (`grep -rn 'becameAuthorized' --include '*.swift'` confirmed). A user who denied Camera at first launch and later granted via Settings while the app was running never recovered — the capture session stayed dormant until app restart. Fix: AppDelegate now registers a `NSApplication.didBecomeActive` observer in `installVision()` that re-checks `AVCaptureDevice.authorizationStatus(for: .video)` on every foreground return. On a `non-authorized → .authorized` flip it calls `capture.becameAuthorized()` to reopen the session. State machinery is MainActor-isolated and cleaned up alongside the existing camera degradation task in `applicationWillTerminate`. Skipped under XCTest (`Self.isRunningAsTestHost`) so the host app doesn't double-register during the test bundle's app lifecycle. Tests: - `CameraCaptureMidSessionRevokeTests.testInterruptionYieldsMidSessionRevoked` fires a synthetic interruption and asserts the degradation stream yields `.midSessionRevoked`. - `CameraCaptureTCCTests.testBecameAuthorizedOpensSessionAfterRegrant` seeds `.denied` → asserts `open()` throws `tccDenied` → flips the probe to `.authorized` → asserts `becameAuthorized()` opens the session. Co-Authored-By: Claude Opus 4.7 --- App/AppDelegate.swift | 73 +++++++++++++++++++ .../Vision/Sources/Vision/CameraCapture.swift | 45 ++++++++++++ .../CameraCaptureMidSessionRevokeTests.swift | 70 ++++++++++++++++++ .../VisionTests/CameraCaptureTCCTests.swift | 49 +++++++++++++ 4 files changed, 237 insertions(+) create mode 100644 packages/Vision/Tests/VisionTests/CameraCaptureMidSessionRevokeTests.swift diff --git a/App/AppDelegate.swift b/App/AppDelegate.swift index 7f680a3..2add558 100644 --- a/App/AppDelegate.swift +++ b/App/AppDelegate.swift @@ -489,6 +489,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate { /// graceful denial). Drains `CameraCapture.degradationStream`. var cameraDegradationTask: Task? + /// Audit 2026-05-12 F-V3 — observer for `NSApplication.didBecomeActive`. + /// macOS does not publish an `AVCaptureDevice.authorizationStatus` + /// change notification, so we re-check the status whenever the app + /// returns to the foreground. If the status flipped from + /// non-authorized → `.authorized` (user granted via System Settings + /// while the app was running), we call `becameAuthorized()` on the + /// camera capture so presence + frame-attach come back online without a + /// restart. + var cameraAuthRecheckObserver: NSObjectProtocol? + /// Builds + starts the vision subsystem. Spawned BEFORE `agentInstallTask` /// because the orchestrator constructor takes `visionRouter` and the /// broadcaster's frame-attach release subscriber needs @@ -1202,6 +1212,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate { memoryInstallTask?.cancel() visionInstallTask?.cancel() cameraDegradationTask?.cancel() + if let obs = cameraAuthRecheckObserver { + NotificationCenter.default.removeObserver(obs) + cameraAuthRecheckObserver = nil + } if let monitor = presenceMonitor { Task { await monitor.cancel() } } @@ -2613,11 +2627,70 @@ final class AppDelegate: NSObject, NSApplicationDelegate { ) } + // 8. Audit 2026-05-12 F-V3 — TCC re-grant detection via foreground + // re-check. macOS does not surface an `AVCaptureDevice` auth + // status change notification, so we observe + // `NSApplication.didBecomeActive` and compare current status + // against the last-seen value. On a `non-authorized → authorized` + // flip we call `capture.becameAuthorized()` to bring the camera + // session back online. Skip under XCTest (no AppKit run loop). + if !Self.isRunningAsTestHost { + installCameraAuthRecheckObserver(capture: capture) + } + systemLogger?.info( "installVision: presence + VisionRouter + FrameAttachController wired (T2 sidecar deferred)" ) } + /// Audit 2026-05-12 F-V3 — last camera TCC status observed during + /// foreground re-checks. Updated only by + /// `handleCameraAuthRecheck(capture:)` which runs on the main actor; the + /// observer closure dispatches there so this property stays + /// MainActor-isolated. + private var lastCameraAuthStatus: AVAuthorizationStatus? + + /// Audit 2026-05-12 F-V3 — install the foreground TCC re-check observer. + /// macOS does not surface a camera authorization-change notification, so + /// we observe `NSApplication.didBecomeActive` and compare the current + /// status against the last-seen value. On a `non-authorized → authorized` + /// flip we call `capture.becameAuthorized()`. + @MainActor + private func installCameraAuthRecheckObserver(capture: CameraCapture) { + lastCameraAuthStatus = AVCaptureDevice.authorizationStatus(for: .video) + cameraAuthRecheckObserver = NotificationCenter.default.addObserver( + forName: NSApplication.didBecomeActiveNotification, + object: nil, + queue: .main + ) { [weak self] _ in + // The block lands on `.main`; dispatch to MainActor for property + // access. `weak self` keeps the AppDelegate retain cycle clean. + Task { @MainActor [weak self] in + await self?.handleCameraAuthRecheck(capture: capture) + } + } + } + + /// Foreground TCC re-check body. Invoked from + /// `installCameraAuthRecheckObserver`'s notification closure. + @MainActor + func handleCameraAuthRecheck(capture: CameraCapture) async { + let current = AVCaptureDevice.authorizationStatus(for: .video) + let previous = lastCameraAuthStatus + lastCameraAuthStatus = current + guard previous != .authorized, current == .authorized else { return } + systemLogger?.info( + "installVision: foreground re-check detected camera permission grant — calling becameAuthorized()" + ) + do { + try await capture.becameAuthorized() + } catch { + systemLogger?.warning( + "installVision: becameAuthorized() after re-grant threw \(error)" + ) + } + } + // MARK: - Test-host detection /// Detects whether this process is hosting an XCTest bundle. XCTest sets diff --git a/packages/Vision/Sources/Vision/CameraCapture.swift b/packages/Vision/Sources/Vision/CameraCapture.swift index ef45f2e..a10e154 100644 --- a/packages/Vision/Sources/Vision/CameraCapture.swift +++ b/packages/Vision/Sources/Vision/CameraCapture.swift @@ -29,6 +29,11 @@ public actor CameraCapture { private var videoOutput: AVCaptureVideoDataOutput? private var photoOutput: AVCapturePhotoOutput? private var runtimeErrorObserver: NSObjectProtocol? + /// Audit 2026-05-12 F-V3 — observer for `AVCaptureSessionWasInterrupted`. + /// macOS surfaces mid-session Camera TCC revoke via the interruption + /// notification, NOT `RuntimeError`. Without this observer the revoke + /// arrives silently and the `cameraRevoked` HUD banner never fires. + private var interruptionObserver: NSObjectProtocol? /// Test seam — defaults to AVCaptureDevice.authorizationStatus(for: .video). /// Tests inject a closure to simulate denied/notDetermined/authorized. @@ -100,6 +105,10 @@ public actor CameraCapture { NotificationCenter.default.removeObserver(obs) runtimeErrorObserver = nil } + if let obs = interruptionObserver { + NotificationCenter.default.removeObserver(obs) + interruptionObserver = nil + } session?.stopRunning() videoSampleDelegate?.finishAllStreams() videoSampleDelegate = nil @@ -237,6 +246,42 @@ public actor CameraCapture { localLogger.warning("CameraCapture mid-session error: \(err)") cont.yield(.midSessionRevoked) } + // Audit 2026-05-12 F-V3 — observe `AVCaptureSessionWasInterrupted`. + // macOS surfaces TCC revoke + device-stolen-by-other-client events + // through the interruption notification, NOT through RuntimeError. + // + // Important macOS quirk: `AVCaptureSessionInterruptionReasonKey` and + // the `AVCaptureSession.InterruptionReason` enum are + // **API_UNAVAILABLE(macos)** (iOS / macCatalyst / tvOS / visionOS + // only — see AVCaptureSession.h, macOS SDK 26). So we cannot + // discriminate by reason here. Any interruption on macOS means the + // session can't continue capturing — we surface the revoked banner + // unconditionally. The most common causes are user-revoke via System + // Settings and device-in-use-by-another-process; both warrant the + // banner. + interruptionObserver = NotificationCenter.default.addObserver( + forName: AVCaptureSession.wasInterruptedNotification, + object: session, + queue: nil + ) { _ in + let localLogger = Logger(label: label) + localLogger.warning("CameraCapture mid-session interrupted") + cont.yield(.midSessionRevoked) + } + } + + /// Audit 2026-05-12 F-V3 — test seam for the mid-session interruption + /// path. Posts an `AVCaptureSession.wasInterruptedNotification` against + /// the live session; the registered observer yields `.midSessionRevoked`. + /// Routes through the SAME code path as a real OS-fired notification, + /// so a passing test proves the production observer is wired correctly. + /// Throws when `open()` hasn't built a session yet. + internal func simulateInterruption() throws { + guard let session else { throw VisionError.sessionNotRunning } + NotificationCenter.default.post( + name: AVCaptureSession.wasInterruptedNotification, + object: session + ) } private static func liveAuthStatus() -> AVAuthorizationStatus { diff --git a/packages/Vision/Tests/VisionTests/CameraCaptureMidSessionRevokeTests.swift b/packages/Vision/Tests/VisionTests/CameraCaptureMidSessionRevokeTests.swift new file mode 100644 index 0000000..7c4e760 --- /dev/null +++ b/packages/Vision/Tests/VisionTests/CameraCaptureMidSessionRevokeTests.swift @@ -0,0 +1,70 @@ +import XCTest +import AVFoundation +@testable import JarvisVision + +/// Audit 2026-05-12 F-V3 — mid-session TCC revoke observation. +/// +/// Before the fix, `CameraCapture.startWatchers()` only observed +/// `AVCaptureSessionRuntimeError`. macOS Sequoia / 26 Tahoe surface +/// mid-session Camera TCC revoke via `AVCaptureSessionWasInterrupted`, +/// not `RuntimeError`. Without an interruption observer, a user revoking +/// camera access via System Settings while the app was running never +/// drove the `cameraRevoked` HUD banner — the producer signal never fired. +/// +/// **macOS quirk:** `AVCaptureSessionInterruptionReasonKey` and the +/// associated `InterruptionReason` enum are `API_UNAVAILABLE(macos)` per +/// AVFoundation headers (iOS / macCatalyst / tvOS / visionOS only). So +/// on macOS the observer cannot discriminate reasons; any interruption +/// means we can't capture, and the user should see the revoked banner. +final class CameraCaptureMidSessionRevokeTests: XCTestCase { + + /// Posting an `AVCaptureSessionWasInterrupted` against the live session + /// must drive the degradation stream to yield `.midSessionRevoked`. + /// Without the observer this notification would arrive silently. + func testInterruptionYieldsMidSessionRevoked() async throws { + let (stream, cont) = AsyncStream.makeStream() + let capture = CameraCapture(degradationContinuation: cont, degradationStream: stream) + await capture.setAuthStatusProbe { .authorized } + + do { + try await capture.open() + } catch VisionError.noCameraDevice { + try XCTSkipIf(true, "no camera device available — see Track-C 6 hardware test") + return + } catch { + try XCTSkipIf(true, "camera open failed (\(error)) — see Track-C 6 hardware test") + return + } + + try await capture.simulateInterruption() + + let reason = await Self.firstDeg(stream: stream, timeout: 1.0) + XCTAssertEqual(reason, .midSessionRevoked, + "AVCaptureSession.wasInterruptedNotification must yield .midSessionRevoked") + + await capture.shutdown() + } + + /// Race the next event off the stream against a sleep so the test does + /// not hang when no event will ever arrive. Mirrors the helper in + /// `CameraCaptureTCCTests`. + private static func firstDeg( + stream: AsyncStream, + timeout: Double + ) async -> VisionDegradationReason? { + let collector = Task { () -> VisionDegradationReason? in + for await reason in stream { + return reason + } + return nil + } + let timer = Task { () -> VisionDegradationReason? in + try? await Task.sleep(nanoseconds: UInt64(timeout * 1_000_000_000)) + collector.cancel() + return nil + } + let result = await collector.value + timer.cancel() + return result + } +} diff --git a/packages/Vision/Tests/VisionTests/CameraCaptureTCCTests.swift b/packages/Vision/Tests/VisionTests/CameraCaptureTCCTests.swift index 8db73d6..2bd4564 100644 --- a/packages/Vision/Tests/VisionTests/CameraCaptureTCCTests.swift +++ b/packages/Vision/Tests/VisionTests/CameraCaptureTCCTests.swift @@ -42,6 +42,55 @@ final class CameraCaptureTCCTests: XCTestCase { await capture.shutdown() } + /// Audit 2026-05-12 F-V3 — `becameAuthorized()` is the re-grant entry + /// point AppDelegate's foreground TCC re-check calls when the user + /// flips camera permission `.denied → .authorized` via System Settings. + /// Before the fix, this method had zero non-doc callsites in production + /// (grep returned nothing outside its own declaration), so a re-grant + /// after first-launch denial never reopened the session. The + /// AppDelegate-side wiring is exercised by host integration; this test + /// covers the Vision-side contract: after the auth probe flips to + /// .authorized, calling becameAuthorized() opens the session + /// successfully. + func testBecameAuthorizedOpensSessionAfterRegrant() async throws { + // Class-backed status box so the @Sendable probe can read the + // latest value (simulating the user flipping permission in + // System Settings between calls without needing an actor hop). + final class StatusBox: @unchecked Sendable { + private let lock = NSLock() + private var value: AVAuthorizationStatus + init(_ v: AVAuthorizationStatus) { self.value = v } + func read() -> AVAuthorizationStatus { lock.lock(); defer { lock.unlock() }; return value } + func write(_ v: AVAuthorizationStatus) { lock.lock(); defer { lock.unlock() }; value = v } + } + let box = StatusBox(.denied) + + let (stream, cont) = AsyncStream.makeStream() + let capture = CameraCapture(degradationContinuation: cont, degradationStream: stream) + await capture.setAuthStatusProbe { box.read() } + + do { + try await capture.open() + XCTFail("Expected VisionError.tccDenied on first open") + } catch VisionError.tccDenied { + // Expected — first-launch denial. + } + + // Flip the probe — user granted via Settings. + box.write(.authorized) + do { + try await capture.becameAuthorized() + } catch VisionError.noCameraDevice { + try XCTSkipIf(true, "no camera device available — see Track-C 6 hardware test") + return + } catch { + XCTFail("becameAuthorized() threw after re-grant: \(error)") + } + + await capture.shutdown() + _ = stream + } + func testCaptureFrameThrowsWhenSessionNotRunning() async throws { let (stream, cont) = AsyncStream.makeStream() let capture = CameraCapture(degradationContinuation: cont, degradationStream: stream) -- 2.54.0