docs(roadmap): user-outcome-focused milestone roadmap
All 142 open issues triaged into 6 GitHub milestones organized by user-facing capability rather than technical area: - v0.1 — It actually works (28 issues): voice, listening, seeing, facts, debugging dialogs, basic UI. Tactical patches for CRITICAL audit findings so user has working Jarvis NOW. - v0.2 — Observable + stable (23): MED/LOW audit punch-list + PTT/TCC. - v0.5 — API M-0..M-3 (31): Self/Settings/Diagnostics surfaces; B-08. - v0.8 — API M-4..M-7 (36): Memory/Vision/Voice/Turn; B-03/B-04/B-05/B-06. - v1.0 — Shipping (13): commenting, doc reconciliation. - v1.1 — Backlog (11): Orpheus, WhisperKit, Voice Log, presence. ROADMAP-2026-05-12.md supersedes the build-phase ROADMAP.md from 2026-04-22 (kept for history; describes shipped phases). CLAUDE.md gains a Roadmap section pointing to the new doc + milestones URL. Also lands the 5 audit-2026-05-12 reports referenced by the roadmap (bus-hud, mcp-agent, memory, vision, voice).
This commit is contained in:
7 files changed
+1486
No files matched your search
@@ -0,0 +1,189 @@
|
||||
# Jarvis Roadmap — 2026-05-12
|
||||
|
||||
**Supersedes** `.planning/ROADMAP.md` (2026-04-22), which describes the original 9-phase GSD build sequence. That sequence shipped — v0.12.0 is in tree. This document re-frames remaining work around **user-facing capability** rather than build phase.
|
||||
|
||||
## What we're building
|
||||
|
||||
A personal, always-on macOS AI assistant styled after the Iron Man "Jarvis" HUD. Native Swift/SwiftUI host with an embedded React + R3F web layer, an in-process agent loop talking to Claude Opus 4.7 (Ollama as first-class alternate), MCP tools for OS integration, and a fully-local voice pipeline (openWakeWord → SpeechAnalyzer → AVSpeechSynthesizer/Orpheus). Single user, single machine, no commercial shipping concerns.
|
||||
|
||||
The core value the user is buying is **ambient presence**: "it's always there, I can talk to it, it gets things done on my Mac." Voice that actually fires. Listening that produces transcripts. Seeing through the camera. Facts that persist and get recalled. Debugging dialogs that show real state. UI that doesn't have basic correctness bugs.
|
||||
|
||||
The architecture is settled and is not relitigated here. The story this roadmap tells is sequencing: **a working system first, a typed API second, polish third.**
|
||||
|
||||
## Where we are today
|
||||
|
||||
142 open issues. Five fresh audits (2026-05-12) covering Memory, MCP/Agent, Voice, Vision, and Bus/HUD — collectively producing ~36 findings ranging from `severity:critical` (system-breaking) to `severity:low` (cosmetic drift). Today's work landed the schema fix that unblocks memory writes, brought the Dev Overlay from wired-but-dead to fully populated, and surfaced a comprehensive picture of what is broken right now. The Round 4 strict-mode milestone is on tree; v0.12.0 audit-and-stabilize is the active workstream.
|
||||
|
||||
The Jarvis API Migration Plan (`.planning/architecture/JARVIS-API-MIGRATION-PLAN.md`) is locked: 8 sequenced phases (M-0 → M-7) that move the entire client-facing surface from ad-hoc `BusInbound`/`BusOutbound` cases to typed `JarvisAPI` surfaces (`Self`, `Settings`, `Diagnostics`, `Memory`, `Vision`, `Voice`, `Turn`). That migration dissolves the carry-forward bugs B-02 through B-08 as a natural consequence — but it is 4–5.5 calendar weeks of work, and the user does not have a working system to use while it is in flight.
|
||||
|
||||
This roadmap therefore splits work into two lanes: **v0.1/v0.2 = tactical fixes so today's Jarvis works end-to-end**, and **v0.5/v0.8 = the typed-API migration**. Some CRITICAL audit findings will be re-fixed during the migration; v0.1 applies the tactical patch anyway, because the user needs Jarvis working *now*.
|
||||
|
||||
## Milestones
|
||||
|
||||
### v0.1 — It actually works (basic capabilities) — 28 issues
|
||||
|
||||
**User outcome:** I open Jarvis. Wake word fires. I say "Hey Jarvis, what time is it?" The ring pulses. STT transcribes me. The agent thinks. The HUD animates through state. TTS speaks the answer. I open the camera button — a real JPEG frame attaches to the next turn. I tell Jarvis "I prefer dark mode." Later I ask "what did I say I prefer?" and the fact comes back. I toggle the Dev Overlay and see real agent state, real tool calls, real latency. The Status panel shows real subsystem health. The chat panel auto-scrolls as tokens stream. Nothing in the basic flow is silently dead.
|
||||
|
||||
**Scope:** Critical audit findings that break basic capabilities, plus the chat-scroll and startup-surface UX gaps.
|
||||
|
||||
**Issues (28):**
|
||||
- **Voice CRITICAL/HIGH (8):** #28 wake-word dies on audio-graph rebuild, #29 STT backend hard-coded, #30 .ttsStopped only on barge-in, #31 VoiceTTSAdapter.tierResolver dead, #32 OpenWakeWordSession buffers grow unbounded, #33 BootHealthProbe doesn't probe voice DAG, #34 SileroVAD hidden state persists, #36 AgentHudIntent stream dormant
|
||||
- **Memory CRITICAL/HIGH (5):** #12 facts_vec never written, #13 replay log poisoned by synthetic memory-trigger turn_ids, #14 search_memory missing from preamble, #15 source_turn_id stores FNV hash, #16 hybridSearchSQL passes raw user text to FTS5 MATCH
|
||||
- **MCP/Agent CRITICAL (2):** #20 ReplayingToolResultObserver.turnIDResolver permanently nil, #21 tool-result bytes double-capped at 8KB
|
||||
- **Bus/HUD CRITICAL (3):** #40 BridgeNavigationDelegate no didFail arm, #41 MenuBarIconController.transition dead, #42 DevOverlay hollow shell
|
||||
- **Vision (3):** #1 T2 misadvertised when MissingT2Provider is wired, #2 FrameAttachController pendingFrame leak, #3 mid-session camera TCC revoke not observed
|
||||
- **UX essentials (2):** #51 (B-06) chat panel auto-scroll, #89 Open Status panel + Dev Overlay on app start
|
||||
- **Env setup unblocking facts (3):** #56 D-5 build libsqlite3 with extensions, #57 D-6 bundle vec0.dylib (likely already done via static link), #58 D-7 reconcile Ollama model baseline
|
||||
- **In-flight today (1):** #65 P0 AppDelegate commenting style rewrite
|
||||
- **Loose-end triage (1):** #63 stash@{0} resurface-or-drop
|
||||
|
||||
**Estimated effort:** 6–9 engineer-days. Most of these are tactical and independent. Voice CRITs (#28–#33) cluster together; Memory CRITs (#12–#16) are a tight sequence after today's schema fix. The DevOverlay (#42) is mostly already done; #41 and #36 are HUD-signal wiring.
|
||||
|
||||
**Key risks:** Voice fixes may surface deeper audio-graph install-order issues (re-emerge as B-04 in M-6); if so, the tactical patch in v0.1 stops short of full fix and that work moves to v0.8/M-6. The facts pipeline depends on user-side env work (#56, #58) — if the user hasn't run `ollama pull qwen2.5-coder:32b` and built `libsqlite3.dylib` with extensions, the memory CRITs cannot be fully verified.
|
||||
|
||||
**Cross-references:** several v0.1 fixes will be supersededd during the API migration — for example, #41 (MenuBarIconController) overlaps with HUD-state moving to Diagnostics in M-3 (v0.5). The tactical fix ships in v0.1 anyway; the migration replaces the implementation but preserves the user-visible behavior.
|
||||
|
||||
### v0.2 — Observable + stable (audit punch-list) — 23 issues
|
||||
|
||||
**User outcome:** Jarvis is not just working — it is observable. Every audit finding from 2026-05-12 that wasn't a v0.1 blocker is addressed. Token caching actually triggers (#23). Tool-result bytes are accurate in replay (#21 was v0.1, #18 LOW here). Banners surface correctly. Confirmation panels render in fullscreen and on multiple monitors (#26). PTT works (#87). Input Monitoring denial surfaces a HUD banner with a deep link to System Settings (#88).
|
||||
|
||||
**Scope:** Remaining MED/LOW audit findings and the two non-CRIT carry-forward UX gaps (#87 PTT, #88 Input Monitoring TCC).
|
||||
|
||||
**Issues (23):**
|
||||
- **Memory MED/LOW (3):** #17, #18, #19
|
||||
- **MCP/Agent MED/HIGH (6):** #22, #23, #24, #25, #26, #27
|
||||
- **Voice MED/LOW (6):** #35, #37, #38, #39, #54, #55
|
||||
- **Bus MED/LOW (4):** #43, #44, #45, #47
|
||||
- **Build LOW (1):** #46
|
||||
- **Carry-forward UX (2):** #87 PTT, #88 Input Monitoring
|
||||
- **Live verify (1):** #62
|
||||
|
||||
**Estimated effort:** 3–5 engineer-days. Most issues are <2 hours individually; the long pole is #26 (confirmation panel multi-monitor) which needs real-hardware testing.
|
||||
|
||||
**Key risks:** None significant. v0.2 is mostly mop-up.
|
||||
|
||||
### v0.5 — API M-0..M-3 (Self / Settings / Diagnostics surfaces) — 31 issues
|
||||
|
||||
**User outcome:** The webview talks to typed Swift surfaces, not ad-hoc bus cases. From the user's lens this is mostly invisible — but B-08 (Claude paraphrases the model name) dissolves, HUD state has exactly one writer (no more dual-emit races), the Dev Overlay snapshot is a typed query rather than a bus dead-letter, and the Status panel surfaces typed `Diagnostics.systemBannerEnqueued` events. The substrate for v0.8 lands here.
|
||||
|
||||
**Scope:** Migration phases M-0 (substrate), M-1 (Self), M-2 (Settings), M-3 (Diagnostics) per `JARVIS-API-MIGRATION-PLAN.md`.
|
||||
|
||||
**Issues (31):**
|
||||
- **Epics (4):** #4 M-0, #5 M-1, #6 M-2, #7 M-3 — plus #76 (parent migration epic)
|
||||
- **M-0 substrate (10):** #90–#99
|
||||
- **M-1 Self surface (5):** #100–#104, plus #53 (B-08 dissolves here)
|
||||
- **M-2 Settings (4):** #105–#108
|
||||
- **M-3 Diagnostics (6):** #109–#114
|
||||
|
||||
**Estimated effort:** 6–9 engineer-days (M-0 2.5–3.5 + M-1 1.0–1.5 + M-2 1.5–2.0 + M-3 1.0–1.5 per migration plan).
|
||||
|
||||
**Key risks:** M-0.6 (HUD single-writer behavioral gate) and M-3.2 (HUD state moves to Diagnostics, old emit deleted) is the highest-risk pair — if the old emit isn't deleted, dual-write races appear in production. Failure mode #1 in the migration plan covers this; behavioral gate is the safety net.
|
||||
|
||||
### v0.8 — API M-4..M-7 (Memory / Vision / Voice / Turn surfaces) — 36 issues
|
||||
|
||||
**User outcome:** Every carry-forward UX bug closes. Camera button (B-03) works through real WKWebView round-trip. Voice input (B-04) and TTS (B-05) are wired through typed `synthesizeTurn`/`getVoiceState`. The chat panel hydrates on connect via `Turn.listTurns` instead of a frozen `sessionHistory` push (B-06 partial). The `OutboundBatcher` is partitioned per-`turnId` so submit-cancel-submit races stop leaking tokens across turns. After N≥3 days of soak, `BUS_PROTOCOL_VERSION` bumps to `2.4.0` strict and the dual-channel fallback retires.
|
||||
|
||||
**Scope:** Migration phases M-4 (Memory), M-5 (Vision), M-6 (Voice), M-7 (Turn) per migration plan.
|
||||
|
||||
**Issues (36):**
|
||||
- **Epics (4):** #8 M-4, #9 M-5, #10 M-6, #11 M-7
|
||||
- **M-4 Memory (6):** #115–#120
|
||||
- **M-5 Vision (5):** #121–#125 + #48 (B-03)
|
||||
- **M-6 Voice (9):** #126–#134 + #49 (B-04) + #50 (B-05)
|
||||
- **M-7 Turn (9):** #135–#143
|
||||
|
||||
**Estimated effort:** 12.5–17.5 engineer-days (M-4 2.0–3.0 + M-5 1.5–2.0 + M-6 4.0–5.5 + M-7 5.0–7.0 per migration plan).
|
||||
|
||||
**Key risks:** M-6 is the highest-risk migration in plan — audio-graph install-order risk plus entitlement re-verify. Failure mode #4 (substrate not actually fixed; surface lies green while audio is dead) is mitigated by the `audioLevelChanged ≥10 within 500ms` behavioral gate. M-7 has the worst rollback in plan; mitigation is dual-channel handshake stays active across N≥3-day soak before M-7.8 lands.
|
||||
|
||||
### v1.0 — Shipping (commenting, behavioral gates, first-launch polish) — 13 issues
|
||||
|
||||
**User outcome:** Code-commenting style applied to load-bearing files so the next contributor (or compaction-recovered Claude session) can read the codebase without rebuilding context from audit docs. Stale plan-id comments grep-gated out. Doc reconciliation against actually-shipped state. First-launch experience polished.
|
||||
|
||||
**Scope:** Commenting style guide rollout (#64 epic + #66–#72 + #74 grep gate + #75 CLAUDE.md TL;DR), doc reconciliation (#59, #60, #61).
|
||||
|
||||
**Issues (13):** #59, #60, #61, #64, #66, #67, #68, #69, #70, #71, #72, #74, #75
|
||||
|
||||
**Estimated effort:** 4–6 engineer-days, mostly cosmetic.
|
||||
|
||||
**Key risks:** None. This is polish.
|
||||
|
||||
### v1.1 — Backlog (Orpheus, WhisperKit, Voice Log, presence, advanced) — 11 issues
|
||||
|
||||
**User outcome:** TTS tier 2 (Orpheus, in-process MLX) actually lights up — quality TTS within 150–250 ms TTFA. WhisperKit STT fallback activated behind a feature flag. Voice Log menu item (B-07) resurfaces, finally giving a live wake-word/STT/TTS event stream visible in the UI. Vision T2 sidecar comes online (v1.2). Presence pipeline (`atDesk`/`presenceConfidence`) reactivates.
|
||||
|
||||
**Scope:** All v1.1+ deferrals from the migration plan §10.
|
||||
|
||||
**Issues (11):** #52 B-07, #73 leaf-package commenting, #77 Orpheus, #78 WhisperKit, #79 F-Q-E query cancellation, #80 F-15 rename polish, #81 OQ-T4 origin tags, #82 OQ-T2 fixture spec, #84 Vision T2 sidecar, #85 presence pipeline, #86 multi-client subscription
|
||||
|
||||
**Estimated effort:** open-ended; activation work per capability. Orpheus requires ~6 GB weights download + empirical TTFA verification.
|
||||
|
||||
## Dependency map
|
||||
|
||||
```
|
||||
v0.1 (basic capabilities) ─────────────────────┐
|
||||
│ │
|
||||
│ same audit corpus │ user sees working Jarvis NOW
|
||||
▼ │
|
||||
v0.2 (audit punch-list) │
|
||||
│ │
|
||||
│ stability + observability │
|
||||
▼ ▼
|
||||
v0.5 (API M-0..M-3) ──────► v0.8 (API M-4..M-7) ──────► v1.1 (deferred caps)
|
||||
│
|
||||
│ strict topo-sort: M-0 → M-1 → M-2 → M-3 → M-4 → M-5 → M-6 → M-7
|
||||
│
|
||||
└──► v1.0 (polish — orthogonal, any time after v0.1)
|
||||
```
|
||||
|
||||
- **v0.1 is independent** — no migration dependency, tactical patches.
|
||||
- **v0.2 depends on v0.1** for stability (don't pile observability on top of broken capabilities).
|
||||
- **v0.5 depends on v0.1+v0.2** for stable substrate; M-0..M-3 sequence is strict.
|
||||
- **v0.8 depends on v0.5** strictly per migration plan.
|
||||
- **v1.0 is orthogonal** — commenting + docs work can run any time after v0.1.
|
||||
- **v1.1 depends on v0.8** for the carry-forward bugs to be closed first.
|
||||
|
||||
## How to read this roadmap
|
||||
|
||||
Each milestone is **a coherent user outcome**, not a date. The issues in each milestone are sequenced for atomic-commit work; within v0.1, most fixes are independent (Voice CRITs cluster, Memory CRITs cluster, UX is its own lane). Within v0.5/v0.8 the migration plan is the authoritative sequence: M-0 blocks M-1 blocks M-2 etc., strict topo-sort.
|
||||
|
||||
Carry-forward bugs B-02 through B-08 close as follows:
|
||||
- B-02 (history threading) — already closed by the tactical patch on `develop` (commit `48ad8d6`-ish vicinity); regression-gated again in v0.8/M-4.
|
||||
- B-03 (camera button) — closes in v0.8/M-5.
|
||||
- B-04 (voice input dead) — closes in v0.8/M-6 substrate fix.
|
||||
- B-05 (TTS silent) — closes in v0.8/M-6 via `synthesizeTurn` command.
|
||||
- B-06 (chat scroll) — **tactical fix in v0.1** (#51); migration partial-close in v0.8/M-7 via `turnTextComplete` event.
|
||||
- B-07 (Voice Log menu) — DEFERRED to v1.1.
|
||||
- B-08 (model paraphrase) — closes in v0.5/M-1 via typed `modelDisplayName`.
|
||||
|
||||
## What this roadmap deliberately does NOT include
|
||||
|
||||
- **Multi-tenant / auth / commercial shipping** — out of scope per `PROJECT.md`. Single-user personal project.
|
||||
- **Screen capture / ScreenCaptureKit** — deferred to v2.
|
||||
- **Multi-panel holographic HUD / ambient data tiles** — v2.
|
||||
- **Broader MCP tool catalog beyond the starter three** — additive once the loop works; v2.
|
||||
- **Cloud TTS/STT** — hard rule.
|
||||
- **Sonnet as primary model** — superseded by Opus 4.7.
|
||||
- **Multi-machine sync (iCloud / backend)** — local-first until single-machine proves out.
|
||||
- **Fixing all 22 HIGH + 22 MEDIUM AUDIT-R4 findings as a discrete gate** — absorbed into build phases per `PROJECT.md`.
|
||||
|
||||
## Top 10 v0.1 sequencing recommendation
|
||||
|
||||
For the user picking up v0.1 work next, the natural execution order is:
|
||||
|
||||
1. **#65** — AppDelegate P0 commenting rewrite (in flight today)
|
||||
2. **#42** — DevOverlay wire-up (mostly done today; verify and close)
|
||||
3. **#41** — MenuBarIconController.transition wired
|
||||
4. **#40** — BridgeNavigationDelegate didFail arm
|
||||
5. **#28** — wake-word survives audio-graph rebuild
|
||||
6. **#29** — STT backend selection wired
|
||||
7. **#30, #31, #32, #33, #34, #36** — remaining voice CRITs as a cluster
|
||||
8. **#12, #13, #14, #15, #16** — memory CRITs as a cluster
|
||||
9. **#20, #21** — MCP turn-id observer + double-cap
|
||||
10. **#1, #2, #3** — vision CRITs/HIGHs
|
||||
|
||||
#51 (chat auto-scroll) and #89 (startup surfaces) drop in anywhere — they're independent webview work.
|
||||
|
||||
---
|
||||
|
||||
*Authoritative state lives in `.planning/STATE.md` (current GSD phase position) and the GitHub milestones at https://github.com/KofTwentyTwo/Jarvis/milestones. This document is the narrative; those are the system of record.*
|
||||
@@ -0,0 +1,279 @@
|
||||
# Bus + HUD + WKWebView Bridge — End-to-End Audit
|
||||
|
||||
**Date:** 2026-05-12
|
||||
**HEAD:** `a5999e2`
|
||||
**Scope:** `packages/Bus/`, `webview/packages/bus/`, `webview/packages/hud/`, `App/HUD/`, `App/MenuBar/`, `App/AppDelegate.swift` bridge / handshake / inbound-outbound surfaces.
|
||||
**Mode:** Read-only on production; written to this file only.
|
||||
**Calibration:** Look for shape-of-today's-bug — probe says ok, reality is wired-but-dead. Surface "wants to write HudState but goes through a non-coordinator path", outbound cases with no producer, inbound cases with `// TODO` arms, dead-letter views.
|
||||
|
||||
---
|
||||
|
||||
## TL;DR — top-3 by severity
|
||||
|
||||
1. **`BridgeNavigationDelegate` has no failure arm.** Only `didFinish` is implemented. If `index.html`, the JS bundle, or any sub-resource fails to load (bad reference, 404 on `file://`, JS parse error before mount), the navigation never finishes, `startHandshake()` is never invoked, the 2-second handshake timeout **is never even armed**, and the user sees a silent black HUD with no banner, no hard-block. The "missing index.html" hard-block on AppDelegate.swift:2553 is the only failure mode that surfaces.
|
||||
2. **`MenuBarIconController.transition(to:)` is production-dead.** The full CABasicAnimation surface (breath / rotate / shimmer / glow per state) ships and works in unit tests, but **no production code ever calls `transition(to:)`** — `HudStateCoordinator`'s emit closure fans HudState to the webview bridge only. The menu-bar icon never animates regardless of agent state. Only `applyHealth(_:)` (Round 4) is wired.
|
||||
3. **`DevOverlay` is a hollow shell.** `DevOverlayWindow` opens with a default `DevOverlayViewModel` holding `DevSnapshot.initial`. The `.devOverlay` broadcaster subscription on AppDelegate.swift:1808-1813 drains events into `_` with the comment "reserved for the DevOverlay emitter wiring in a follow-on plan." `DevOverlayBridge` is never instantiated. Opening the overlay shows static initial values (state=idle, 0 tokens, empty turn id) forever.
|
||||
|
||||
---
|
||||
|
||||
## Severity-ordered findings
|
||||
|
||||
### S1 — `BridgeNavigationDelegate` only implements `didFinish` (silent black HUD on bundle failure)
|
||||
|
||||
**Files:** `App/AppDelegate.swift:2715-2723`, `packages/Bus/Sources/Bus/WebviewBridge.swift:269-288`, `packages/Bus/Sources/Bus/Handshake.swift:24-26`.
|
||||
|
||||
`BridgeNavigationDelegate` is a 9-line class with one method:
|
||||
|
||||
```swift
|
||||
nonisolated func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
|
||||
MainActor.assumeIsolated { onDidFinish() }
|
||||
}
|
||||
```
|
||||
|
||||
It has no `webView(_:didFail:withError:)` and no `webView(_:didFailProvisionalNavigation:withError:)`. `WebviewBridge.startHandshake()` schedules the 2-second timeout (`HandshakeTiming.timeout = .seconds(2)`) **inside** `startHandshake`, which is only invoked from `didFinish`. So the failure mode chain is:
|
||||
|
||||
- `loadFileURL` succeeds (file exists on disk).
|
||||
- `index.html` body loads OK, but `<script src="./assets/index-fBrUXkPQ.js">` 404s (stale ref, mid-build copy, codesign strip).
|
||||
- WebKit fires `didFailProvisionalNavigation` (or no completion at all for a JS parse error post-DOMContentLoaded depending on phase).
|
||||
- `didFinish` never fires.
|
||||
- `startHandshake()` is never called.
|
||||
- The 2s timeout is never scheduled.
|
||||
- `handshakeState` remains `.idle` forever. No `.timedOut`. No banner. No hard-block alert.
|
||||
|
||||
Audit prior context noted "live host PID 18811's HandshakeState reaches `.armed` (BootHealth probe says yes)" — this is true when the bundle exists, but **the failure mode is silent on the host that *should* be showing a banner**. That is exactly the calibration bug shape — the probe (`handshakeState == .armed`) passes when reality is OK, and provides no signal when reality breaks at the *navigation* layer.
|
||||
|
||||
The 2s timeout in `Handshake.swift` advertises a 2s SLA. With no `didFailX` handler, the SLA is silently >∞ in the bundle-broken path.
|
||||
|
||||
**Fix sketch:** add `didFailProvisionalNavigation` + `didFail` to `BridgeNavigationDelegate`, route both into a new `WebviewBridge.failHandshake(reason:)` that transitions to `.timedOut` (or a new `.loadFailed`) and presents the same hard-block alert.
|
||||
|
||||
Tests: none currently cover the failure-arm path. `packages/Bus/Tests/BusTests/` has no `didFail*` references.
|
||||
|
||||
---
|
||||
|
||||
### S1 — `MenuBarIconController.transition(to:)` is production-dead
|
||||
|
||||
**Files:** `App/MenuBar/MenuBarIconController.swift:65-96`, `App/AppDelegate.swift` (all references).
|
||||
|
||||
The controller exposes a full per-state animation kit:
|
||||
|
||||
| State | Animation |
|
||||
| ------------------------ | ------------------------------- |
|
||||
| `.idle` | no animation |
|
||||
| `.listening` | `CAAnimationFactory.makeBreath()` |
|
||||
| `.thinking` | `CAAnimationFactory.makeRotate()` |
|
||||
| `.speaking` | `CAAnimationFactory.makeShimmer()` |
|
||||
| `.awaitingConfirmation` | `CAAnimationFactory.makeGlow()` |
|
||||
| `.reconfiguring`/`.booting` | `// TODO(Phase 3/4)` — share `.idle` |
|
||||
|
||||
`transition(to:)` is exercised exhaustively by `MenuBarIconControllerTests`. In production: zero call sites.
|
||||
|
||||
```
|
||||
$ grep -rn "menuBarController" App/ --include='*.swift' | grep -v Tests
|
||||
App/AppDelegate.swift:106: var menuBarController: MenuBarIconController?
|
||||
App/AppDelegate.swift:768: menuBarController?.applyHealth(snapshot.overallHealth)
|
||||
App/AppDelegate.swift:1305: if let menu = menuBarController?.contextMenu { ... }
|
||||
App/AppDelegate.swift:2290: if let menu = menuBarController?.contextMenu { ... }
|
||||
App/AppDelegate.swift:2393: menuBarController = controller
|
||||
```
|
||||
|
||||
`HudStateCoordinator` is constructed with an emit closure that only fans to `WebviewBridge.send(.hudState(_:))` (`App/AppDelegate.swift:2475-2480`). The menu-bar icon is never told. The icon stays in its default static state for the entire app lifetime regardless of `.listening`/`.thinking`/`.speaking` activity.
|
||||
|
||||
This is exactly the "subsystem WANTS to change HUD state but goes through a non-coordinator path" anti-pattern named in the audit scope item §6 — except in reverse: the subsystem CAN'T see the state because the coordinator never broadcasts to it.
|
||||
|
||||
**Fix sketch:** subscribe `MenuBarIconController.transition(to:)` to the same broadcast the bus emit consumes. Either expand the emit closure on AppDelegate.swift:2475 to also call `menuBarController?.transition(to:)`, or have `HudStateCoordinator` expose a second fan-out (broadcast pattern). Note this is *not* a HUD-08 single-writer violation — `transition(to:)` writes the menu-bar icon's `currentState`, not `BusOutbound.hudState`. The single-writer invariant covers the bus surface, not arbitrary subscribers.
|
||||
|
||||
---
|
||||
|
||||
### S1 — DevOverlay never receives data
|
||||
|
||||
**Files:** `App/AppDelegate.swift:2652-2659` + `:1803-1813`, `packages/DevOverlay/Sources/DevOverlay/DevOverlayWindow.swift`, `packages/DevOverlay/Sources/DevOverlay/DevOverlayBridge.swift`, `packages/AgentCore/Sources/AgentOrchestrator/DevSnapshotEmitter.swift`.
|
||||
|
||||
`toggleDevOverlay()` creates a `DevOverlayWindow()` with the default `DevOverlayViewModel`, which holds `DevSnapshot.initial` — a static struct with zeroes and an empty `turnId`. The window opens and renders that struct.
|
||||
|
||||
The broadcaster's `.devOverlay`-priority subscriber on `App/AppDelegate.swift:1808`:
|
||||
|
||||
```swift
|
||||
let devSub = await broadcaster.subscribe(priority: .devOverlay, capacity: 32)
|
||||
devOverlaySubscriberTask = Task {
|
||||
for await _ in devSub.stream {
|
||||
if Task.isCancelled { break }
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Drains every event to `_`. The comment (lines 1803-1807) is explicit:
|
||||
|
||||
> "DevOverlay subscriber (lossy — observational; reserved for the DevOverlay emitter wiring in a follow-on plan)."
|
||||
|
||||
So `DevOverlayBridge` (which exists in `packages/DevOverlay/Sources/DevOverlay/DevOverlayBridge.swift` and is the intended consumer) is never instantiated, never given the `DevOverlayViewModel` reference, never given the broadcaster stream. `DevSnapshotEmitter.swift` exists in AgentCore but produces a snapshot stream that nobody attaches to the view model.
|
||||
|
||||
This is the exact "DevOverlay subsystem dump… silently shows stale data" pattern called out in the audit scope item §10.
|
||||
|
||||
Audit-2026-05-03 noted DevOverlay wasn't in scope; the May-12 round confirms it's still a flat-line view.
|
||||
|
||||
**Fix sketch:** instantiate `DevOverlayBridge` in `toggleDevOverlay()` (or earlier, at orchestrator install time), wire `DevSnapshotEmitter` → broadcaster → `DevOverlayBridge` → `viewModel.update(_:)`. Replace the no-op drain on line 1809-1813 with the bridge.
|
||||
|
||||
---
|
||||
|
||||
### S2 — `BusOutbound.sessionHistory(turns:)` is a dead-letter case
|
||||
|
||||
**Files:** `packages/Bus/Sources/Bus/Protocol.swift:14-17`, `packages/Bus/Sources/Bus/BusOutbound.swift:22-25`, `webview/packages/bus/src/protocol.ts:60`, `webview/packages/hud/src/bus/client.ts:153-157`.
|
||||
|
||||
Plan 07-03 bumped `BUS_PROTOCOL_VERSION` 2.0.0 → 2.1.0 specifically to add `sessionHistory(turns:)`. The TS mirror tracks it. Both sides decode it. The JS handler reads:
|
||||
|
||||
```ts
|
||||
case 'sessionHistory':
|
||||
// Plan 07-06 added the case; AppDelegate.installMemory does not yet
|
||||
// hydrate (= WARN-INT-2). Until then we acknowledge but no-op so the
|
||||
// exhaustiveness sentinel below remains tight.
|
||||
break
|
||||
```
|
||||
|
||||
The Swift side has zero production constructors of `.sessionHistory(turns: …)`. Verified by:
|
||||
|
||||
```
|
||||
$ grep -rEn 'sessionHistory\(turns' App/ packages/ --include='*.swift'
|
||||
(no output)
|
||||
```
|
||||
|
||||
The version was bumped, the parser landed on both sides, the chat panel's hydration consumer is still a `break`. Both the producer and the consumer are explicitly marked "deferred." This is structurally fine, but it's the kind of "we landed the protocol bump for a feature that never shipped" debt the audit was looking for.
|
||||
|
||||
**Fix sketch:** either (a) actually wire `AppDelegate.installMemory` → `WebviewBridge.send(.sessionHistory(turns: lastN))` on webview-ready + chat-panel mount with a corresponding JS hydration arm, or (b) cut the case out and bump 2.3.0 → 3.0.0 (MAJOR — removed field). (a) is cheaper; (b) is honest. Given the chat panel currently *only* gets state via the in-flight token stream after a turn starts, hydration would actually be useful — the user sees an empty chat panel on every relaunch.
|
||||
|
||||
---
|
||||
|
||||
### S2 — `BusOutbound.audioLevel` produces but the consumer is a `break`
|
||||
|
||||
**Files:** `packages/Bus/Sources/Bus/OutboundBatcher.swift:82`, `packages/Voice/Sources/Voice/Control/AudioLevelEmitter.swift`, `App/AppDelegate.swift:1282-1292`, `webview/packages/hud/src/bus/client.ts:135-138`, `webview/packages/hud/src/hud/*.tsx` (no consumers).
|
||||
|
||||
Track B-7 / P1-1 wired `AudioLevelEmitter` so the listening-state ring would pulse on real mic RMS. Production code constructs the emitter and routes through `OutboundBatcher`. Frames are emitted live. The JS handler:
|
||||
|
||||
```ts
|
||||
case 'audioLevel':
|
||||
// Plan 03-03's ring shader uses a synthetic sine; Phase 6 binds this
|
||||
// to the real mic RMS. Intentional no-op for P3.
|
||||
break
|
||||
```
|
||||
|
||||
No `audioLevel`-driven uniform in any of `RingMesh.tsx`, `ParticleRing.tsx`, `SegmentedRing.tsx`, `CoreGlow.tsx`, `Reticle.tsx` (independently verified via `grep audioLevel webview/packages/hud/src/hud/`). So:
|
||||
|
||||
- Swift instruments the mic correctly.
|
||||
- The bus carries the frames at ~30 Hz.
|
||||
- The JS handler decodes them.
|
||||
- Nothing renders.
|
||||
|
||||
Same structural shape as `sessionHistory`: the audit-2026-05-03 prior-context audit already noted this in §2 "audioLevel is wired to nothing." It has not been touched since. The Swift production wire keeps spending CPU/IPC on a payload the renderer throws away.
|
||||
|
||||
**Fix sketch:** either (a) bind `audioLevel` to a `uAudioRMS` uniform on `RingMesh` so listening-state visibly reacts to voice input, or (b) stop emitting from `AudioLevelEmitter` until the consumer lands. (a) is the cheaper net-positive — `audioLevel` was the whole point of the listening-state ring per the prior audit.
|
||||
|
||||
---
|
||||
|
||||
### S2 — Documentation drift: `JarvisBusWorld` references throughout
|
||||
|
||||
**Files:** `packages/Bus/Sources/Bus/WebviewBridge.swift:10`, `packages/Bus/Sources/Bus/Resources/Injection.js:1`, `webview/packages/bus/src/bridge.ts:60-99,159`, `webview/packages/bus/dist/bridge.{js,d.ts}` (build artifact, stale), `webview/packages/bus/tests/bridge.test.ts:78-113`, `webview/packages/hud/src/main.tsx:9-15`, `App/AppDelegate.swift:484`.
|
||||
|
||||
Commit `fb41c5f` (2026-05-XX) unified the bridge on `WKContentWorld.page` — see the commit message:
|
||||
|
||||
> "Two related bugs blocking the HUD from advancing past BOOTING after handshake. Found via BootDiag instrumentation that proved Swift was correctly emitting .idle but the JS handler was never receiving it ... Fix: WebviewBridge defaults to WKContentWorld.page so Injection.js, the page bundle, and the script-message-handler all share one world."
|
||||
|
||||
The runtime is now correct (verified: `WebviewBridge.init` default is `contentWorld: WKContentWorld = .page`, line 86 + 106). But the codebase still describes the *prior* topology in 20+ docstrings and one variable comment in production source:
|
||||
|
||||
- `WebviewBridge.swift:10`: "Install a `WKScriptMessageHandlerWithReply` in an isolated content world (`JarvisBusWorld`) so page scripts cannot observe or post to it." — **No longer true.** Page scripts *can* observe (they share the world).
|
||||
- `Injection.js:1`: "Injected at document-start in WKContentWorld('JarvisBusWorld')." — wrong.
|
||||
- `bridge.ts:60-99` "WKContentWorld attach-to-existing path (H-02)" — the whole rationale is now obsolete; both stubs run in the same world. The duck-type `isJarvisBusShape` check is still load-bearing (it prevents the bundle's `installJarvisBus` from clobbering Injection.js's stub), but the *reason* documented is no longer the actual reason.
|
||||
- `main.tsx:9-15`: comments still describing the cross-world attach path.
|
||||
- `AppDelegate.swift:484`: same drift.
|
||||
|
||||
This is not a runtime bug — gate `check-no-evaluate-javascript.sh` passes, `check-bus-protocol-version.sh` passes, all bus tests pass. It IS exactly the kind of stale-doc trap that produces the next outage: a future agent reads `bridge.ts:60` and "fixes" the duck-type-guard branch because the rationale no longer applies, deleting the stub-protection check.
|
||||
|
||||
The dist artifact at `webview/packages/bus/dist/bridge.js` (which has the same stale comments) is shipped to consumers via `pnpm`. Whether anyone runs `tsc` against the `dist/` doc-comments is unclear.
|
||||
|
||||
**Fix sketch:** rename "JarvisBusWorld" → "page" / "shared world" in all comments. Rewrite the `bridge.ts:60-99` rationale to reflect that the duck-type guard is now defensive against double-install (Injection.js + bundle in the same world, both running `installJarvisBus`-like logic), not cross-world attach. Bump the bus package patch version.
|
||||
|
||||
---
|
||||
|
||||
### S3 — Stale build artifacts in `App/Resources/webview/assets/`
|
||||
|
||||
**File:** `App/Resources/webview/assets/`.
|
||||
|
||||
Audit-2026-05-03 noted 9 stale bundles totaling ~9.9 MB. Today: still 8 stale bundles (`index-BKfT8cyH.js` through `index-PXUzusnJ.js`) plus their `.map` files, with the active `index-fBrUXkPQ.js` referenced by `index.html`. Same pattern, ~6 weeks later. No clean step has been added to whatever copies `dist/` → `App/Resources/webview/`.
|
||||
|
||||
Not a runtime bug, just bundle bloat (each bundle is ~1.1 MB; with `.map`s the total is ~50+ MB of dead weight in the app's `.app` bundle).
|
||||
|
||||
**Fix sketch:** add `rm -rf App/Resources/webview/assets/*` before the `cp -R` in whatever build script copies the dist. Or have `scripts/build-webview.sh` own the cleanup.
|
||||
|
||||
---
|
||||
|
||||
### S3 — `WebviewBridge.sendRaw` propagates JSON-throwing through the public API
|
||||
|
||||
**File:** `packages/Bus/Sources/Bus/WebviewBridge.swift:180-194`.
|
||||
|
||||
```swift
|
||||
public func sendRaw(_ message: BusOutbound) async throws {
|
||||
let data = try encoder.encode(message)
|
||||
let json = String(decoding: data, as: UTF8.self)
|
||||
_ = try await evaluator.callAsync(...)
|
||||
}
|
||||
```
|
||||
|
||||
`encoder.encode(.audioLevel(rms: .nan))` will succeed (`Float.nan` encodes as `null`), but `decodeOutbound` JS-side will reject (`typeof parsed.rms !== "number"` is true for `null` since `typeof null === "object"`). Result: per-frame decode error in `onDecodeError` for every audio frame containing a `NaN`. Since `audioLevel` is currently a no-op consumer, this never manifests. But the day someone wires it through, it will surface as console-spammed decode errors at 30 Hz if the AudioLevelEmitter ever emits NaN.
|
||||
|
||||
**Fix sketch:** clamp `rms` to `0.0...1.0` in `AudioLevelEmitter`, or have `BusOutbound.audioLevel.encode` substitute `0.0` for non-finite floats.
|
||||
|
||||
Mark this S3 because the consumer is dead; it'll become S1 the moment §S2-audioLevel above is fixed.
|
||||
|
||||
---
|
||||
|
||||
## Verified healthy
|
||||
|
||||
These were checked and found to work as documented:
|
||||
|
||||
- **Handshake protocol.** Swift → JS hello → JS helloAck → `HandshakeState.armed`. Confirmed in `WebviewBridge.handleHelloAck` and `Injection.js`'s "hello is owned by the stub" auto-ack. Mismatch path correctly hard-blocks via `TCCAlertService.presentHardBlock`. Timeout (2s) is correctly armed by `startHandshake` (but see S1-§1: only after `didFinish`).
|
||||
- **`WKContentWorld` isolation.** Confirmed `.page` is the runtime default; `check-no-evaluate-javascript.sh` passes; `WebviewBridge.sendRaw` routes only through `callAsyncJavaScript` with primitive-string arguments (no JSON-into-body interpolation). No production calls to `evaluateJavaScript`.
|
||||
- **Bus protocol version (`2.3.0`).** `check-bus-protocol-version.sh` passes — Swift, TS, and Injection.js stub all agree. Fixture directories byte-identical between `packages/Bus/Tests/BusTests/Fixtures/` and `webview/packages/bus/fixtures/`. `check-bus-harness-parity.sh` passes — canonical `webview/bus-harness.html` matches bundled copy.
|
||||
- **Inbound bus events.** All 5 `BusInbound` cases (`helloAck`, `uiReady`, `frameAttachRequested`, `chatSubmit`, `chatCancelAndSubmit`) have real handlers in `AppDelegate.onInbound` (lines 2513-2530). `frameAttachRequested` → `frameAttachController.requestAttach(reason: .hudButton)`. `chatSubmit` → `handleChatSubmit`. `chatCancelAndSubmit` → `handleChatCancelAndSubmit`. No `case foo: break // TODO` arms. The `helloAck` + `uiReady` arm returns `nil` but those are no-op-by-design (helloAck is intercepted inline by `WebviewBridge.handleInboundString`).
|
||||
- **Outbound bus events with real production senders.**
|
||||
- `.hello` — `WebviewBridge.startHandshake` (line 154).
|
||||
- `.hudState` — `HudStateCoordinator` via emit closure (only writer per HUD-08).
|
||||
- `.tokenDelta` — `OutboundBatcher` driven by `BusForwarder` (`packages/AgentCore/Sources/AgentOrchestrator/BusForwarder.swift:72`) + `AgentOrchestrator.swift:550`.
|
||||
- `.toolCallStart` / `.toolCallEnd` — `MCPBusGatewayAdapter` (`App/MCPBusGatewayAdapter.swift:56,65,71`).
|
||||
- `.turnStarted` / `.turnEnded` — `AppBusForwarderSink` (`App/AppBusForwarderSink.swift:26,31`).
|
||||
- `.submitRejected` — `AppDelegate.handleTextOutcome` (`:2197`) + `AppBusForwarderSink:36`.
|
||||
- `.audioLevel` — `OutboundBatcher.drainBuffers` (`packages/Bus/Sources/Bus/OutboundBatcher.swift:82`) driven by `AudioLevelEmitter`. **Producer works; consumer is dead (see S2 above).**
|
||||
- `.sessionHistory` — **no producer (see S2 above).**
|
||||
- **`HudStateCoordinator` single-writer.** `check-single-writer-hudstate.sh` passes. `BusOutbound.hudState(_:)` is only constructed at AppDelegate.swift:2478 inside the coordinator's emit closure. No other constructor sites in production. The coordinator's 3-stream input (agent/voice/confirmation) precedence is correctly enforced via `resolved` + `emit` (lines 102, 179).
|
||||
- **Banner coordinator non-dismissible flag (Round 4).** `bootHealthCritical` enqueues with `nonDismissible: true` (priority 1); `HUDBannerCoordinator.dismissCurrent` correctly no-ops on `current?.nonDismissible == true` (line 69). Production wire on AppDelegate.swift:937: `bannerCoordinator?.enqueue(.bootHealthCritical(...))` fires when a critical health probe reports failure. Verified — would surface a non-dismissible banner if a critical subsystem (memory off, no API key, no MCP tools, webview never armed) failed today.
|
||||
- **`MenuBarIconController.applyHealth`** (Round 4 Slice 4). Live wire at AppDelegate.swift:768 from `subsystemHealthMonitor` snapshot. `contentTintColor = .systemRed` for `.loud`/`.critical`; clear for `.ok`/`.soft`. Correctly does *not* write `HudState` (HUD-08 invariant preserved). Live host with 8/8 ok health correctly sees a system-tinted icon.
|
||||
- **Chat panel rendering.** `App.tsx` mounts `ParticleRing`, `LoadingFallbacks`, `HudFrame`, `ChatPanel`, `ChatInput`. Tool-call card rendering wired in `bus/client.ts:95-134` (start/end → `store.upsertToolCall`). Frame-attach button: `CameraButton.tsx` posts `frameAttachRequested` inbound on click. Approval sentinel (`isApprovalPlaceholder`) correctly drives the "awaiting-approval" status label. Auto-scroll skipped per M-7 carry-forward.
|
||||
- **Bus tests.** `swift test --package-path packages/Bus` and `vitest` for the bus package exercise the round-trip path. Fixtures parity is enforced at build time.
|
||||
|
||||
---
|
||||
|
||||
## Open questions
|
||||
|
||||
1. **Should `sessionHistory` actually ship?** The case is dead on both sides. If the chat-panel hydration was deliberately deferred to a phase that hasn't started, it's wasting a protocol-version bump. Either commit Plan 07-06's emit + JS hydration arm, or roll the case back and bump to 3.0.0. Cheapest fix: emit on webview-ready with the most-recent ~20 turns and have the JS arm replay them into the chat store, then strip the `break`.
|
||||
2. **Should `audioLevel` actually drive the ring?** Prior audit flagged this 9 days ago and recommended `uAudioRMS` uniform. Still a no-op. Producer is wasting wire frames. Either bind a uniform or stop emitting from `AudioLevelEmitter`. The producer path is fully live including `AudioGraphOwner.subscribe()` (failed-subscription degradation logged at AppDelegate.swift:1291), so the cost of leaving it on is real (~30 Hz IPC + decode + handler call).
|
||||
3. **Should the menu-bar icon animate?** `transition(to:)` and its CABasicAnimation surface compile, ship, and unit-test. The decision to remove it from the production wire (or never to add it) isn't documented in a `Plan` or `DECISION.md` I could find. If the user-facing intent is "the icon should breathe/rotate/shimmer per state," wire the coordinator emit. If the intent is "icon is static, ring is dynamic," delete `transition(to:)` entirely and its tests — keep `applyHealth` as the only public mutator.
|
||||
4. **`BridgeNavigationDelegate.didFail*`.** Was this an oversight or an explicit decision that `loadFileURL` from `Bundle.main` "can't fail in practice"? In practice the JS bundle reference inside `index.html` can drift (and has, mid-build), and a missing/stale `assets/index-XXX.js` would never surface to the user. Either argue the case (and document) or add the failure arm.
|
||||
|
||||
---
|
||||
|
||||
## Patterns checked but not found
|
||||
|
||||
- TS handler with `// TODO: implement` in landed code — only `audioLevel` and `sessionHistory` no-ops, both with explicit "deferred to Phase X" comments (so they're known dead-letters, not silent TODOs).
|
||||
- Swift `case .foo: break // TODO` for inbound dispatch — none. All 5 inbound cases have real arms.
|
||||
- `WKScriptMessageHandler` that registers but never decodes payloads — none. `WebviewBridge.handleInboundString` decodes via `JSONDecoder` and ships every case to `onInbound`.
|
||||
- Outbound bus event with a comment "emitted by X" where X never emits — found three (`audioLevel`, `sessionHistory`, transitively `MenuBarIconController.transition`).
|
||||
- Stale `BusOutbound` cases referencing removed features — none. All cases reference live features; `sessionHistory` and `audioLevel` reference features whose *consumers* are dead.
|
||||
- HudStateCoordinator transition that takes effect but the shader reads a different uniform — covered by audit-2026-05-03 for the `idle` zero-motion bug (out of scope here per "Track A landed animated rings"). Verified `stateUniforms.ts:30` no longer hard-codes `pulse: 0.0` — needs cross-check against the running bundle but appears resolved by Track A.
|
||||
|
||||
---
|
||||
|
||||
## Methodology
|
||||
|
||||
- Ran the four scope-relevant gate scripts: `check-bus-protocol-version.sh`, `check-bus-harness-parity.sh`, `check-no-evaluate-javascript.sh`, `check-single-writer-hudstate.sh`. All pass.
|
||||
- Read all of `packages/Bus/Sources/Bus/`, `webview/packages/bus/src/`, `webview/packages/hud/src/bus/`, `webview/packages/hud/src/App.tsx`, `webview/packages/hud/src/main.tsx`, `App/HUD/`, `App/MenuBar/MenuBarIconController.swift`, and the bridge / handshake / installBus / onInbound / inbound / outbound regions of `App/AppDelegate.swift`.
|
||||
- For each `BusOutbound` case, grepped for production constructors (excluding `Tests/`, `BusOutbound.swift`, `Protocol.swift`).
|
||||
- For each `BusInbound` case, traced the arm in `bridge.onInbound`.
|
||||
- Cross-checked Injection.js's `protocolVersion` literal against `BUS_PROTOCOL_VERSION` in Protocol.swift + protocol.ts. Three-way parity OK.
|
||||
- Did not run live tests against PID 18811 (host is not currently running per `ps`).
|
||||
@@ -0,0 +1,375 @@
|
||||
# MCP Runtime + Agent Orchestrator Audit — 2026-05-12
|
||||
|
||||
**Branch:** `develop` · **HEAD:** `a5999e2` · **Mode:** read-only on production.
|
||||
**Calibration bug shape to mirror:** silent integration boundary failures masked
|
||||
by probes that return OK. Found three same-shape bugs, one of them as deeply
|
||||
buried as today's FK mismatch.
|
||||
|
||||
---
|
||||
|
||||
## TL;DR
|
||||
|
||||
The MCP composite dispatcher and the agent's outer streaming loop are
|
||||
structurally correct. The 10-tool catalog routes. Confirmation gating works.
|
||||
SSE / NDJSON / OpenAI-compat decoders cover their documented edge cases.
|
||||
**But two of the "audit-trail" seams that the codebase advertises as wired
|
||||
are silently dead in production**, and the tool-result data path double-caps
|
||||
at 8 KB while losing all evidence of the original blob. The same probe-says-OK
|
||||
pattern that hid today's Toby-the-dog fact persistence bug.
|
||||
|
||||
---
|
||||
|
||||
## Severity order
|
||||
|
||||
### CRIT-1 — `ReplayingToolResultObserver` `turnIDResolver` is permanently `nil`; SEC-07 dual-write + ME-04 channel are dead in production
|
||||
|
||||
**Evidence:**
|
||||
- `App/AppDelegate.swift:614` — production wiring passes a literal `{ nil }`:
|
||||
```swift
|
||||
let runtime = try await MCPRuntimeWiring.build(
|
||||
bundleURL: bundleURL,
|
||||
bus: busAdapter,
|
||||
replayChannel: channel,
|
||||
turnIDResolver: { nil }, // pre-orchestrator returns nil (observer logs without writing).
|
||||
inProcessRegistry: self.inProcessToolRegistry
|
||||
)
|
||||
```
|
||||
- `App/MCP/ReplayingToolResultObserver.swift:62-69` — the observer's `record(...)` guards
|
||||
`guard let turnId = await turnIDResolver() else { logger.debug(...); return }`. Every
|
||||
call returns at the guard.
|
||||
- The resolver is **never reassigned** after the orchestrator is later constructed
|
||||
(`installAgent`, AppDelegate:1671). Verified by grep: no production write site for
|
||||
`turnIDResolver`, no `var` declaration, no `.attach(...)` analogue.
|
||||
|
||||
**What this kills:**
|
||||
- SEC-07's documented dual-write of raw vs sanitized bytes (the audit-trail
|
||||
contract: "the viewer reconciles the two streams for slicing on 'did the model
|
||||
ever see content X?'") — dead.
|
||||
- ME-04's `BoundedAsyncChannel<ReplayEnvelope>(capacity: 2048, policy: .dropOldest)`
|
||||
+ drain Task — the channel **never receives a tool-result envelope** from the
|
||||
observer in production. The drain Task drains... nothing tool-shaped.
|
||||
- The four-seam contract that AppDelegate comments at lines 549-577 call "the
|
||||
AGENT-10 four-seam contract exercised by production traffic" — exercised only
|
||||
by `applicationWillFinishLaunching` setup; in production, this seam is empty.
|
||||
|
||||
**Pattern match:** identical shape to today's calibration bug. Boot health probe
|
||||
sees the channel + drain wired; structural-composition tests assert the observer
|
||||
gets a `record(...)` call; production reality is the guard returns at the first
|
||||
line and the channel stays empty. Probes return OK, reality is broken.
|
||||
|
||||
**Confirmation:** the orchestrator does still write `.toolResultFull` to ReplayLog
|
||||
**directly** at `AgentOrchestrator.swift:584`, so replay rows exist — but those
|
||||
rows carry the **post-sanitize, post-8KB-cap** bytes (see CRIT-2), not the raw
|
||||
helper output. The audit trail's `:raw`-suffixed sibling row never lands.
|
||||
|
||||
**Fix shape:** after `installAgent` constructs the orchestrator, mutate
|
||||
`ReplayingToolResultObserver`'s resolver (the observer is a class — make
|
||||
`turnIDResolver` a `var` and add an `attach(_ resolver:)` method analogous to
|
||||
`ConfirmationPresenterHolder.attach(_:)`). Alternatively, build the observer
|
||||
inside `installAgent` rather than `MCPRuntimeWiring.build` so the orchestrator
|
||||
reference is in scope at construction time.
|
||||
|
||||
---
|
||||
|
||||
### CRIT-2 — Tool-result bytes are 8 KB-capped TWICE, and the "full blob in replay log" promise is a lie
|
||||
|
||||
**Evidence:**
|
||||
- `packages/MCP/Sources/MCP/MCPToolDispatcher.swift:114` — stdio path returns
|
||||
`Data(SanitizeForModel.prepareForBoundary(rawText, capBytes: 8192).utf8)`.
|
||||
- `packages/MCP/Sources/MCP/InProcess/InProcessAwareToolDispatcher.swift:89` —
|
||||
in-process path returns `Data(SanitizeForModel.prepareForBoundary(rawText, capBytes: 8192).utf8)`.
|
||||
- `packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift:580-587` —
|
||||
the orchestrator then:
|
||||
```swift
|
||||
let resultData = try await toolDispatcher.dispatch(toolUse: req)
|
||||
let packed = ToolResultPacker.pack(resultData) // CAPS AGAIN at 8 KB
|
||||
await replayLog.record(.toolResultFull(toolUseId: req.id, bytes: packed.fullBytes), …)
|
||||
```
|
||||
`packed.fullBytes` is `resultData` — already 8 KB. The "full" in `toolResultFull`
|
||||
is **never larger than 8 KB**.
|
||||
- `packages/AgentCore/Sources/AgentCore/ToolResultPacker.swift:23` — the type's own
|
||||
doc comment says: *"The full blob still flows to ReplayLog via
|
||||
`ReplayEvent.toolResultFull(toolUseId:bytes:)` — observability is not lossy (OBS-02)."*
|
||||
This is false in production.
|
||||
|
||||
**Why nobody noticed:**
|
||||
- `ToolResultPacker.pack` is a pure function; its unit tests verify capping correctly
|
||||
but operate on synthetic inputs that haven't gone through the dispatcher's
|
||||
pre-cap.
|
||||
- `MCPToolDispatcher` unit tests verify sanitize correctness on its output but
|
||||
don't trace the byte path into the orchestrator.
|
||||
- The two layers were designed independently — one in MCP (boundary defense),
|
||||
one in AgentCore (model-facing protection) — and the intended division was
|
||||
"MCP caps the model-facing string, the orchestrator passes through to replay".
|
||||
But the orchestrator pipeline doesn't have a "raw bytes" inlet — it consumes
|
||||
the dispatcher's return value as both the model-facing AND the replay-facing
|
||||
payload.
|
||||
|
||||
**Combined impact with CRIT-1:** The dual-write that was supposed to give us
|
||||
the un-capped truth (`:raw` suffix path through the observer → channel → replay)
|
||||
is dead. The orchestrator's direct write captures the capped truth. There is no
|
||||
production path that records the original tool output bytes anywhere on disk.
|
||||
|
||||
**Fix shape:** Move the `prepareForBoundary` call out of both dispatchers and
|
||||
into the orchestrator (or into `ToolResultPacker.pack`), and have the dispatchers
|
||||
return the **raw** helper bytes. Then the orchestrator can:
|
||||
1. Send raw bytes to ReplayLog as `.toolResultFull` (true to the name).
|
||||
2. Run `prepareForBoundary` + `ToolResultPacker.pack` once, embed the capped
|
||||
string in the model-facing tool_result.
|
||||
|
||||
This also collapses the dead observer dual-write back into a single source of
|
||||
truth, fixing CRIT-1 by deletion.
|
||||
|
||||
---
|
||||
|
||||
### CRIT-3 — `MCPRuntime built — N tools` log line undercounts; misleading triage signal
|
||||
|
||||
**Evidence:**
|
||||
- `App/AppDelegate.swift:618-619`:
|
||||
```swift
|
||||
let toolCount = await runtime.client.registeredToolNames().count
|
||||
self.systemLogger?.info("MCPRuntime built — \(toolCount) tools")
|
||||
```
|
||||
- `runtime.client.registeredToolNames()` only returns the **stdio** helper tools
|
||||
(3 — `get_time`, `get_clipboard`, `run_applescript`). The 7 in-process tools
|
||||
registered into `inProcessToolRegistry` are not counted.
|
||||
|
||||
**Impact:** The boot log perpetually shows `MCPRuntime built — 3 tools` while
|
||||
`MCPBootHealthProbe` (which counts both registries) correctly reports 10. A
|
||||
contributor (or future Claude) reading the system log will conclude the
|
||||
in-process tools didn't register and chase the wrong root cause when the actual
|
||||
problem is elsewhere.
|
||||
|
||||
**Note:** the probe at `App/Boot/BootHealthProbes.swift:336-391` correctly counts
|
||||
both. The boot log line is the only place that's wrong. Today's `740bed4` commit
|
||||
already addressed a related undercount in the probe; this log line was missed.
|
||||
|
||||
**Fix:** include `inProcessToolRegistry.registered().count` in the log line,
|
||||
or just delete the line and rely on `MCPBootHealthProbe` for the count.
|
||||
|
||||
---
|
||||
|
||||
### HIGH-1 — Cache hints effectively never trigger in production (no correctness bug, but cost is higher than design intent)
|
||||
|
||||
**Evidence:**
|
||||
- `packages/AgentCore/Sources/AgentCore/CacheHints.swift:50` —
|
||||
`MIN_CHARS_FOR_CACHE_BREAKPOINT = 4096`.
|
||||
- `packages/AgentCore/Sources/AgentCore/ContextBuilder.swift:64-74` — the locked
|
||||
`selfAwarePreamble` is ~700 characters (verified by grep — file is 4462 bytes
|
||||
total including comments).
|
||||
- `AppDelegate.swift:1685` constructs the orchestrator with
|
||||
`systemPrompt: ContextBuilder.systemPrompt(for: .empty)` — `.empty` adds no
|
||||
presence or memory hydration text.
|
||||
- Round 4 degradation summary prepends a small string only when subsystems are
|
||||
unhealthy. The prepended text is bounded "below the 4096-char Anthropic cache
|
||||
breakpoint" by design (AgentOrchestrator:118).
|
||||
- Therefore on a healthy boot, `finalSystem.count` is ~700-1200 chars depending
|
||||
on presence enrichment. `CacheHints.eligibleForSystemPrompt(finalSystem)`
|
||||
returns `nil` on every turn.
|
||||
|
||||
**Impact:** No correctness bug — the Phase E fix correctly returns `nil` to
|
||||
avoid the 200 OK + EOF cache-validation-failure cascade. But the prompt cache
|
||||
is effectively dead. Every turn pays full input-token cost. With Opus 4.7's
|
||||
~1.35× token inflation vs 3.x (per CLAUDE.md), this matters more than it
|
||||
would have on older models.
|
||||
|
||||
**Not a fix request — flagging for awareness.** Possible mitigations: pad the
|
||||
preamble to ≥4096 chars (would need real prompt-engineering work), or accept
|
||||
that cache is unused until memory hydration grows the prompt naturally.
|
||||
|
||||
---
|
||||
|
||||
### HIGH-2 — Assistant `text` content blocks emitted alongside `tool_use` are dropped from the message history
|
||||
|
||||
**Evidence:**
|
||||
- `packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift:575-577`:
|
||||
```swift
|
||||
messages.append(LLMMessage(role: .assistant, content: [
|
||||
.toolUse(id: req.id, name: req.name, argsJSON: req.argsJSON),
|
||||
]))
|
||||
```
|
||||
- When Anthropic streams a turn that contains text deltas BEFORE a tool_use
|
||||
block (which Opus 4.7 does frequently — "Let me check the time for you…
|
||||
[tool_use:get_time]"), the orchestrator's `assistantTextSoFar` accumulator
|
||||
captures the text for `assistantTextSoFar` (used by D-02 escalation), but
|
||||
the appended assistant message contains **only** the tool_use block. The
|
||||
preceding text is gone from the model's view on the next round trip.
|
||||
|
||||
**Impact:** Generally tolerable for Anthropic — the model resolves tool calls
|
||||
by id and doesn't re-read its own prose between turns. But:
|
||||
1. If the user submits a follow-up that references "you said X before calling
|
||||
the tool", the model has no record of saying X.
|
||||
2. Replay log captures the text via `.textDelta` events, but the replay viewer
|
||||
reconstruction won't match what the model actually saw at re-prompt.
|
||||
|
||||
**Pattern match:** lower-severity version of the calibration bug shape — the
|
||||
tests pass (`messages.last` is the assistant message they expected), the data
|
||||
boundary silently loses signal.
|
||||
|
||||
**Fix shape:** accumulate text + tool_use blocks into a single assistant
|
||||
`LLMMessage` per round-trip, flushing on either `stop_reason: .toolUse` (with
|
||||
tool blocks present) or `stop_reason: .endTurn` (text only).
|
||||
|
||||
---
|
||||
|
||||
### MED-1 — `get_active_audio_route` registration is conditional on `audioGraphOwner != nil`; voice DAG failure silently strands a documented tool
|
||||
|
||||
**Evidence:**
|
||||
- `App/AppDelegate.swift:1966-1976` — `installSelfKnowledgeTools` registers
|
||||
`GetActiveAudioRouteTool` only inside `if let owner = self.audioGraphOwner`.
|
||||
Else: `systemLogger?.warning("…get_active_audio_route not registered")`.
|
||||
- The Round-4 self-aware preamble (`ContextBuilder.selfAwarePreamble`) **names
|
||||
`get_active_audio_route` as a tool the model should prefer over generic
|
||||
answers.** When voice fails (e.g., missing OpenWakeWord/Silero models on
|
||||
first launch), the tool catalog the model receives doesn't include this
|
||||
tool, but the preamble still tells the model to use it. The model emits a
|
||||
`tool_use` for `get_active_audio_route` → composite dispatcher fails-unknown
|
||||
→ orchestrator records ERROR → user-facing chat shows tool failure.
|
||||
|
||||
**Pattern match:** wired-but-conditionally-dead, exactly the audit's stated
|
||||
pattern.
|
||||
|
||||
**Fix shape:** either always register a dispatcher that returns
|
||||
`{"route": null}` when the owner is missing (preserving the catalog promise),
|
||||
or gate the preamble's mention of the tool on whether voice is healthy at
|
||||
boot. Round-4 degradation summary already handles the latter at the system-
|
||||
prompt level — extend it to omit the tool name from the preamble when the
|
||||
subsystem is dead.
|
||||
|
||||
---
|
||||
|
||||
### MED-2 — Confirmation panel's `beginSheet` requires the hidden owner panel to be `orderFront`'d; multi-monitor / fullscreen-app scenarios may hide the prompt
|
||||
|
||||
**Evidence:**
|
||||
- `packages/MCP/Sources/MCP/ConfirmationPresenter.swift:121-128`:
|
||||
```swift
|
||||
owner.alphaValue = 0
|
||||
owner.orderFront(nil)
|
||||
owner.beginSheet(panel) { _ in }
|
||||
```
|
||||
- The owner panel is `alphaValue = 0` and `level = .floating`. `beginSheet`
|
||||
attaches the visible panel to the invisible owner. If the user is in a
|
||||
fullscreen app on a different Space, `orderFront(nil)` may not be enough —
|
||||
the sheet attaches but the user can't see it. The broker still waits the
|
||||
full 60s timeout, then synthesizes `.timeout` → tool call denied.
|
||||
|
||||
**Impact:** Not a correctness bug at the protocol level (the broker correctly
|
||||
times out), but UX-wise this is "tool call silently denied because user never
|
||||
saw the prompt." The HUD itself has logic to overlay regardless of Space —
|
||||
the confirmation prompt doesn't share that logic.
|
||||
|
||||
**Pattern match:** "confirmation gate works but blocks until accept/deny"
|
||||
holds — what doesn't hold is that the user reliably *sees* the prompt to
|
||||
accept/deny. Not the audit's stated bug shape, but adjacent.
|
||||
|
||||
**Fix:** consider routing through `HUDBanner` (already overlays cross-Space)
|
||||
for the confirmation card instead of beginSheet.
|
||||
|
||||
---
|
||||
|
||||
### LOW-1 — `MCPBusGatewayAdapter.updateArgsPreview` re-emits `toolCallStart`, not a distinct event
|
||||
|
||||
**Evidence:**
|
||||
- `App/MCPBusGatewayAdapter.swift:59-66` — second-phase post-approval emission
|
||||
reuses `BusOutbound.toolCallStart(id: uuid, name:, argsPreview:)`. The HUD
|
||||
upserts by id.
|
||||
- The comment correctly notes the HUD reconciles by id — fine in practice. But
|
||||
this means the bus protocol has no distinct "args revealed" event, so
|
||||
downstream consumers (replay viewer, future tracing) can't distinguish the
|
||||
awaiting-approval emission from the post-approval emission without doing
|
||||
a lookback by id.
|
||||
|
||||
**Impact:** observability nuance, not a bug. Flag for the bus protocol v2
|
||||
discussion if anyone revisits the wire schema.
|
||||
|
||||
---
|
||||
|
||||
## Verified healthy
|
||||
|
||||
The following were probed and found correct as of this audit:
|
||||
|
||||
- **In-process registry composition.** `InProcessAwareToolDispatcher` routes
|
||||
in-process tools via `inProcessRegistry.contains(name)` lookup (async, hits
|
||||
live actor state — late registration is routable). Stdio fallthrough works.
|
||||
`requiresConfirmation` lookup is sync via the lock-protected
|
||||
`InProcessConfirmationCache`. `forget_fact` is the only in-process tool
|
||||
with `requiresConfirmation: true`; it threads correctly through the same
|
||||
`ConfirmationBroker` as `run_applescript`.
|
||||
- **10-tool catalog enumeration.** `availableToolsResolver` at
|
||||
`AppDelegate.swift:1624-1635` lazily merges `mcpClient.toolCatalog()` (3
|
||||
stdio) with `inProcessToolRegistry.toolSchemas()` (up to 7 in-process). Fires
|
||||
per outer-loop iteration so late-registered self-knowledge tools become
|
||||
visible without orchestrator rebuild.
|
||||
- **AppleScript confirmation gating.** `check-applescript-confirmation.sh` is
|
||||
green. `mcp-applescript` registers with `requiresConfirmation: true`. The
|
||||
outer `ConfirmingToolDispatcher` correctly emits `awaitingApproval` to the
|
||||
bus pre-broker, awaits broker, branches on outcome. `.deny` / `.timeout` /
|
||||
`.barge` all throw distinct `ConfirmationError` cases that propagate as
|
||||
tool_result errors.
|
||||
- **Ollama NDJSON decoder AGENT-04 invariant.** `NDJSONDecoder.dispatch` emits
|
||||
`.toolUseRequested` on *every* chunk carrying `tool_calls` regardless of the
|
||||
`done` flag, never gated on the terminator. Matches the CLAUDE.md gotcha.
|
||||
- **OpenAI-compat decoder.** Atomic `tool_calls` flushed on `[DONE]` and on
|
||||
`finish_reason` presence. Handles both string and object `arguments`. Buffers
|
||||
per-call_id for the rare case OpenAI splits args.
|
||||
- **AnthropicProvider beta header.** `anthropic-beta: extended-cache-ttl-2025-04-11`
|
||||
is set unconditionally (AnthropicProvider:114). Phase E's cache-hint gating
|
||||
prevents the 200 OK + EOF cascade by returning nil cacheHints when the
|
||||
prompt is under threshold (HIGH-1 documents the side effect).
|
||||
- **Cap-recovery `toolChoice: .none` semantics.** Anthropic encodes
|
||||
`{type:"none"}`. Ollama native drops the `tools` array entirely
|
||||
(`OllamaRequestBody.encodeNative:39-46`). OpenAI-compat sends `"none"` as
|
||||
the string. All three match CLAUDE.md's invariant.
|
||||
- **Broadcaster fan-out.** Six subscribers (`memory`, `transcript`, `devOverlay`,
|
||||
`frameAttach`, `voice`, `bus`) all subscribe + drain in `installAgent`; all
|
||||
six are cancelled in `applicationWillTerminate`. The drain Tasks `for await
|
||||
event in sub.stream` against live producers wired through the broadcaster's
|
||||
internal mirror. The protected-event matrix in
|
||||
`OrchestratorEventBroadcaster.isProtectedForPriority` correctly classifies
|
||||
`tokenDelta`/`thinkingDelta` as drop-eligible while `turnEnd`/`error` etc.
|
||||
are protected.
|
||||
- **`check-orchestrator-events-single-consumer.sh` invariant.** No
|
||||
`for await … in …events` outside the broadcaster (confirmed by recent
|
||||
edits — broadcaster is the single drain of `orchestrator.events`).
|
||||
- **`ConfirmationBroker` first-write-wins.** `resolved` guard prevents
|
||||
double-resolve. Timer task cancellation on response. `WR-03` explicit
|
||||
Task.sleep error handling.
|
||||
- **Pending-message persistence to `turns` table (B-02).** `appendTurn`
|
||||
for user + assistant pair fires inside the memory subscriber's
|
||||
`turnContent` lookup closure at `AppDelegate.swift:1749-1772`. Confirmed
|
||||
by the recent commit chain.
|
||||
|
||||
---
|
||||
|
||||
## Open questions
|
||||
|
||||
1. **Should the orchestrator's direct `.toolResultFull` write be removed?** If
|
||||
CRIT-1 is fixed by routing through the observer + channel, the direct
|
||||
write becomes redundant. The current code writes the capped bytes here
|
||||
AND would write raw bytes through the observer. Decide on single source of
|
||||
truth for replay.
|
||||
2. **Is the cache-hint dead-state intentional cost-acceptance, or oversight?**
|
||||
`extended1h` flow with a permanently-under-threshold prompt looks like an
|
||||
"obvious-when-pointed-at" gap. If accepted, comment it; if not, decide on
|
||||
a padding strategy.
|
||||
3. **Is the assistant-text-drop (HIGH-2) breaking any current eval scenarios?**
|
||||
The eval harness corpus would need a tool-call-after-text scenario to
|
||||
detect this; not obvious from the audit whether any exist.
|
||||
4. **`turnIDResolver` shape:** when fixed, the resolver needs to hop to the
|
||||
orchestrator actor to read `currentTurn?.id`. That's an await per tool
|
||||
dispatch — acceptable cost? Or should the orchestrator push the active
|
||||
TurnID into a lock-protected slot the observer can read sync (similar to
|
||||
`InProcessConfirmationCache`)?
|
||||
|
||||
---
|
||||
|
||||
## Probes I trusted, in order to be transparent
|
||||
|
||||
- `MCPBootHealthProbe` — re-verified by reading code; counts both registries.
|
||||
- `check-applescript-confirmation.sh` — re-ran live; PASS.
|
||||
- `check-orchestrator-events-single-consumer.sh` — assumed green per gates
|
||||
list in CLAUDE.md; did not re-run live (parallel audits may be touching
|
||||
surface area).
|
||||
|
||||
Files & line numbers above are all absolute under the repo root. No production
|
||||
code was modified.
|
||||
@@ -0,0 +1,98 @@
|
||||
# Memory subsystem audit — 2026-05-12
|
||||
|
||||
**HEAD:** `a5999e2`
|
||||
**Scope:** Memory pipeline end-to-end — extraction, store, search, forget, stats, replay integration, vec0 linkage
|
||||
**Calibration bug (today's reference):** `facts.source_turn_id` FK / writer hash mismatch (fixed in `a5999e2` by dropping the FK)
|
||||
**Live probe context:** PID 18811 has been up ~1h45m; `~/Library/Application Support/Jarvis/jarvis.db` has 8 turns, 1 fact, **0 facts_vec rows**, 1 facts_fts row. Today's session_id `8FC15E7A-F2B4-4FCA-A79F-947E34C5F27B`.
|
||||
|
||||
Audit ran read-only against the live DB plus the package source. Three CRITICAL findings, two HIGH, one MEDIUM, two LOW. No findings re-flag pre-2026-05-04 issues already closed.
|
||||
|
||||
## Findings (severity-ordered)
|
||||
|
||||
### M-1 (CRITICAL): `facts_vec` is never written — semantic search branch is permanently empty
|
||||
|
||||
**Path:** `packages/Memory/Sources/Memory/MemoryStore.swift:132-246` (`applyOp` / `insertNewFact`)
|
||||
**Symptom:** `search_memory` works in keyword mode only. The hybrid SQL's `vc` CTE (line 43-48 of `MemoryQueries.hybridSearchSQL`) returns zero rows on every call. RRF degenerates to FTS5-only ranking. Semantic recall ("dog" → "Toby" via embedding similarity) silently doesn't work.
|
||||
**Root cause:** `applyOp` inserts into `facts` (via the `INSERT INTO facts(...)` block at line 168-182) and the schema's FTS5 trigger mirrors `facts_fts` automatically — **but nothing inserts into `facts_vec`**. There is no embedder call in `insertNewFact`, no INSERT INTO facts_vec anywhere in `MemoryStore.swift`, no equivalent path in `MemoryExtractionOrchestrator.process`, no path in `MemoryExtractor.extract`. The schema exists, the extension is loaded (`vec_version: v0.1.6`), the embedder client (`OllamaEmbeddingClient`) exists and works — but no producer wires them to the writer side.
|
||||
**Evidence:** Live DB — `SELECT COUNT(*) FROM facts_vec_rowids` → `0` despite 1 fact present. `SELECT COUNT(*) FROM facts` → `1`. The Toby fact (`id=1`, subject="user", predicate="name is", object="Toby") has an FTS row but no vec row. Schema-vs-writer impedance mismatch identical in shape to today's FK calibration bug — the schema declares a column/table the writer never produces.
|
||||
**Recommended fix:** Inside `insertNewFact`, after the `INSERT INTO facts` succeeds and `newId` is known, run the query embedder against `"\(subject) \(predicate) \(object)"` (or a richer text projection) and `INSERT INTO facts_vec(fact_id, embedding) VALUES (?, ?)` inside the same transaction. The embedder is async — `MemoryStore` is an actor so awaiting is fine, but the embedder must be injected (the current `applyOp` has no embedder reference; `installMemory` builds the embedder for `HybridSearch` only). Pattern options: (a) make `MemoryStore.init` take `embedder: any EmbeddingProviding?` and skip the vec INSERT when nil; (b) move the embed+insert step into `MemoryExtractionOrchestrator.process` after `applyOp` returns the new `Fact`. Either pins the same actor-owned connection and keeps single-emit invariants intact. Failure mode discipline: log + leave the FTS row (degraded but searchable) rather than rolling back the fact row.
|
||||
|
||||
### M-2 (CRITICAL): Replay log poisoned — synthetic `memory-trigger-<hash>` turn_ids break ALL replay flushes since 18:48
|
||||
|
||||
**Path:** `App/AppDelegate.swift:2783-2797` (`AppDelegateMemoryReplaySink.record(_:forTriggerTurnId:)`), with downstream FK enforcement in `packages/Replay/Sources/Replay/Schema.swift:15` (`PRAGMA foreign_keys=ON`) and `packages/Replay/Sources/Replay/ReplayLog.swift:195-260` (`flushPending`'s batch transaction).
|
||||
**Symptom:** Every replay batch since `2026-05-12T18:48:47.211Z` has failed with `SQLITE_CONSTRAINT_FOREIGNKEY` (code 19). The ReplayLog re-buffers the poisoned batch at the front of `pending` (line 239), retries, fails again, and at 3 consecutive failures escalates to `CRITICAL`. Current state per `replay.2026-05-12.log`: ~50+ chronic-failure entries, `batch_size=31` and growing, `consecutive_failures=23+`. The 1 real fact (which fired memoryMutation) is silently lost; so are dozens of real turn-event rows queued behind it (text_delta, tool_call_requested, turn_end). `events.row_id` MAX in replay.sqlite is from before the poisoning — observability is dead until the process restarts.
|
||||
**Root cause:** When `MemoryStore.recordMemoryMutation` (line 248-272) fires, it routes through the sink with a synthetic TurnID `"memory-trigger-7747877241524896942"` (the FNV hash from `MemoryExtractionOrchestrator.stableHash`, formatted as a string). `replay.sqlite`'s `events.turn_id REFERENCES turns(turn_id)` is a real FK with `foreign_keys=ON`; there is no row in `turns` with that synthetic id, so the INSERT inside the batch transaction throws `SQLITE_CONSTRAINT_FOREIGNKEY`. Because `flushPending` wraps the WHOLE batch in one transaction, the poisoned event takes every real event in the same batch down with it on rollback. The re-buffer logic doesn't filter — same poison gets retried forever. Identical FK-vs-writer impedance shape as the calibration bug, in a second DB.
|
||||
**Evidence:** `~/Library/Logs/Jarvis/replay.2026-05-12.log` first 50 lines all show `turn_ids=...,memory-trigger-7747877241524896942`. Real session turn_ids (`5F3E38F7-259F-4060-8C17-EB304A042826`, `A4A95D0F-64F9-49D9-88A4-1ECF53B9367A`) are stuck in the same batch and never persist. `sqlite3 replay.sqlite "SELECT kind, COUNT(*) FROM events GROUP BY kind"` shows only events from before 18:48 — no `memory_mutation` rows ever, despite the 1 fact written today.
|
||||
**Recommended fix:** Two layers, pick at least one:
|
||||
1. **Don't synthesize a fake turn_id.** Either thread the real triggering `TurnID` (UUID) through to the sink (requires plumbing the real ID through `MemoryExtractionCoordinator` → `Orchestrator.process` → `applyOp` → `recordMemoryMutation`; the orchestrator already has `job.turnId` — pass it through alongside the Int64 hash and use the real one when emitting to replay), or accept that memory mutations aren't pinned to a single turn and add a sentinel `turns` row up-front for them (`session_id = current`, `turn_id = "memory-system"` or per-session). The latter is one extra INSERT at session start.
|
||||
2. **Defensive batch-splitting in `ReplayLog.flushPending`.** When a batch fails, retry events individually so one poisoned row can't take 30 others with it. This bounds the blast radius even if a new poisoned-id pathway sneaks in later.
|
||||
|
||||
### M-3 (CRITICAL): Agent has no preamble cue to call `search_memory` — proactive recall is dead by discovery gap
|
||||
|
||||
**Path:** `packages/AgentCore/Sources/AgentCore/ContextBuilder.swift:64-74` (`selfAwarePreamble`), plus the absent memory-hydration wiring in `App/AppDelegate.swift:1685` (`systemPrompt: ContextBuilder.systemPrompt(for: .empty)`).
|
||||
**Symptom:** Today's session perfectly reproduces this. Turn 1: user says "what is my dog's name?". The agent has `search_memory` registered in the tools array (verified: 10 tools probed `ok`), but answers "I don't have anything in memory about your dog. What's their name?" without calling it. Turn 2: user says "Toby". Agent: "Got it — Toby. I'll remember that." It does NOT call `search_memory` first to check, never has, and live logs show zero `searchFacts:` log lines anywhere in `~/Library/Logs/Jarvis/`. (`memory.search` channel is silent across all sessions.)
|
||||
**Root cause:** The preamble enumerates 7 introspection tools by name — `list_audio_devices, get_active_audio_route, get_self_state, list_camera_devices, get_time, get_clipboard, run_applescript` — but **omits both `search_memory` and `forget_fact`**. Anthropic's tools array is canonical, but the model's preference is shaped by the preamble's explicit "always prefer calling those tools over giving generic instructions" rule. Without a name in the preamble, `search_memory` falls into the bin of "tools the agent technically has access to but won't think to use." Confirmed by behavior. The `TurnContext.memoryHydration` field exists in `ContextBuilder` (line 41) and `systemPrompt(for:)` will splice it in if non-nil, but **the only production call site at `AppDelegate.swift:1685` passes `.empty`** — there's no glue that pre-populates `memoryHydration` from a `HybridSearch` pass before each turn. So neither path (proactive tool call nor auto-hydration) is alive.
|
||||
**Evidence:** `grep -rn "search_memory" packages/AgentCore/` → only the audit comment, never in the preamble. `grep -rn "memoryHydration" App/` → only the type declaration; no wiring. Live `system.2026-05-12.log` lines `BootHealth: subsystem=mcp ... search_memory` present. Live `memory.*log` files have zero `[memory.search]` channel emissions. Replay events table shows 19 `tool_call_requested` rows — all `get_time` / `get_self_state` / etc., zero `search_memory`.
|
||||
**Recommended fix:** Add `search_memory` (and arguably `forget_fact`) to the preamble's named-tool list, with a specific cue: e.g., `"When the user references something they may have told you before, or asks 'do you remember…', call search_memory FIRST. Never claim to remember without searching."` The `SearchMemoryTool.toolDescription` already contains that sentence — it's just not in the preamble, where the cache-locked early-prompt nudge actually lives. Optionally wire `TurnContext.memoryHydration` in `AppDelegate.swift:1685` to call `HybridSearch.searchFacts(query: input.userText, k: 5, ...)` before each turn, but this needs orchestrator API plumbing — the named-tool fix is much cheaper and ships M-3 alone.
|
||||
|
||||
### M-4 (HIGH): `MemoryExtractionOrchestrator.process` still hashes `TurnID` and stores the hash in `facts.source_turn_id` — correlation is permanently broken
|
||||
|
||||
**Path:** `packages/Memory/Sources/Memory/MemoryExtractionOrchestrator.swift:122-152` (`process` body + `stableHash`)
|
||||
**Symptom:** After today's FK fix, fact INSERTs succeed, but `facts.source_turn_id` holds the FNV-1a hash of `TurnID.rawValue` (a UUID string), not a real `turns.id` rowid. Querying "what turn produced this fact?" can't be answered — `SELECT t.* FROM facts f JOIN turns t ON t.id = f.source_turn_id` returns nothing. The replay-trigger TurnID surfaced via the synthetic `memory-trigger-<hash>` string (see M-2) inherits the same hash. The schema's stale-comment-now-truth: "the writer's correlation tag and `turns.id` are disjoint number systems by design."
|
||||
**Root cause:** When the FK was dropped in `a5999e2`, the writer-side change to use a real `turns.id` rowid wasn't made — the FNV hash is a Phase-7 artifact that survived the fix. The orchestrator has `job.turnId: TurnID` (UUID string) and the turns table autoincrement `id INTEGER PRIMARY KEY` — but no lookup translates one to the other. The `B-02` patch (commit `48ad8d6`, `AppDelegate.swift:1753-1766`) writes turn rows BEFORE memory extraction enqueues, but doesn't return the rowid; even if it did, there's no plumbing to feed it back into the orchestrator.
|
||||
**Evidence:** Live DB `SELECT source_turn_id FROM facts;` → `7747877241524896942` (an FNV hash, easily verified by hashing today's TurnID). No row in `turns` has `id=7747877241524896942`. The schema comment at `MemorySchema.swift:35-50` openly acknowledges this — but acknowledgement isn't a fix. Anyone trying to debug "which turn caused this fact?" via SQL will get blank rows forever.
|
||||
**Recommended fix:** Two surgical options:
|
||||
1. Drop `source_turn_id` from the schema entirely if no consumer reads it (and document that fact-to-turn correlation is via `valid_from` timestamp proximity instead). Cleaner.
|
||||
2. Make `B-02`'s `appendTurn` return the new rowid, thread it through the `flushPair` callback (return a triple `(userText, assistantText, userTurnRowId)`), and have `MemoryExtractionCoordinator.start`'s `turnContent` closure return it; pass it as `sourceTurnId` through to `applyOp` instead of the FNV hash. Drop `stableHash` entirely.
|
||||
|
||||
Option 1 is a one-day fix; option 2 is half a day and preserves the correlation. The current state — silently storing nonsense — is the worst of both worlds.
|
||||
|
||||
### M-5 (HIGH): `MemoryQueries.hybridSearchSQL` passes raw user text directly to `facts_fts MATCH ?` — punctuation crashes the query
|
||||
|
||||
**Path:** `packages/Memory/Sources/Memory/MemoryQueries.swift:33-63` + `packages/Memory/Sources/Memory/HybridSearch.swift:32-57`
|
||||
**Symptom:** Real user queries like `"what is my dog's name?"`, `"remember when I said X?"`, or anything with an unescaped `?`, `:`, `-`, `"`, `*`, or FTS5-reserved operator throws `fts5: syntax error near "..."` at the SQLite layer, propagates as a thrown `MemoryError`, and the agent gets a tool-call error from `search_memory` instead of results. The replay shows zero `search_memory` calls today, so we have no live evidence yet — but the moment the agent does start calling it (per M-3 fix), every natural-language query with punctuation will fail.
|
||||
**Root cause:** `HybridSearch.searchFacts` (line 32-57) passes `query` straight through to `MemoryStore.runHybridSearchSQL` (line 311-339) which binds it to `MATCH ?` positional parameter. FTS5's MATCH syntax accepts a small grammar (terms, AND/OR/NOT/NEAR, column filters, phrase quoting with `"`); raw natural language with `?` or `'` characters is a syntax error.
|
||||
**Evidence:** Live DB probe: `sqlite3 jarvis.db "SELECT rowid FROM facts_fts WHERE facts_fts MATCH 'what is my dog name?' LIMIT 10"` → `Error: stepping, fts5: syntax error near "?"`. Same query without `?` works. No sanitization or escaping anywhere in `packages/Memory/Sources/` or `packages/MCP/Sources/MCP/InProcess/SearchMemoryTool.swift`.
|
||||
**Recommended fix:** Add an FTS5-safe query rewriter in `HybridSearch.searchFacts` before calling the store. Two-line approach for now: strip punctuation, split on whitespace, double-quote each term, join with spaces — turns `"what is my dog's name?"` into `"what" "is" "my" "dog's" "name"`. This is a phrase-OR query and is FTS5-safe. Long-term: an actual query AST. Until done, search_memory is a footgun the agent will inevitably step on.
|
||||
|
||||
### M-6 (MEDIUM): `MemoryBootHealthProbe` evidence is misleading after today's FK fix — `0/0 facts` is no longer a hidden-failure signal
|
||||
|
||||
**Path:** `App/Boot/BootHealthProbes.swift:32-55` (probe), `App/Boot/BootHealthProbes.swift:179-181` (stale comment about "the Toby case")
|
||||
**Symptom:** The probe still reports `state=ok evidence="N turns, 0/0 facts, vec=v0.1.6"` when memory has been written successfully (it just reports row counts at probe time). The 2026-05-12 user-burning bug was caught because the probe was happy with `0/0` while every insert failed; after the FK fix, `0/0 facts` is genuinely a legit empty state. The probe doesn't know the difference between "extractor never fired" and "extractor fired and got NOOP" and "extractor fired and FK-failed."
|
||||
**Root cause:** Probe asserts schema queryability + vec0 loadability + DB openness — three real things — but doesn't probe the writer end-to-end. After M-1 (no facts_vec writes) is fixed, a stronger probe would also assert `facts_vec` row count tracks `facts` row count within ±1. After M-2 is fixed, the probe could check `replay.sqlite` events table has recent rows.
|
||||
**Evidence:** Live probe output line `subsystem=memory state=ok evidence="0 turns, 0/0 facts, vec=v0.1.6, sqlite=3.51.0"` from 18:47:35. By 19:00 the user had submitted turns, but the probe ran once at boot — its evidence is stale by construction.
|
||||
**Recommended fix:** Two complementary improvements:
|
||||
1. Update the stale comment at `BootHealthProbes.swift:179-181` ("missing models = the Toby case. Embedding step fails, no row inserted in `facts`") — this is no longer accurate. After today's fix the embedding step (which doesn't exist in the writer per M-1) has nothing to do with whether `facts` gets a row. Rewrite as: "missing models = embedder fails → search degrades to FTS-only; extractor fails → no facts inserted at all."
|
||||
2. Add a parity check to `MemoryStats`: `factsTotal == factsVecCount ± tolerance`, surface as the probe's degraded path. This is the structural defense against M-1-style impedance bugs.
|
||||
|
||||
### M-7 (LOW): Stale doc-comment in `MemoryStore.swift:380` references "future ops/build plan" that already shipped
|
||||
|
||||
**Path:** `packages/Memory/Sources/Memory/MemoryStore.swift:464-468`
|
||||
**Symptom:** Cosmetic. Comment says "the prior `loadVecExtension(on:)` was retired 2026-05-11 (D-5/D-6 closure). vec0 is now statically linked..." This is fine. But other comments in the file still hedge with "Plan 07-02 wires", "Plan 07-03 will replace", "future ops plan" language that no longer reflects reality.
|
||||
**Recommended fix:** Comment scrub pass when the file is next touched. Not urgent.
|
||||
|
||||
### M-8 (LOW): `MemoryReplaySink.record` is a fire-and-forget detached Task — failures are unobservable from MemoryStore
|
||||
|
||||
**Path:** `App/AppDelegate.swift:2786-2797`
|
||||
**Symptom:** `recordMemoryMutation` calls `sink.record(...)` synchronously per protocol, and the sink dispatches into a detached `Task` (line 2793-2795). Any throw inside the task body (including the FK violation from M-2) is swallowed silently from MemoryStore's perspective. MemoryStore logs success of `applyOp` while the replay event vanishes downstream. Removing this swallow makes M-2 visible at the right layer.
|
||||
**Recommended fix:** The actor surface is sync, so a true terminal log is hard. But the Task can `do { await replayLog.record(...) } catch { logger.error("replay sink record failed: \(error)") }`. `record(_:for:)` on `ReplayLog` is currently non-throwing — events go into a pending buffer and the throw is at flush time. So this is also a structural observability gap on ReplayLog itself: writes happily succeed at the call site even when flush has been failing for 30 batches. The MemoryStore comment should at minimum acknowledge "best-effort with no terminal observability — failures appear only in replay.<date>.log."
|
||||
|
||||
## Verified healthy
|
||||
|
||||
- **vec0 linkage on cold start.** `core_vec_init()` runs in MemoryStore's `_vecAutoExtensionRegistered` static-let; `SELECT vec_version()` returns `v0.1.6` on every install. Logged at INFO from `memory` channel in `memory.2026-05-12.log` for all three of today's boots.
|
||||
- **MemoryStore.appendTurn → recentTurnsForSession (B-02 patch).** Live DB has 8 turn rows for today's session, all four user/assistant pairs intact, role/source/content columns populated, FTS5 trigger fired (1 row in `turns_fts_data`). `sessionHistoryLookup` in `AppDelegate.swift:1647-1669` reads the right session_id (matches `replay.sessions.session_id`). Multi-turn context threading is alive.
|
||||
- **`MemoryExtractionCoordinator.start` single-consumer invariant.** Coordinator's `subscription = Task.detached` guarded by `guard subscription == nil` — calling twice is a no-op. Drain task filters to `stop == .endTurn` only.
|
||||
- **`MemoryStore.forgetFact` idempotency.** `WHERE valid_to IS NULL AND forgotten_at IS NULL` makes second call a 0-row UPDATE; returns false the second time. Schema-clean.
|
||||
- **`MemoryStore.applyOp` supersede transaction.** `beginTransaction` / `commit` / explicit `rollback` on error; the supersede UPDATE asserts exactly 1 affected row. Pattern is correct.
|
||||
- **`OllamaEmbeddingClient` loopback enforcement + dimension check.** Tested behavior, not just declarative. 768-dim mismatch throws early.
|
||||
- **`installMemory` boot policy.** Hard-block on init failure (line 1374-1380) — no more silent dormancy from the D-1 era. Probe registers `DormantSubsystemProbe` only if the hard-block path was taken (which terminates the app anyway).
|
||||
- **Memory tools registration.** Live BootHealth: `10 tools (3 stdio + 7 in-process)` including `search_memory`, `forget_fact`, `get_memory_stats`. The tools are registered and dispatch-routable — only the discoverability cue is missing (M-3).
|
||||
- **Single-emit grep gates.** `scripts/check-single-memory-mutated-emit.sh` and `scripts/check-single-memory-used-emit.sh` both pass at HEAD. MemoryStore is the sole construction site for both `ReplayEvent.memoryMutation` and `ReplayEvent.memoryRetrieval`.
|
||||
|
||||
## Open questions
|
||||
|
||||
- **Was M-1 (no facts_vec writes) ever intended to ship?** The schema comment + `SearchMemoryTool.toolDescription` ("semantic (vec0)") strongly suggest yes. The orchestrator's `process(_:)` would be the natural place to call the embedder after `applyOp` returns the new fact. Likely a Phase-7 deferral that never landed — analogous to D-5/D-6 except no corresponding TODO line tracks it. Worth checking whether anyone noticed the gap during the 2026-05-04 audit (synthesis report mentions vec0 as a deferred environment problem, but doesn't mention the writer-side gap).
|
||||
- **Should `MemoryExtractor` ALSO call the embedder for `priorActiveFacts` rendering?** Probably not — the priorFacts go into the system prompt as text; vec only matters when searching. Out of scope unless M-1 fix surfaces it.
|
||||
- **Is the `OrchestratorEvent.memoryMutated` referenced in the audit prompt's scope §6 a planned event?** I couldn't find it in any current source. If it's planned for v1.0 migration, M-1/M-2 should be addressed before the bus forwarding lands — otherwise the HUD card will be wired to a dead pipeline.
|
||||
- **M-2 mitigation choice.** The synthetic-turn-id approach is a Phase-7 design (the trigger turn for memory mutation is real, but the mutation can in principle span multiple turns). Real-turn-id threading is cleaner but adds API surface. Sentinel turn rows are the smallest fix. User preference?
|
||||
- **Does `MemoryExtractor` ever actually fire today?** Replay events show 19 `tool_call_requested` rows but extractor calls aren't on the same code path (extractor uses local Ollama, not the agent loop's replay log). Live `memory.2026-05-12.log` doesn't show extractor-issued log lines (the extractor only logs the failure modes — "no apply_memory_ops tool call"). If the extractor is firing but the qwen3.6:latest model isn't returning the tool call, we have no signal. **Recommended:** add INFO logging to `MemoryExtractor.extract` on the success path so the orchestrator's drain progress is observable. Today's "1 fact persisted" implies it fired at least once, but the orchestrator log only shows the pre-fix failures.
|
||||
@@ -0,0 +1,126 @@
|
||||
# Vision Subsystem — Audit (2026-05-12)
|
||||
|
||||
**Audit head:** `a5999e2` (develop). **Prior baseline:** `.planning/audit-2026-05-03/vision-audit.md` (Grade F).
|
||||
**Calibration bug pattern this pass:** silent integration-boundary failures masked by probes returning ok.
|
||||
|
||||
## TL;DR — Grade: **B-**
|
||||
|
||||
The big "wired but dead" claims from 2026-05-03 are all closed (Track C-1..C-6). End-to-end on real hardware the camera produces multi-KB JPEGs (`>1000 bytes`, valid SOI), `frameStream` fan-out yields real `CMSampleBuffer` samples within 2 s, the HUD `CameraButton` is rendered and posts `frameAttachRequested`, the chat-submit handlers consume the pending frame, and the assistant-turn-complete release subscriber is wired. The remaining defects are subtler:
|
||||
|
||||
1. **HIGH — `t2AvailableForThisTurn` lies to the router.** `AgentOrchestrator.runTurn` (line 321) sets `t2AvailableForThisTurn = (decision.tier != .t3Cloud)` — i.e. `true` for every local route. The router then escalates low-confidence T1 responses to T2, where T2 is `MissingT2Provider` and throws `t2ProviderUnavailable`. The whole vision turn dies with a provider error instead of staying on T1. Calibration shape: the comment at line 317-320 explicitly says "evaluatePostResponse correctly stays-on-T1 when t2Available == false" — but production passes `true`. No AgentCore test exercises image+escalation against the production wiring; the Vision-package router tests pass `t2Available: false` explicitly, so the regression is invisible.
|
||||
2. **MEDIUM — pendingFrame leaks on `.rejected` submit.** `FrameAttachController.confirmSend(...)` returns an `ImageBlock` but does NOT clear `pendingFrame`. Per design, the slot is released by `onAssistantTurnComplete()` driven by the broadcaster's `.turnEnd` watcher. If `AgentOrchestrator.submit(...)` returns `.rejected`, no `.turnEnd` fires, no release runs, and the JPEG bytes sit in actor memory until the next `requestAttach` overwrites the slot. Privacy regression for D-15 byte-release invariant.
|
||||
3. **MEDIUM — Mid-session TCC revocation not robustly detected.** `CameraCapture.startWatchers()` only observes `AVCaptureSessionRuntimeError`. macOS Sequoia/Tahoe weekly TCC reprompt + Settings-side revocation can surface as a session interruption (`AVCaptureSessionWasInterrupted`) without firing a runtime error. There is no periodic `AVCaptureDevice.authorizationStatus(for: .video)` re-check, no `AVCaptureSessionWasInterrupted` observer, and no `becameAuthorized()` callsite from AppDelegate. The `cameraRevoked` banner watcher is alive but the producing signal may never reach it.
|
||||
|
||||
Verified healthy: real-hardware JPEG capture, frameStream fan-out, presence monitor face-detection pipeline, HUD camera button JS wiring, T2-explicit-error provider, VISION-03 isolation boundary, 72/72 SPM tests (incl. real-hardware suite when JARVIS_REAL_CAMERA=1).
|
||||
|
||||
---
|
||||
|
||||
## Findings (severity-ordered)
|
||||
|
||||
### F-V1 — HIGH — Orchestrator passes `t2Available: true` when only `MissingT2Provider` is wired
|
||||
|
||||
**Locations:**
|
||||
- `packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift:321` — `t2AvailableForThisTurn = (decision.tier != .t3Cloud)`
|
||||
- `packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift:670-689` — escalation branch in `runTurnLoop`
|
||||
- `App/AppDelegate.swift:2319` — `t2Provider: MissingT2Provider()`
|
||||
- `packages/Vision/Sources/Vision/MissingT2Provider.swift:36,50` — `continuation.finish(throwing: VisionError.t2ProviderUnavailable)`
|
||||
|
||||
**Shape:** Calibration-bug pattern — probe (the router heuristic + tests) returns ok, but reality (production provider wiring) is broken. Two layers of test coverage *both* miss it because:
|
||||
- `VisionRouterEscalationTests` passes `t2Available: false` for the unavailable case and `t2Available: true` only against a stub `t2Provider` (`MockProvider`/inline).
|
||||
- `AgentOrchestrator` has zero tests that combine `input.images.nonEmpty` + a low-confidence T1 response + `MissingT2Provider` wired as t2.
|
||||
|
||||
**Repro path:** Camera frame attached → T1 (`OllamaProvider` qwen2.5-coder over the local Ollama) returns "I'm not sure" or any response under 24 chars / containing one of the substrings in `VisionRouterConfig.default.lowConfidenceSubstrings`. The escalation block at line 668-690 runs because `t2AvailableForThisTurn = true`, swaps `currentProvider` to `MissingT2Provider`, the next `outer` iteration calls `currentProvider.stream(...)`, the stream `finish(throwing:)` propagates to the `catch` at line 820, and the user sees an error event for what should have been a T1 answer.
|
||||
|
||||
**Fix:** Either (a) pass the actual provider availability — feature-flag or runtime probe — to `evaluatePostResponse(t2Available:)` so it stays on T1 when `MissingT2Provider` is the t2 wiring, or (b) move the "is T2 a real provider" check into the router itself (`VisionRouter` already holds the providers; `evaluatePostResponse` could ask `t2Provider is MissingT2Provider`). Add a regression test in AgentOrchestratorTests that wires `MissingT2Provider` as t2, feeds a low-confidence T1 mock, and asserts the turn finishes with `.turnEnd(... stopReason: .endTurn)` instead of `.error(...)`.
|
||||
|
||||
---
|
||||
|
||||
### F-V2 — MEDIUM — `confirmSend` leaves `pendingFrame` populated on rejected submit
|
||||
|
||||
**Locations:**
|
||||
- `packages/Vision/Sources/Vision/FrameAttachController.swift:104-112` — `confirmSend(userText:)` does not call `discardFrame()`
|
||||
- `App/AppDelegate.swift:1827-1839` — release watcher only fires on `.turnEnd` events with `turnHadImage == true`
|
||||
- `App/AppDelegate.swift:2160-2164` — `consumePendingFrameIfAny` produces the block but doesn't release the slot
|
||||
|
||||
**Shape:** D-15 sole-emission-site invariant is intact (every release goes through `discardFrame()`), but there is no release on the `.rejected` outcome path. Sequence:
|
||||
1. User clicks Camera → `requestAttach(.hudButton)` → `pendingFrame = frame`.
|
||||
2. User types and presses Send → `confirmSend(userText:)` returns the `ImageBlock` (slot still populated by design).
|
||||
3. `AgentOrchestrator.submit(...)` returns `.rejected(reason: .providerUnavailable)` (or `.maxToolCalls`, or any non-running outcome).
|
||||
4. No `.turnEnd` for this turn ever flows through the broadcaster's frame-attach subscriber → `onAssistantTurnComplete()` never called → JPEG bytes persist in `FrameAttachController.pendingFrame` until the next `requestAttach` overwrites the slot or app shutdown.
|
||||
|
||||
`FrameAttachControllerTests.testConfirmSendKeepsPendingUntilAssistantTurnComplete` documents the design (line 110-117), but no test exercises the `.rejected` outcome path. Privacy regression: the byte-release window can grow unbounded.
|
||||
|
||||
**Fix:** Either (a) add a `discardFrame()` from `AppDelegate.handleChatSubmit` / `handleChatCancelAndSubmit` when `outcome` is `.rejected` (single new line per handler — call `frameAttachController?.onAssistantTurnComplete()` after `handleTextOutcome` if outcome is rejected), or (b) add a `releaseAfterRejectedSubmit()` method to `FrameAttachController` and route through it explicitly. The second keeps the sole-emission-site invariant readable. Add a test that wires a stub `AgentOrchestrator` returning `.rejected`, confirms `hasPendingFrame == false` afterward.
|
||||
|
||||
---
|
||||
|
||||
### F-V3 — MEDIUM — Mid-session TCC revocation has no observer that fires on TCC revoke alone
|
||||
|
||||
**Locations:**
|
||||
- `packages/Vision/Sources/Vision/CameraCapture.swift:220-240` — only `AVCaptureSessionRuntimeError` observed
|
||||
- `App/AppDelegate.swift:2226-2235` — `cameraDegradationTask` reads `.cameraRevoked` from degradation stream; alive but trigger may not fire
|
||||
- `packages/Vision/Sources/Vision/CameraCapture.swift:94-96` — `becameAuthorized()` is declared but never called from production (zero matches outside its own definition)
|
||||
|
||||
**Shape:** macOS Sequoia and 26 Tahoe re-prompt for Camera/Microphone TCC on a rolling basis. When the user revokes via System Settings without quitting Jarvis, the AVCaptureSession typically posts `AVCaptureSessionWasInterruptedNotification` (with `AVCaptureSessionInterruptionReasonKey == .videoDeviceNotAvailableInBackground` or `.videoDeviceNotAvailableWithMultipleForegroundApps`). It does NOT reliably post `AVCaptureSessionRuntimeError`. The current observer set therefore misses TCC-revoke as a degradation source. `cameraDegradationTask` will sit waiting for a yield that never comes.
|
||||
|
||||
Compounding factor: the install-time camera-status check at `AppDelegate.swift:2244-2256` runs once at boot; `becameAuthorized()` is declared in `CameraCapture` for the post-grant re-open path but has zero non-doc callsites in the tree (`grep -rn 'becameAuthorized' --include='*.swift'` outside its own declaration returns 0). After first-launch denial + later Settings grant, the camera session never reopens.
|
||||
|
||||
**Fix:** (a) Add an `AVCaptureSessionWasInterrupted` observer in `startWatchers()` and yield `.midSessionRevoked` when the interruption reason indicates device-unavailable or TCC-revoked. (b) Wire a periodic (e.g. on app foreground) `AVCaptureDevice.authorizationStatus(for: .video)` re-check in AppDelegate that triggers `becameAuthorized()` when the status flips `.notDetermined → .authorized`. (c) Add a unit test that injects a synthetic interruption notification and asserts the degradation continuation yields `.midSessionRevoked`.
|
||||
|
||||
---
|
||||
|
||||
### F-V4 — LOW — Continuation race possible when `frameStream(forPresence:)` is called before `open()`
|
||||
|
||||
**Locations:**
|
||||
- `packages/Vision/Sources/Vision/CameraCapture.swift:153-184` — `frameStream` hops into actor, finds `videoSampleDelegate == nil`, calls `cont.finish()`.
|
||||
|
||||
**Shape:** Not a current bug (AppDelegate always calls `open()` before `frameStream(forPresence:)`), but the order is structural: any future re-arrangement that subscribes before opening will silently produce a finished stream and PresenceMonitor will exit its feed loop forever. The `attachFrameContinuation` path that calls `cont.finish()` when the delegate is absent has no signal back to the caller — PresenceMonitor cannot distinguish "stream finished naturally" from "stream finished because session not yet built."
|
||||
|
||||
**Fix:** Make this contract enforceable — either (a) hold pending continuations in a separate list inside `CameraCapture` and flush them onto the delegate when `buildSession()` completes, or (b) throw from `frameStream(forPresence:)` when `session == nil` (changes the API from `AsyncStream` to `throws -> AsyncStream`). Minimum: add a structural test that fails if `frameStream(forPresence:)` is called before `open()` and silently returns a finished stream (the existing `testFrameStreamFinishesImmediatelyWhenNotOpen` documents this as desired behavior — flip it to a stricter contract).
|
||||
|
||||
---
|
||||
|
||||
### F-V5 — LOW — `t2AvailableForThisTurn` comment is now wrong
|
||||
|
||||
**Location:** `packages/AgentCore/Sources/AgentOrchestrator/AgentOrchestrator.swift:317-321`
|
||||
|
||||
```
|
||||
// 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)
|
||||
```
|
||||
|
||||
The "D-04 ships with t2Provider == t1Provider" premise was the pre-Track-C-4 wiring. After Track-C-4, t2 is `MissingT2Provider`. The comment now misleads any reader into thinking the silent T1-as-T2 fallback is the live behavior, hiding F-V1. Fix as part of F-V1.
|
||||
|
||||
---
|
||||
|
||||
## Verified healthy
|
||||
|
||||
- **CameraCapture real-hardware JPEG** — `JARVIS_REAL_CAMERA=1 swift test --filter CameraCaptureRealHardwareTests` (run during audit, exit 0). Real frames are >1000 B with valid SOI marker (`Data([0xFF, 0xD8])`) at 1280×720; `frameStream(forPresence:)` yields a `.sampleBuffer(...)` within 2 s of session start.
|
||||
- **`PhotoCaptureProxy` retain cycle** — proxy retains self in init, breaks cycle on first delegate callback (`packages/Vision/Sources/Vision/CameraCapture.swift:270-313`). Verified by structural test `testCaptureFrameSourceDoesNotEmbedHardcodedJPEG` and by real-hardware test exit 0.
|
||||
- **`VideoSampleDelegate` fan-out** — UUID-keyed continuation map, NSLock-guarded snapshot under `captureOutput(_:didOutput:from:)`, `finishAllStreams()` snapshot-then-clear pattern is race-free. Verified by `CameraCaptureFrameStreamTests` (testFrameStreamYieldsInjectedSamplesAfterOpen + testShutdownFinishesOutstandingStreams) and by `CameraCaptureRealHardwareTests`.
|
||||
- **PresenceMonitor pipeline** — face-detection request runs against real `CMSampleBuffer` from frameStream; the debounced state machine fires `.transition(to:at:debounced:)` to `PresenceSignalBus`. Verified by `PresenceMonitorDebounceTests` (synthetic) and by code inspection of `runVision(on:now:)` (lines 176-186) — Vision-framework `VNDetectFaceRectanglesRequest` is real, run synchronously, results checked, observation propagated via `applyObservation(...)`.
|
||||
- **HUD camera button wiring (B-03 carry-forward — not re-flagged)** — `CameraButton.tsx` posts `BusInbound.frameAttachRequested`; `ChatInput.tsx:71` renders it; `AppDelegate.swift:2520-2522` routes the inbound to `frameAttachController.requestAttach(reason: .hudButton)`. Wiring intact. The missing piece is user-visible UI confirm/preview state, which is the M-5 carry-forward.
|
||||
- **`FrameAttachController.confirmSend` non-test callers** — `AppDelegate.consumePendingFrameIfAny` (line 2160-2164) is the production caller, reached from both `handleChatSubmit` (line 2121) and `handleChatCancelAndSubmit` (line 2146). Verified by `FrameAttachConfirmSendWiringTests` greps.
|
||||
- **Frame-attach release subscriber** — `AppDelegate.swift:1827-1839` subscribes at `.frameAttach` priority and calls `frameAttachController.onAssistantTurnComplete()` when `agentOrchestrator.turnHadImage(turnId)` returns true. Priority isolation per `OrchestratorEventBroadcaster` protection matrix.
|
||||
- **`MissingT2Provider` explicit-error path** — `stream(...)` overloads both call `continuation.finish(throwing: VisionError.t2ProviderUnavailable)`. Verified by `MissingT2ProviderTests`. Surfaces as a debuggable error rather than silent T1-rerun. (F-V1 above is about the orchestrator side mis-gating the call — not about this provider's correctness.)
|
||||
- **TCC denied banner watcher** — `cameraDegradationTask` (AppDelegate.swift:2226-2235) is a long-lived `Task { @MainActor [weak self] in for await reason in degStream { ... } }`. Reads `.cameraDenied` and `.midSessionRevoked`; routes to `bannerCoordinator.enqueue(...)`. Task is created and never explicitly cancelled before `applicationWillTerminate`. Verified by code inspection — the producer-side signal sources (CameraCapture.open() denial path; AVCaptureSessionRuntimeError) are documented above; F-V3 covers the missing trigger paths.
|
||||
- **VISION-03 isolation invariant** — `bash scripts/check-presence-vision-isolation.sh && bash scripts/check-vision-isolation.sh` — both exit 0 at HEAD. `packages/Vision/Package.swift` does not declare a dep on `Voice` or `AgentOrchestrator`; no Vision source imports them.
|
||||
- **Vision SPM test suite** — `swift test --package-path packages/Vision` — 72 tests, 0 failures, 2 skipped (real-hardware suite when `JARVIS_REAL_CAMERA` unset). Real-hardware suite passes when env-gate is set.
|
||||
|
||||
---
|
||||
|
||||
## Open questions
|
||||
|
||||
1. **Should `t2AvailableForThisTurn` be derived in the router instead of the orchestrator?** The router holds the providers, knows `t2Provider is MissingT2Provider`, and can fold that into `evaluatePostResponse`. Moving it eliminates the calibration-bug class entirely — no caller can pass the wrong flag because there is no flag. Cost: router gains a tiny bit of awareness about provider identity (currently it's pure dispatch).
|
||||
2. **What's the user-facing confirmation UX for the HUD camera button?** Today the click silently arms the slot for 2 s; if the user doesn't type+send in that window, the frame is discarded with no UI feedback. The M-5 carry-forward presumably scopes this; flagging here so the audit→backlog handoff has the dead-air symptom recorded alongside the wiring-is-intact verification.
|
||||
3. **`AVCaptureSessionWasInterrupted` vs runtime error** — empirical confirmation needed of which notification(s) fire when (a) user revokes Camera TCC via Settings while app is foreground, (b) user toggles Camera off then on, (c) USB camera unplug/replug. The fix for F-V3 should be guided by which signals are reliable in macOS 26 Tahoe specifically, not by what Apple's docs promised in macOS 12.
|
||||
4. **Why is `becameAuthorized()` declared with no production callsite?** Either it's leftover scaffolding for a re-grant flow that was never wired (delete it and document), or the re-grant flow IS desired and the wiring just never landed (add it and reference F-V3). Picking either resolves a small chunk of dead-code.
|
||||
|
||||
---
|
||||
|
||||
## Summary (top-3, ~150 words)
|
||||
|
||||
The "scaffold simulating a feature" verdict from 2026-05-03 is closed: Track C-1..C-6 delivered real AVCapturePhotoOutput delegate capture (real-hardware test passes with >1000-byte SOI-valid JPEGs), real `VideoSampleDelegate` fan-out feeding the presence pipeline, a wired `CameraButton.tsx`, a non-orphaned `confirmSend`, and an explicit `MissingT2Provider` instead of the silent T1-as-T2 fallback. What remains is subtler. **(1) HIGH:** `AgentOrchestrator` passes `t2Available: true` on every local-tier vision turn, so a low-confidence T1 answer escalates straight into `MissingT2Provider` and the turn errors out — a calibration-bug pattern (probe says ok, production wiring breaks). **(2) MED:** `confirmSend` doesn't release `pendingFrame` on `.rejected` submit, leaving JPEG bytes resident past the D-15 release window. **(3) MED:** mid-session TCC revoke isn't observed reliably — only `AVCaptureSessionRuntimeError`, not `AVCaptureSessionWasInterrupted`, and no foreground re-check.
|
||||
@@ -0,0 +1,407 @@
|
||||
# Voice Subsystem Audit — 2026-05-12
|
||||
|
||||
Read-only audit, HEAD `a5999e2`. Live host PID 18811 is running with
|
||||
`installVoice: VoiceController started` in `system.2026-05-12.log`; voice
|
||||
BootHealthProbe reports `ok` (variant=aecOn, 16 kHz Float32). Audit traced
|
||||
the canonical wake → STT → orchestrator → TTS → HUD flow line-by-line and
|
||||
diffed it against the 2026-05-03 / 2026-05-04 findings.
|
||||
|
||||
The big lifts from prior audits are real: models bundle into
|
||||
`Contents/Resources/Models/{openWakeWord,silero}/` (verified in
|
||||
`build/Build/Products/Debug/Jarvis.app/...`), `LiveSpeechAnalyzerBridge` is
|
||||
no longer a `_ = chunk` stub, `TTSEngineActor` is constructed in production
|
||||
via `VoiceOutputWiring.makeTier1Engine()`, `wakeWordDAG.start(ring:)` is
|
||||
wired against a real ring, `ProductionChunkPump` subscribes via
|
||||
`AudioGraphOwner.subscribe()` (no SPSC sample-stealing), and
|
||||
`AudioLevelEmitter` is constructed in `installVoice` against its own
|
||||
broadcaster subscription.
|
||||
|
||||
But three new "wired but dead" bugs and several integration mismatches
|
||||
remain — concentrated at the rebuild boundary, the STT backend factory, and
|
||||
the TTS event stream. Voice on a cold-launched, freshly-permitted machine
|
||||
**does** work for one wake-word turn. Voice **does not** survive any audio
|
||||
graph rebuild (device change, mic re-grant, ring overflow). And the
|
||||
ducking/HUD signal the rest of the system depends on is structurally
|
||||
mismatched against the only place it ever fires.
|
||||
|
||||
---
|
||||
|
||||
## P0 — Critical (will silently break voice in production)
|
||||
|
||||
### P0-1. `cancelInFlight` permanently kills wake-word on every audio-graph rebuild
|
||||
|
||||
`App/AppDelegate.swift:1192–1194`:
|
||||
|
||||
```swift
|
||||
await graphOwner.setCancelInFlight { [wakeWordDAG] in
|
||||
await wakeWordDAG.cancel()
|
||||
}
|
||||
```
|
||||
|
||||
`WakeWordDAG.cancel()` (`packages/Voice/.../WakeWordDAG.swift:137–142`) calls
|
||||
`streamCont.finish()` — the public `wakeWordStream` is now closed forever.
|
||||
`VoiceController.spawnWakeWordConsumer`'s `for await event in stream` exits.
|
||||
`AudioGraphOwner.teardown` (`AudioGraphOwner.swift:265–290`) invokes
|
||||
`cancelInFlight` on **every** rebuild trigger: `.deviceChange`,
|
||||
`.micRegrant`, `.ringOverflow`, `.aecFallback`. The new graph builds with a
|
||||
fresh ring, but nothing re-arms the DAG, and the consumer Task has already
|
||||
exited so even calling `start(ring:)` again wouldn't drive listeners that
|
||||
have already finished the for-loop.
|
||||
|
||||
The DAG file documents the correct contract at line 144–162: a separate
|
||||
`stopFeed()` method exists *specifically* so `cancelInFlight` can stop the
|
||||
feed Task **without** finishing the stream, with a literal docstring
|
||||
warning: *"Previously `cancelInFlight` called `cancel()` which finished the
|
||||
stream — combined with the `lastMicStatus = false` startup bug in
|
||||
`AudioGraphOwner.startMicRegrantWatcher`, the wake path died ~2 s after
|
||||
launch on every cold start."* The mic-regrant startup bug was fixed
|
||||
(`AudioGraphOwner.swift:340–354` primes `lastMicStatus`); the AppDelegate
|
||||
wiring of `stopFeed()` was not. The fix is one line.
|
||||
|
||||
**Plus** the rebuild-completion consumer at `AppDelegate.swift:1125–1133`
|
||||
only dismisses the AEC banner; it never calls
|
||||
`wakeWordDAG.start(ring: newRing)`. So even after switching to `stopFeed()`,
|
||||
the DAG needs to be re-armed against the new ring inside the rebuild
|
||||
consumer.
|
||||
|
||||
Detection on the live host is hard because the boot probe only checks
|
||||
`variant != nil` (see P1-2). Symptom is "wake word stopped working after
|
||||
plugging in headphones" — exactly the canonical "wired but dead" pattern.
|
||||
|
||||
### P0-2. STT backend hard-coded; ignores `PerTurnSnapshot.stt`
|
||||
|
||||
`App/AppDelegate.swift:1272`:
|
||||
|
||||
```swift
|
||||
sttFactory: { STTBackendSelector.make(backend: "speech_analyzer") },
|
||||
```
|
||||
|
||||
The closure ignores `configStore` entirely. Setting
|
||||
`stt.whisperKitFallback = true` in config changes nothing for voice — the
|
||||
factory always builds `SpeechAnalyzerSTT`.
|
||||
|
||||
To make it worse, the `get_self_state` tool dispatcher *does* read from
|
||||
config (`AppDelegate.swift:2025–2028`):
|
||||
|
||||
```swift
|
||||
return snap.stt.whisperKitFallback ? "whisperKit" : "speechAnalyzer"
|
||||
```
|
||||
|
||||
…and those strings are also **wrong** for `STTBackendSelector.make`. The
|
||||
selector accepts `"speech_analyzer"` / `"whisperkit"` (snake_case, lower);
|
||||
the closure returns `"whisperKit"` / `"speechAnalyzer"` (camelCase, capital
|
||||
W/A). If anyone ever wires the snapshot.stt.backend through, every value
|
||||
hits the `default` case and falls back to SpeechAnalyzer. Two unrelated
|
||||
bugs — name mismatch + hard-coded factory — both pointing the same
|
||||
direction (WhisperKit is unreachable).
|
||||
|
||||
Combined with P0-3 (asset-missing maps to STTError), this means: if
|
||||
SpeechAnalyzer asset download fails on a fresh-install Release, the user
|
||||
cannot fall back to WhisperKit via config — they have to ship a new build.
|
||||
|
||||
### P0-3. `.ttsStopped` is emitted ONLY on barge-in; normal completion never releases ducking
|
||||
|
||||
`TTSEngineActor.swift:91–178` runs `synthesize(_:tier:voice:)` and yields
|
||||
`.started`, `.firstAudio(at:)`, `.finished` — never `.ttsStopped`. The class
|
||||
header (lines 21–23) and `TTSEvent.swift:7–12` document the canonical
|
||||
sequence as `started → firstAudio → ... → finished → ttsStopped`, and the
|
||||
`TTSInterrupt.swift:97` grep gate enforces `wc -l == 1` for `.ttsStopped`
|
||||
emission sites *across the whole TTS package*. The one site that emits it
|
||||
is `InterruptSequence.run` — i.e. **only when barge-in fires**.
|
||||
|
||||
Per the file-level comment "Ducking releases ONLY on `.ttsStopped`" (and
|
||||
the I4 test `TTSInterruptTests.swift:181` which validates the release path
|
||||
against `.ttsStopped`), any future consumer of `ttsEventStream` that
|
||||
implements ducking will leave the input duck engaged forever after a normal
|
||||
spoken reply. There is no ducking consumer wired in production yet
|
||||
(`grep -rn ttsEventStream App/ packages/.../Sources/` only finds the
|
||||
`TTSEngineActor` definition site itself), so this isn't biting anyone
|
||||
today — but the design contract is broken and the first production
|
||||
consumer of `ttsEventStream` will inherit a silent bug. Either
|
||||
`TTSEngineActor.synthesize` needs to emit `.ttsStopped` on natural
|
||||
completion (in addition to `.finished`), or the contract docs need to be
|
||||
rewritten to say "ducking releases on `.finished` for natural completion
|
||||
and `.ttsStopped` for barge-in" — and the InterruptSequence comment
|
||||
loosened from "ONLY site that emits".
|
||||
|
||||
`AVSpeechSmokeTests` / `TTSEngineActorTier1Tests.swift:37` filter the event
|
||||
stream for `.ttsStopped` while breaking out of the for-loop. If the test
|
||||
ran a real synthesis it would hang, but the suite passes because it uses
|
||||
short test strings and the test runner aborts before timeout. (The
|
||||
`OrpheusSerializationTests` use `ScriptedSpeechModel` which is a separate
|
||||
event source.) The contract / test pairing here is exactly the
|
||||
"impedance mismatch hidden by mocks" shape from the canonical example.
|
||||
|
||||
### P0-4. `VoiceTTSAdapter.tierResolver` is never wired; `tts.tier` config is dead
|
||||
|
||||
`App/AppDelegate.swift:1236`:
|
||||
|
||||
```swift
|
||||
let ttsAdapter = VoiceTTSAdapter(engine: VoiceOutputWiring.makeTier1Engine())
|
||||
```
|
||||
|
||||
`VoiceTTSAdapter.init` (`App/Voice/VoiceTTSAdapter.swift:29–35`) accepts
|
||||
`tierResolver: @escaping @Sendable () async -> TTSTier = { .tier1 }` but
|
||||
AppDelegate passes only `engine:`. So the resolver is the default constant
|
||||
`{ .tier1 }`. Setting `tts.tier = "tier2"` in config does nothing for the
|
||||
real synthesis path — only for the `get_self_state` tool, which reads
|
||||
`snap.tts.tier` directly via a separate closure (line 2020–2024).
|
||||
|
||||
Today this hides P0-5 (no Orpheus shipped anyway, so tier-1 is correct).
|
||||
The moment Orpheus weights ship, the user can't enable tier-2 via config.
|
||||
|
||||
---
|
||||
|
||||
## P1 — High (real correctness gaps, not yet user-fatal)
|
||||
|
||||
### P1-1. `OpenWakeWordSession.melFrameBuffer` and `embeddingBuffer` grow without bound
|
||||
|
||||
`packages/Voice/.../OpenWakeWordSession.swift:175–201`. Streaming path
|
||||
appends mel frames into `melFrameBuffer` on every `feed(samples:)` and
|
||||
advances `nextEmbStart` by 8 frames at a time. Mel frames are never popped
|
||||
or trimmed; `embeddingBuffer.append(emb)` (line 191) is also append-only.
|
||||
The classifier only reads the most recent 16 embeddings (`suffix(16)`),
|
||||
but the rest of the buffer is retained forever.
|
||||
|
||||
At one mel frame per `feed` call (80 ms @ 16 kHz, the production cadence),
|
||||
24 hours of always-on listening accumulates roughly:
|
||||
|
||||
- ~1.08M mel frames × 32 floats × 4 B ≈ 130 MB live retained
|
||||
- ~135k embeddings × 96 × 4 B ≈ 50 MB
|
||||
|
||||
Plus the `[[Float]]` Swift Array-of-Array overhead. The system survives
|
||||
overnight because `WakeWordDAG` cancels and rebuilds on every audio-graph
|
||||
rebuild — *if you fix P0-1*. Today the leak is masked by P0-1: the DAG
|
||||
dies before it can leak. Fix one without the other and memory grows
|
||||
unbounded. Recommend trimming `melFrameBuffer` and `embeddingBuffer` on a
|
||||
rolling-window basis (keep last `76 + embStride` mel frames; keep last 16
|
||||
embeddings).
|
||||
|
||||
### P1-2. `VoiceBootHealthProbe` doesn't probe anything voice-specific
|
||||
|
||||
`App/Boot/BootHealthProbes.swift:213–266`. Status `.ok` only requires:
|
||||
1. `audioGraphOwner` non-nil
|
||||
2. `micStatus == .authorized`
|
||||
3. `currentVariant != nil`
|
||||
|
||||
It does **not** verify:
|
||||
- the wake-word DAG is actually feeding (P0-1 leaves the probe reporting
|
||||
`ok` even after the wake path is dead)
|
||||
- SpeechAnalyzer assets are downloaded (`SFSpeechErrorCode.assetUnavailable`
|
||||
doesn't surface until the first `transcribe()` call)
|
||||
- WhisperKit fallback model weights exist (only matters once P0-2 is fixed)
|
||||
- TTS engine is alive (Tier-1 AVSpeechSynth always claims success even
|
||||
on a denied audio output route)
|
||||
|
||||
Hence the live log shows `voice state=ok` every probe interval despite
|
||||
P0-3 / P0-4 being latent. Recommend: cheap watchdog that asserts the
|
||||
wakeWordTask is still alive *and* the wakeWordDAG's feed Task isn't nil.
|
||||
|
||||
### P1-3. `VoiceController.vadFactory` returns same `SileroVAD` every session; never `reset()`
|
||||
|
||||
`App/AppDelegate.swift:1271`: `vadFactory: { sileroVAD }` captures the
|
||||
single instance constructed at line 1080. `SileroVAD.reset()` exists
|
||||
(`packages/Voice/.../SileroVAD.swift:292`) and zeroes the LSTM hidden
|
||||
state `h, c` plus the `wasSpeech` flag — but `VoiceController` never
|
||||
calls it. `startSTTSession` (line 380 of VoiceController) creates a fresh
|
||||
chunk stream and pump but reuses the same VAD instance.
|
||||
|
||||
Effect: LSTM hidden state from the end of session N persists into the
|
||||
start of session N+1. After session N's `.speechEnd`, the LSTM state
|
||||
encodes "we just exited speech"; the first 1–3 windows of session N+1
|
||||
inherit that bias and may misclassify quiet onset as `.silence`. Worst
|
||||
case: a soft-spoken first syllable goes undetected and the user's
|
||||
opening word is dropped from the STT input.
|
||||
|
||||
This is hard to detect in tests because most tests construct a fresh
|
||||
VAD per case. Recommend: call `await vad.reset()` at the top of
|
||||
`startSTTSession` (one line; VAD is constructed lazily-per-call via the
|
||||
factory, but the closure pattern caches the single instance, so reset is
|
||||
the right idempotency primitive).
|
||||
|
||||
### P1-4. `voiceHudCont.finish()` on `shutdown()` is a one-way ratchet
|
||||
|
||||
`packages/Voice/.../VoiceController.swift:181` finishes the HUD continuation.
|
||||
The continuation flows into `HudStateCoordinator` (constructed at
|
||||
`AppDelegate.swift:2475`). After `voiceController.shutdown()`, the
|
||||
coordinator's voice subscriber Task exits its for-loop and the HUD
|
||||
permanently ignores future voice intents — even if voice is reinstalled.
|
||||
|
||||
Today `shutdown()` is called from one site (`applicationWillTerminate`,
|
||||
`AppDelegate.swift:993` if memory serves), so this doesn't bite in normal
|
||||
exits. But the install path's failure branches (line 1163, 1174–1177,
|
||||
1219) **don't** call `shutdown` — they `return` early after constructing
|
||||
some subset of state. If we ever add a `reinstall on resume from sleep`
|
||||
path, finishing the cont on shutdown means voice can't come back without
|
||||
restarting HudStateCoordinator. Recommend: don't finish the cont in
|
||||
`shutdown`; the actor's drop already releases its end of the stream.
|
||||
|
||||
### P1-5. AgentHudIntent stream is dormant — HUD has no signal during thinking/speaking
|
||||
|
||||
`AppDelegate.swift:2487–2490` constructs the AgentHudIntent stream and
|
||||
stores `dormantAgentContinuation`, but **no production code ever yields
|
||||
into it**. `grep -rn dormantAgentContinuation App/` returns only the
|
||||
declaration and the assignment.
|
||||
|
||||
Combined with `VoiceController.hudIntentFor` (lines 626–632), which maps
|
||||
`.thinking` / `.speaking` / `.idle` all to `.silent`, the HUD ring sees:
|
||||
- `.listening` when wake-word fires (real signal)
|
||||
- `.reconfiguring` during audio graph rebuilds (real signal)
|
||||
- `.silent` everywhere else (thinking and speaking look the same as idle)
|
||||
|
||||
The HUD ring doesn't change shape during agent thinking or TTS playback.
|
||||
This is cross-cutting with the HUD audit, but the voice surface owns one
|
||||
end of it: `VoiceController` could emit a distinguishing `.speaking` /
|
||||
`.thinking` intent (extends VoiceHudIntent enum), or the agent install
|
||||
path could pump AgentHudIntent values from the orchestrator events
|
||||
broadcaster. Either fix is fine; neither exists.
|
||||
|
||||
---
|
||||
|
||||
## P2 — Medium (correctness drift, hygiene, lurking)
|
||||
|
||||
### P2-1. `STTBackendSelector` string mismatch is a documentation lie
|
||||
|
||||
`STTBackendSelector.backendSpeechAnalyzer = "speech_analyzer"` (snake_case)
|
||||
vs `AppDelegate.swift:2028`'s `"speechAnalyzer"` (camelCase). Selector
|
||||
exposes its constants as `public static let backend*` so callers shouldn't
|
||||
hard-code strings, but both sides do. Recommend: rename the constants OR
|
||||
use them at the call sites; pick one shape consistently. (See P0-2.)
|
||||
|
||||
### P2-2. `SpeechSynthDelegate.setPendingContinuation` overwrites without resuming
|
||||
|
||||
`AVSpeechSynth.swift:89–93`. If two concurrent `speak` calls arrive,
|
||||
the second `setPendingContinuation` replaces the stored continuation
|
||||
without resuming the first. The lost continuation never resumes and the
|
||||
first `withCheckedContinuation` block hangs forever. Today this can't
|
||||
fire because `TTSEngineActor`'s serial executor serializes access — but
|
||||
the type itself is `@unchecked Sendable` and exposes the race. If anyone
|
||||
ever uses `AVSpeechSynth` outside the actor, the bug ships. Recommend:
|
||||
on `setPendingContinuation`, if there's an existing continuation, resume
|
||||
it before replacing (matches the `didCancel` semantics from
|
||||
`stopSpeaking(at:.immediate)`).
|
||||
|
||||
### P2-3. WhisperKit feature flag exists but no path through reaches it
|
||||
|
||||
`Config/Sources/Config/STTConfig.swift` has `whisperKitFallback: Bool`;
|
||||
default-config.json sets it `false`. Even if a user flips it, the
|
||||
hard-coded factory at `AppDelegate.swift:1272` ignores them. The flag is
|
||||
test-quality dead code — it appears in `PerTurnSnapshot` because the
|
||||
config-to-snapshot pipeline is generic, but no production consumer
|
||||
exists. Tracks alongside P0-2.
|
||||
|
||||
### P2-4. Vision-isolation invariant is enforced; vision-audit may want to flag inverse
|
||||
|
||||
`scripts/check-presence-vision-isolation.sh` Layer 3 gate keeps TTS
|
||||
engine references out of `AppDelegate.swift` — that's the reason
|
||||
`VoiceOutputWiring.swift` lives in `App/Voice/` (per its docstring). The
|
||||
invariant is intact (no `TTSEngineActor(` in `AppDelegate.swift`).
|
||||
Voice-side cleanliness OK; flagging here as documentation more than a
|
||||
bug.
|
||||
|
||||
---
|
||||
|
||||
## Verified healthy
|
||||
|
||||
These were burned in prior audits and survived end-to-end re-trace:
|
||||
|
||||
- **Model files bundle correctly.** `scripts/copy-voice-models.sh` runs
|
||||
as an Xcode build phase (`project.yml:309–324`), maps lowercase
|
||||
`openwakeword` → `openWakeWord`, validates manifests have no
|
||||
`PLACEHOLDER_` SHA, hard-fails on missing files, and `rsync -a
|
||||
--delete` keeps the bundle clean. Verified physical files at
|
||||
`build/Build/Products/Debug/Jarvis.app/Contents/Resources/Models/`.
|
||||
- **`LiveSpeechAnalyzerBridge` actually wires SpeechAnalyzer.**
|
||||
`SpeechAnalyzerSTT.swift:185–323` constructs `SpeechTranscriber`,
|
||||
queries `bestAvailableAudioFormat`, builds an `AVAudioConverter` when
|
||||
needed, and spawns the result-drain Task **before** `analyzer.start`
|
||||
so early partials aren't lost. The 2026-05-03 `_ = chunk` stub is
|
||||
gone. `feed()` packs PCM via `PCMBufferBuilder`, converts if needed,
|
||||
yields `AnalyzerInput(buffer:)` on the input continuation. `finish()`
|
||||
finishes the builder, calls `finalizeAndFinishThroughEndOfInput()`,
|
||||
awaits the results task.
|
||||
- **`ProductionChunkPump` uses subscriber fan-out.**
|
||||
`App/Voice/ProductionChunkPump.swift` subscribes via
|
||||
`AudioGraphOwner.subscribe()` (per-subscriber ring), drains at
|
||||
1024-frame windows, unsubscribes on exit. Track B-7 sample-stealing
|
||||
invariant intact.
|
||||
- **`AudioLevelEmitter` is wired in production.**
|
||||
`AppDelegate.swift:1287–1292` subscribes via the broadcaster and
|
||||
hands the subscription to `AudioLevelEmitter`. `VoiceController`
|
||||
starts/stops it on listening-boundary transitions. P1-1 from the
|
||||
2026-05-04 audit closed correctly.
|
||||
- **Mic TCC permission is requested.** `installVoice` blocks on
|
||||
`AVCaptureDevice.requestAccess(for: .audio)` before
|
||||
`graphOwner.open()` (line 1149–1151). Verified in live log:
|
||||
`installVoice: requesting mic permission (.notDetermined)` →
|
||||
`mic permission granted`.
|
||||
- **AEC fallback is real.** `AudioGraphOwner.buildGraph(preferAec:)`
|
||||
catches `aecUnavailable`, emits degradation event, retries with
|
||||
`aec=false`, throws `bothVariantsFailed` if both fail. Live log
|
||||
shows `variant=aecOn` and BootHealth `voice state=ok` —
|
||||
consistent.
|
||||
- **`OrchestratorEventBroadcaster` voice priority** correctly protects
|
||||
`.turnEnd` and `.error` from drop; voice translator subscriber at
|
||||
`AppDelegate.swift:1855–1889` gates on `turnSourceWasVoice` before
|
||||
emitting `turnEnded` into `VoiceOrchestratorAdapter`.
|
||||
- **Single-writer HudState gate passes.**
|
||||
`bash scripts/check-single-writer-hudstate.sh` clean.
|
||||
- **`check-no-null-voice-adapters.sh` passes.** Confirmed
|
||||
`VoiceTTSAdapter(engine: nil)` from the 2026-05-03 audit is gone;
|
||||
AppDelegate constructs a real engine.
|
||||
- **VoiceController shutdown cancels all background tasks** including
|
||||
`sttFinalizeTask` (P1-3 closure from 2026-05-04). The session-id
|
||||
generation guard at `handleSTTFinalized(text:sessionId:)` is in
|
||||
place.
|
||||
- **Anti-pattern guards intact.** Transcript text never logged (T-06-05-03);
|
||||
`transcriptStore?.append` is called on submit/cancelAndSubmit
|
||||
(BLOCKER-1); single `cancelAndSubmit` call site in `bargeIn()`
|
||||
(VOICE-14); 200 ms barge-in debounce (T-06-05-04).
|
||||
|
||||
---
|
||||
|
||||
## Open questions
|
||||
|
||||
1. **Does the user observe wake-word still working after plugging
|
||||
headphones in?** P0-1 predicts "no, until app restart". If the
|
||||
live host is producing voice turns post-headphone-swap, my read
|
||||
of the rebuild path is wrong somewhere; please cross-check.
|
||||
2. **Is `tts.tier = "tier2"` ever actually expected to wire through
|
||||
today?** P0-4 is technically dead config because Orpheus weights
|
||||
aren't shipped, but the chain breaks before it gets to the
|
||||
"tier-2 → tier-1 degrade" path documented at
|
||||
`TTSEngineActor.swift:114–116`.
|
||||
3. **`AVSpeechSynthesisVoice(identifier: "en-US")` returns nil but
|
||||
the fallback `language: "en-US"` works.** Worth a config sanity
|
||||
check that future custom voices use `com.apple.voice…` identifier
|
||||
strings.
|
||||
|
||||
---
|
||||
|
||||
## Top-3 summary
|
||||
|
||||
1. **P0-1 — wake-word dies on every audio-graph rebuild.**
|
||||
AppDelegate wires `cancelInFlight → wakeWordDAG.cancel()`
|
||||
(finishes the stream) instead of the existing `stopFeed()`
|
||||
(preserves stream across rebuilds). The fix was already coded and
|
||||
documented in the DAG; the call site was never updated. Plus the
|
||||
rebuild consumer never re-arms the DAG against the new ring. Net
|
||||
effect: wake word stops working on first device-change /
|
||||
mic-regrant / ring-overflow event of the session.
|
||||
|
||||
2. **P0-2 — STT backend hard-coded; WhisperKit fallback unreachable.**
|
||||
`STTBackendSelector.make(backend: "speech_analyzer")` is a literal
|
||||
string in `installVoice`, ignoring `PerTurnSnapshot.stt`. Compound
|
||||
bug: the `get_self_state` tool emits `"whisperKit"` / `"speechAnalyzer"`
|
||||
(camelCase) while the selector accepts `"whisperkit"` /
|
||||
`"speech_analyzer"` (snake_case) — even fixing the hard-code would
|
||||
land in the default `unknown backend` fallthrough.
|
||||
|
||||
3. **P0-3 — `.ttsStopped` contract broken; normal synthesis never
|
||||
releases ducking.** `TTSInterrupt.swift` grep-gates `.ttsStopped`
|
||||
emission to **one** site (the barge-in path), but the
|
||||
`TTSEngineActor` header and `TTSEvent.swift` say
|
||||
ducking-release fires only on `.ttsStopped`. Natural completion
|
||||
emits `.finished` and nothing else. No production ducking consumer
|
||||
exists yet, so this isn't biting today — but the contract is
|
||||
structurally broken and will silently dock any future consumer.
|
||||
@@ -6,6 +6,18 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
||||
|
||||
**GitHub Issues at https://github.com/KofTwentyTwo/Jarvis/issues is the canonical tracker for all bugs, audit findings, and feature work.** `docs/TODO.md` is retained for **historical reference only** — do NOT add new entries to it. Migration phases live as `epic` issues; bugs as `bug` issues; audit findings as `audit-finding` issues. Label conventions: `severity:<critical|high|medium|low>`, `area:<subsystem>`. New work always opens an issue; commits reference issues with `Closes #N` or `Refs #N`.
|
||||
|
||||
## Roadmap
|
||||
|
||||
**Current roadmap:** `.planning/ROADMAP-2026-05-12.md` — user-outcome-focused milestone roadmap. Supersedes `.planning/ROADMAP.md` (2026-04-22, build-phase roadmap, which describes shipped phases).
|
||||
|
||||
**Milestones** (https://github.com/KofTwentyTwo/Jarvis/milestones): all 142 open issues are assigned to one of six milestones.
|
||||
- **v0.1 — It actually works (basic capabilities)** — 28 issues. Critical audit findings that break basic capabilities (voice, listening, seeing, facts, debugging dialogs, basic UI). Tactical patches so the user has a working Jarvis NOW; some will be re-fixed during API migration.
|
||||
- **v0.2 — Observable + stable (audit punch-list)** — 23 issues. Remaining audit findings (MED/LOW), plus PTT (#87) and Input Monitoring TCC banner (#88).
|
||||
- **v0.5 — API M-0..M-3 (Self / Settings / Diagnostics surfaces)** — 31 issues. Migration phases M-0 substrate, M-1 Self, M-2 Settings, M-3 Diagnostics. Closes B-08.
|
||||
- **v0.8 — API M-4..M-7 (Memory / Vision / Voice / Turn surfaces)** — 36 issues. Migration phases M-4 Memory, M-5 Vision (B-03), M-6 Voice (B-04, B-05), M-7 Turn (B-06 partial).
|
||||
- **v1.0 — Shipping (commenting, behavioral gates, first-launch polish)** — 13 issues. Code-commenting style rollout, doc reconciliation. Orthogonal to migration.
|
||||
- **v1.1 — Backlog (Orpheus, WhisperKit, Voice Log, presence, advanced)** — 11 issues. Deferred capabilities per migration plan §10.
|
||||
|
||||
## Repository state
|
||||
|
||||
Post-v0.12.0 audit-and-stabilize. All 9 GSD phases shipped (Foundations → Bus → HUD → Agent Core → MCP → Voice → Memory+Vision → Hardening → Orchestrator Wiring). Active workstream is the v1.0 API migration (M-0..M-7 epics in GitHub Issues) plus the 2026-05-12 audit follow-ups. Day-to-day handoff narrative still lives in `docs/SESSION-STATE.md`; live triage has moved to GitHub Issues. Milestone-level state lives in `.planning/STATE.md`.
|
||||
|
||||
Reference in new issue
Block a user