james.maesandClaude Opus 4.7
048ab08ef6
The 2026-05-03 INT-3 smoke test surfaced three latent runtime bugs the unit-level GSD verifications all missed because the audits were source- grep, not runtime-trace: 1. **F-E-RACE-1** — `agentInstallTask` awaited memory + vision installs but NOT the MCP runtime build task. MCPRuntime build is ~1s due to helper child-process spawn, so installAgent ALWAYS short-circuited on `mcpRuntime == nil` and the orchestrator + broadcaster + ALL six broadcaster subscribers stayed dormant for the entire process lifetime. Fix: capture `mcpInstallTask` Task handle and await it before calling installAgent. The v0.12.0 milestone audit's claim of "5 broadcaster subscribers wired" was accurate at source-grep level and false at runtime. 2. **F-E-FK-1** — installAgent constructed the orchestrator with a fresh SessionID but never called `replayLog.beginSession(...)`. Every `startTurn` violated the FK and returned `SubmitOutcome.rejected(reason: .configError)` — surfacing in the chat panel as "Config error — see ~/Library/Logs/Jarvis/system.log." Production code only ever called beginSession in tests (MCPRuntimeWiringTests). Fix: installAgent now reads CFBundleShortVersionString + CFBundleVersion from Info.plist, calls beginSession, and passes the returned SessionID to the orchestrator. 3. **F-E-WIRE-1** — `OutboundBatcher` was constructed only inside `installVoice`, AFTER OpenWakeWord/Silero models loaded. Voice DAG short-circuit on missing models in Debug builds left `self.outboundBatcher == nil`, and every `outboundBatcher?...` chained-optional in the .bus subscriber silently dropped tokenDelta / turnStarted / turnEnded / submitRejected. The chat panel got nothing back from the orchestrator. Fix: hoist batcher construction into installAgent (only depends on webviewBridge, not voice models); installVoice reuses self.outboundBatcher rather than overwriting. In addition, the .bus subscriber's translation logic — including StopReason → TurnTerminator mapping, turnStarted synthesis on first emission for a fresh TurnID, error → submitRejected forwarding for mid-stream failures — was inline in AppDelegate and therefore impossible to unit-test (App target xctest harness is upstream-broken on Xcode 26). Extracted to `BusForwarder` in AgentOrchestrator with a sink protocol; App provides `AppBusForwarderSink` (~50 LOC) as the glue. Every translation branch now has unit coverage. Files: - packages/AgentCore/Sources/AgentOrchestrator/BusForwarder.swift — Pure logic. `BusForwarderSink` protocol; `BusForwarder.terminator(for:)` StopReason mapping; `BusForwarder.drain(events:sink:)` async loop with per-turn lastEmittedTurnId state. No Bus package dep — keeps the layering clean. - packages/AgentCore/Tests/AgentOrchestratorTests/BusForwarderTests.swift — 16 cases covering every branch: terminator mapping (5, exhaustive on StopReason), tokenDelta forwarding (3 — synthesize start, dedupe start across same-turn deltas, two-turn handoff), turnEnd forwarding (4 — mapped terminator, synthesize-start-on-bare-turnEnd, streamTruncated→errored, state reset), error forwarding (2), ignored events (1), malformed-id guard (1). - packages/Replay/Tests/ReplayTests/SessionForeignKeyTests.swift — 3 cases (FK1: startTurn-without-beginSession-throws; FK2: post- beginSession-succeeds; FK3: multiple-startTurns-share-one-session). Closes the bug class regardless of any future AppDelegate refactor. - App/AppBusForwarderSink.swift — thin transport adapter. Translates `BusForwarder.Terminator` to wire-format `Bus.TurnTerminator` and dispatches across OutboundBatcher actor. - App/AppDelegate.swift — race fix, beginSession call, batcher hoist, inline subscriber replaced with `BusForwarder.drain(events: sink:)`. - Jarvis.xcodeproj/project.pbxproj — xcodegen regen picks up new App source file. - .planning/AUDIT-FINDINGS.md — three new findings (F-E-RACE-1, F-E-FK-1, F-E-WIRE-1) documented and marked closed; refactor rationale recorded; carry-forward items called out (streamTruncatedFinal cause unverified without Keychain access; F1 chat-turn end-to-end test is next). Verification: - AgentCore 194/194 (was 178; +16 BusForwarder). - Replay 33/33 (was 30; +3 SessionFK). - Bus 55/56 (F-A2-01 pre-existing). - All 15 boundary grep gates PASS. - scripts/check-app-builds.sh PASS. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
53 lines
2.1 KiB
Swift
53 lines
2.1 KiB
Swift
import Foundation
|
|
import AgentOrchestrator // BusForwarder, BusForwarderSink
|
|
import Bus // OutboundBatcher, BusOutbound, TurnTerminator
|
|
|
|
/// App-side adapter that wires `BusForwarder` (pure logic, lives in
|
|
/// AgentOrchestrator for unit-testability) to `OutboundBatcher` (the
|
|
/// transport-coalescing actor in the Bus package). The sink's only job is
|
|
/// translating the protocol-agnostic `BusForwarder.Terminator` enum into
|
|
/// the wire-format `Bus.TurnTerminator` and dispatching across the batcher
|
|
/// actor boundary.
|
|
///
|
|
/// This split is the lesson from the 2026-05-03 INT-3 audit: the bus
|
|
/// forwarder lived inline in `AppDelegate.swift` previously, which made it
|
|
/// untestable because the App target's xctest harness is upstream-broken
|
|
/// on Xcode 26. Every branch now has unit coverage in
|
|
/// `BusForwarderTests.swift`; this file is the (small, hard-to-misread)
|
|
/// glue layer.
|
|
struct AppBusForwarderSink: BusForwarderSink {
|
|
let batcher: OutboundBatcher
|
|
|
|
func postToken(_ chunk: String) async {
|
|
await batcher.postToken(chunk)
|
|
}
|
|
|
|
func sendTurnStarted(id: UUID) async {
|
|
try? await batcher.flushAndSend(.turnStarted(id: id))
|
|
}
|
|
|
|
func sendTurnEnded(id: UUID, terminator: BusForwarder.Terminator) async {
|
|
try? await batcher.flushAndSend(
|
|
.turnEnded(id: id, terminator: Self.wireTerminator(from: terminator))
|
|
)
|
|
}
|
|
|
|
func sendSubmitRejected(reason: String) async {
|
|
try? await batcher.flushAndSend(.submitRejected(reason: reason))
|
|
}
|
|
|
|
/// 1:1 mapping from `BusForwarder.Terminator` (protocol-agnostic, lives
|
|
/// in AgentCore) to `Bus.TurnTerminator` (wire format). Both enums must
|
|
/// stay in sync; the cases are deliberately identical so each call site
|
|
/// either fans out trivially or fails fast at compile time when one
|
|
/// side adds a case.
|
|
static func wireTerminator(from t: BusForwarder.Terminator) -> TurnTerminator {
|
|
switch t {
|
|
case .completed: return .completed
|
|
case .cancelled: return .cancelled
|
|
case .errored: return .errored
|
|
case .superseded: return .superseded
|
|
}
|
|
}
|
|
}
|