From 3f4c8e8fd97045e4eff1d01a6a40aa76b20e37f3 Mon Sep 17 00:00:00 2001 From: James Maes Date: Thu, 14 May 2026 09:11:52 -0500 Subject: [PATCH 1/2] docs(memory): scrub Phase-7 hedge language from MemoryStore (#19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit 2026-05-12 flagged five doc-comment references that mis-described the implementation state. The cited plans (07-02, 07-03, 07-06) shipped months ago — the bounded extraction channel, the hybrid FTS5 + vec0 read path, the ReplayEvent emission, the regression corpus test seams, and `installMemory`'s replay-sink wiring are all live, not "wires" or "will replace" promises. Five touchpoints rewritten to describe the code as it is: - `MemoryReplaySink` doc: drop the "Plan 07-06" plan reference; the seam is just used at startup. - `MemoryStore` actor doc: replace the "Plan 07-02 wires / Plan 07-03 wires" sentence with the actual contract (single writer; hybrid search through `MemoryReadStore`; `recordRetrieval` emits `memoryRetrieval`). - Read-path MARK: drop the "Plan 07-03" plan label; keep the requirement-ID list (MEM-07 / TEXT-03 / D-02 / D-05) since those are still useful for traceability to the audit corpus. - `recordRetrieval` doc: replace "Mirrors recordMemoryMutation from 07-02" with "Mirrors recordMemoryMutation on the write path." - Test-seams MARK: drop "Plan 07-06" prefix; describe by the test target (JARVIS_REAL_MODELS=1 corpus suite) instead. The 2026-05-11 D-5/D-6 markers and the "Boot policy (changed 2026-05-11)" doc are preserved unchanged — they document current behaviour, not historical hedges. `swift build --package-path packages/Memory` clean. Closes #19 Co-Authored-By: Claude Opus 4.7 --- .../Memory/Sources/Memory/MemoryStore.swift | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/Memory/Sources/Memory/MemoryStore.swift b/packages/Memory/Sources/Memory/MemoryStore.swift index 9c849e4..f2b9c86 100644 --- a/packages/Memory/Sources/Memory/MemoryStore.swift +++ b/packages/Memory/Sources/Memory/MemoryStore.swift @@ -12,7 +12,7 @@ import Replay /// Test seam over `Replay.ReplayLog`. The Memory module deliberately does NOT /// depend on a concrete `ReplayLog` instance — `AppDelegate.installMemory` -/// (Plan 07-06) hands a wrapper through `setReplayLog`. Tests inject a +/// hands a wrapper through `setReplayLog` at startup. Tests inject a /// mock conforming to this protocol. public protocol MemoryReplaySink: Sendable { func record(_ event: ReplayEvent, forTriggerTurnId triggerTurnId: Int64) @@ -34,8 +34,10 @@ public protocol MemoryReplaySink: Sendable { /// memory store the app refuses to launch. This eliminates the silent /// dormancy that hid B-02 / B-04-class substrate failures. /// -/// The actor is the only writer. Plan 07-02 wires the bounded extraction -/// channel; Plan 07-03 wires the search path + ReplayEvent emission. +/// The actor is the only writer. The bounded extraction channel writes +/// here via `applyOp`; the hybrid (FTS5 + vec0) search path reads through +/// `MemoryReadStore` conformance and emits `ReplayEvent.memoryRetrieval` +/// via `recordRetrieval`. public actor MemoryStore { public static let logChannel = "memory" @@ -331,11 +333,11 @@ public actor MemoryStore { sink.record(.memoryMutation(bytes), forTriggerTurnId: triggerTurnId) } - // MARK: - Read path (Plan 07-03 — MEM-07 + TEXT-03 + D-02 + D-05) + // MARK: - Read path (MEM-07 + TEXT-03 + D-02 + D-05) /// SOLE production emission site for ReplayEvent.memoryRetrieval (D-05). - /// Mirrors recordMemoryMutation from 07-02. Best-effort; if no replay - /// sink has been injected, the call is a silent no-op. + /// Mirrors `recordMemoryMutation` on the write path. Best-effort; if no + /// replay sink has been injected, the call is a silent no-op. public func recordRetrieval(_ ref: FactRef, triggerTurnId: Int64) { guard let sink = replayLog else { return } let stamp: Int64 = ref.timestamp == 0 @@ -538,13 +540,13 @@ public actor MemoryStore { } } -// MARK: - HybridSearch + SessionHistory conformance (Plan 07-03 Task 2) +// MARK: - HybridSearch + SessionHistory conformance extension MemoryStore: MemoryReadStore {} extension MemoryStore: SessionHistoryReading {} extension OllamaEmbeddingClient: EmbeddingProviding {} -// MARK: - Plan 07-06 / Regression corpus test seams +// MARK: - Regression corpus test seams // // These small read-only seams are exercised exclusively by the env-gated // MemoryRegressionCorpusTests suite (JARVIS_REAL_MODELS=1). They wrap raw -- 2.54.0 From 2478eddcfd608305b1ac116d8b6997c7fb7f2c2c Mon Sep 17 00:00:00 2001 From: James Maes Date: Thu, 14 May 2026 09:27:31 -0500 Subject: [PATCH 2/2] docs(bus): scrub JarvisBusWorld drift + add grep gate (#45) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fb41c5f` (2026-05-02) unified `Injection.js` and the HUD bundle into the default `WKContentWorld.page` and retired the prior isolated `JarvisBusWorld` named world. The doc-comments and test fixtures across 20+ lines kept describing the bug-era topology — pre-existing-bus attach "because cross-world isolation", message-handler-only-visible-in-named- world, etc. — for months afterward. Result: a new reader following the docs would conclude the bus uses two worlds when it actually uses one. Five rationale rewrites: - `WebviewBridge.swift` class doc — handler is on the page world, not an isolated named world. Page scripts in the loaded HUD bundle address it via `window.webkit.messageHandlers.jarvisBus`. - `Injection.js` header — names the page world explicitly; one-line nod to `fb41c5f` for the historical context. - `bridge.ts` JSDoc + `isJarvisBusShape` rationale — recast the H-02 attach-to-existing path as a "defensive double-install guard" inside ONE page world. Injection.js fires first at document-start; the bundle installer fires later as a top-level `main.tsx` side-effect; the guard prevents the bundle from clobbering Injection.js's queued early messages and captured handshake state. The duck-type `isJarvisBusShape` CODE is unchanged — only the comment narrative was misleading. - `webview/packages/hud/src/main.tsx` — drop the cross-world-isolation framing; describe the single-world reality. - `App/AppDelegate.swift` install comment — page world, not isolated. Test-side updates (same scrub, plus one latent test bug surfaced): - `WebviewBridgeTests.test_scriptMessageHandlerIsRegisteredUnderConfigured- Name`: was removing from `WKContentWorld.world(name: "JarvisBusWorld")` while the bridge registered against `.page` — silent no-op since `removeScriptMessageHandler` doesn't throw on absent. Now removes from `.page` (the world the bridge actually uses), making the "registration-is-evidence-of-success" assertion meaningful again. - `WebviewBridgeOutboundTests`: comment-only. - `RealWKWebViewIntegrationTests`: comment-only (forensic narrative kept, literal world name replaced with "prior isolated named content world"). - `webview/packages/bus/tests/bridge.test.ts`: H-02 describe-block + one it-name rewritten to match the new defensive-double-install rationale. Infrastructure: - `scripts/check-no-jarvis-bus-world-references.sh`: new boundary gate. Greps for the literal `JarvisBusWorld` in tracked files; excludes `.planning/`, `docs/`, `webview/packages/*/dist/`, and this script itself. Wired into `CLAUDE.md`'s gate listing; the CI `boundary-gates` job auto-discovers all `scripts/check-*.sh`, so no workflow edit is needed. - `webview/packages/bus/package.json`: 0.0.0 → 0.0.1 (per spec — comment-only changes still bump patch so consumers can pin). - `webview/packages/bus/dist/bridge.{js,d.ts}`: rebuilt via `pnpm --filter @jarvis/bus build`; NOT committed because `dist/` is not tracked (the build phase regenerates on each xcodebuild invocation). Verified locally: - `swift build --package-path packages/Bus` clean - `pnpm --filter @jarvis/bus test` — 37/37 pass - `bash scripts/check-no-jarvis-bus-world-references.sh` — PASS - `bash scripts/check-bus-protocol-version.sh` — PASS (v2.3.0) - `bash scripts/check-bus-harness-parity.sh` — PASS Closes #45 Co-Authored-By: Claude Opus 4.7 --- App/AppDelegate.swift | 9 ++-- CLAUDE.md | 1 + .../Bus/Sources/Bus/Resources/Injection.js | 7 +-- packages/Bus/Sources/Bus/WebviewBridge.swift | 5 +- .../RealWKWebViewIntegrationTests.swift | 16 +++--- .../BusTests/WebviewBridgeOutboundTests.swift | 4 +- .../Tests/BusTests/WebviewBridgeTests.swift | 7 ++- .../check-no-jarvis-bus-world-references.sh | 54 +++++++++++++++++++ webview/packages/bus/package.json | 2 +- webview/packages/bus/src/bridge.ts | 48 +++++++++-------- webview/packages/bus/tests/bridge.test.ts | 24 ++++----- webview/packages/hud/src/main.tsx | 18 +++---- 12 files changed, 129 insertions(+), 66 deletions(-) create mode 100755 scripts/check-no-jarvis-bus-world-references.sh diff --git a/App/AppDelegate.swift b/App/AppDelegate.swift index dcaa730..439ea5a 100644 --- a/App/AppDelegate.swift +++ b/App/AppDelegate.swift @@ -740,10 +740,11 @@ final class AppDelegate: NSObject, NSApplicationDelegate { // 4.5 Bus wiring — construct the `WebviewBridge` around the HUD's // `WKWebView`, install the `WKUserScript` at document-start in - // the isolated `JarvisBusWorld`, start the single-writer - // `HudStateCoordinator`, and load the R3F bundle's - // `index.html` so the handshake kicks off. See `installBus` - // for the seven-step sequence. + // the page world (the shared `WKContentWorld.page` used by + // Injection.js and the HUD bundle alike since `fb41c5f`), + // start the single-writer `HudStateCoordinator`, and load the + // R3F bundle's `index.html` so the handshake kicks off. See + // `installBus` for the seven-step sequence. installBus() // 5. Keychain probe. Banner suppression: when the API key is diff --git a/CLAUDE.md b/CLAUDE.md index 489b8fe..f6f3a97 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -209,6 +209,7 @@ bash scripts/check-corpus-secrets.sh bash scripts/check-embedding-dim-literal.sh bash scripts/check-install-order.sh bash scripts/check-no-evaluate-javascript.sh +bash scripts/check-no-jarvis-bus-world-references.sh bash scripts/check-no-leftover-stubs.sh bash scripts/check-no-modal-presentation.sh bash scripts/check-no-null-voice-adapters.sh diff --git a/packages/Bus/Sources/Bus/Resources/Injection.js b/packages/Bus/Sources/Bus/Resources/Injection.js index ee0cd3a..90d85b3 100644 --- a/packages/Bus/Sources/Bus/Resources/Injection.js +++ b/packages/Bus/Sources/Bus/Resources/Injection.js @@ -1,6 +1,7 @@ -// Injected at document-start in WKContentWorld("JarvisBusWorld"). -// Installs window.jarvisBus with the minimum surface needed to complete -// the handshake. +// Injected at document-start in the page world (the default +// `WKContentWorld.page` since `fb41c5f` retired the prior isolated named +// world; see WebviewBridge.swift). Installs window.jarvisBus with the +// minimum surface needed to complete the handshake. // // IMPORTANT — protocolVersion drift hazard: the full TS bundle's installer // (Vy() in the minified output) shape-checks window.jarvisBus and SKIPS diff --git a/packages/Bus/Sources/Bus/WebviewBridge.swift b/packages/Bus/Sources/Bus/WebviewBridge.swift index f695613..50d25b3 100644 --- a/packages/Bus/Sources/Bus/WebviewBridge.swift +++ b/packages/Bus/Sources/Bus/WebviewBridge.swift @@ -6,8 +6,9 @@ import JarvisLogging /// Main-actor-bound bridge between Swift and the webview's JS layer. /// /// Responsibilities: -/// - Install a `WKScriptMessageHandlerWithReply` in an isolated content world -/// (`JarvisBusWorld`) so page scripts cannot observe or post to it. +/// - Install a `WKScriptMessageHandlerWithReply` on the page world +/// (`WKContentWorld.page`, the default since `fb41c5f`) so page scripts +/// in the loaded HUD bundle can address it via `window.webkit.messageHandlers.jarvisBus`. /// - Inject a `WKUserScript` at `.atDocumentStart` in the same world so /// `window.jarvisBus` exists before any page script runs (pitfall 3). /// - Decode inbound JSON into `BusInbound` and either (a) handle the handshake diff --git a/packages/Bus/Tests/BusTests/RealWKWebViewIntegrationTests.swift b/packages/Bus/Tests/BusTests/RealWKWebViewIntegrationTests.swift index 97ad7c0..fb34f66 100644 --- a/packages/Bus/Tests/BusTests/RealWKWebViewIntegrationTests.swift +++ b/packages/Bus/Tests/BusTests/RealWKWebViewIntegrationTests.swift @@ -15,7 +15,7 @@ import WebKit /// **Why this exists** — Phase D-Phase-2 / Phase F1 of /// `.planning/AUDIT-AND-FIX-PLAN.md`. The 2026-05-02 audit-and-fix /// session uncovered a `WKContentWorld` isolation bug -/// (`Injection.js` ran in `WKContentWorld(name: "JarvisBusWorld")` +/// (`Injection.js` ran in a prior isolated named content world /// while the bundle ran in the page world; `window.jarvisBus` /// properties were per-world isolated; bundle's `attachBus()` saw /// `undefined` and created a SECOND fresh bus object; two stubs, two @@ -127,13 +127,13 @@ final class RealWKWebViewIntegrationTests: XCTestCase { /// **The test that would have caught the WKContentWorld bug.** /// - /// On the bug branch, `Injection.js` ran in - /// `WKContentWorld(name: "JarvisBusWorld")` while - /// `WebviewBridge.sendRaw` invoked `callAsyncJavaScript` against - /// the same named world — but the message *handler* was registered - /// against `WKContentWorld.page`, so JS-side `postMessage` calls - /// from the named-world stub never reached Swift. Handshake hung - /// at `.sentHello`, eventually timed out with an `NSAlert`. + /// On the bug branch, `Injection.js` ran in the prior named + /// content world while `WebviewBridge.sendRaw` invoked + /// `callAsyncJavaScript` against the same named world — but the + /// message *handler* was registered against `WKContentWorld.page`, + /// so JS-side `postMessage` calls from the named-world stub never + /// reached Swift. Handshake hung at `.sentHello`, eventually timed + /// out with an `NSAlert`. /// /// On the fixed branch (fb41c5f), Injection.js auto-acks `hello` /// at the stub layer in the unified `.page` world, the bridge's diff --git a/packages/Bus/Tests/BusTests/WebviewBridgeOutboundTests.swift b/packages/Bus/Tests/BusTests/WebviewBridgeOutboundTests.swift index 8730914..f5b9474 100644 --- a/packages/Bus/Tests/BusTests/WebviewBridgeOutboundTests.swift +++ b/packages/Bus/Tests/BusTests/WebviewBridgeOutboundTests.swift @@ -78,8 +78,8 @@ final class WebviewBridgeOutboundTests: XCTestCase { .decode(BusOutbound.self, from: Data((payload ?? "").utf8)) XCTAssertEqual(decoded, .hudState(.idle)) // F-A2-01: production WebviewBridge uses `WKContentWorld.page` (the - // default since `fb41c5f`); the prior `JarvisBusWorld` named-world - // assertion was retired in that commit but this test wasn't updated. + // default since `fb41c5f`); the prior named-world assertion was + // retired in that commit but this test wasn't updated until #45. XCTAssertEqual(call.contentWorld, .page) } diff --git a/packages/Bus/Tests/BusTests/WebviewBridgeTests.swift b/packages/Bus/Tests/BusTests/WebviewBridgeTests.swift index a63b494..2e7839c 100644 --- a/packages/Bus/Tests/BusTests/WebviewBridgeTests.swift +++ b/packages/Bus/Tests/BusTests/WebviewBridgeTests.swift @@ -160,11 +160,14 @@ final class WebviewBridgeTests: XCTestCase { // Re-adding a handler with the same (contentWorld, name) pair would // raise `NSInvalidArgumentException`. The constructor succeeding is // itself evidence of registration. As a double-check, `remove` on a - // registered handler must succeed without throwing. + // registered handler must succeed without throwing. We remove from + // `WKContentWorld.page` because that is the default world + // `WebviewBridge.init` registers the handler against since `fb41c5f` + // unified bus and bundle into the same page world. let controller = webView.configuration.userContentController controller.removeScriptMessageHandler( forName: "jarvisBus", - contentWorld: WKContentWorld.world(name: "JarvisBusWorld") + contentWorld: .page ) // No XCTAssert needed — the remove call succeeding on a registered // handler is the assertion. diff --git a/scripts/check-no-jarvis-bus-world-references.sh b/scripts/check-no-jarvis-bus-world-references.sh new file mode 100755 index 0000000..466f734 --- /dev/null +++ b/scripts/check-no-jarvis-bus-world-references.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# +# check-no-jarvis-bus-world-references.sh +# +# Issue #45 boundary gate. +# +# The named `WKContentWorld("JarvisBusWorld")` was retired in `fb41c5f` — +# the bus message handler and `Injection.js` user script now both register +# against `WKContentWorld.page` (the default page world). Old doc-comments +# and test fixtures hung onto the name for months afterward; #45 closed +# the cleanup. +# +# This gate prevents new code or comments from re-introducing the literal +# string `JarvisBusWorld` outside the historical reference corpora: +# +# - `.planning/` archived plans / audit reports / handoffs +# - `docs/` spec documents that explain the migration +# - `node_modules/` vendored deps (out of repo control anyway) +# - `.build/`, build/ SwiftPM + Xcode derived data +# - `webview/packages/*/dist/` TypeScript build output (regenerated from src) +# - this script itself we have to reference the literal to grep for it +# - `.git/` git internals +# +# Fails CI if any other tracked file contains the literal string. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(dirname "$SCRIPT_DIR")" + +cd "$REPO_ROOT" + +# Use `git grep` so we automatically respect .gitignore (so dist/ etc. that +# aren't tracked don't trip the gate even if they exist on disk locally). +# `--cached` would miss new working-tree files; default mode greps the +# working tree filtered by tracked-or-untracked-not-ignored, which is what +# we want before a commit. +matches=$(git grep --no-color -n "JarvisBusWorld" -- \ + ':!.planning/' \ + ':!docs/' \ + ':!scripts/check-no-jarvis-bus-world-references.sh' \ + ':!webview/packages/*/dist/' \ + || true) + +if [ -n "$matches" ]; then + echo "[check-no-jarvis-bus-world-references] FAIL: tracked source still references the retired 'JarvisBusWorld' named content world." >&2 + echo "" >&2 + echo "$matches" >&2 + echo "" >&2 + echo "[check-no-jarvis-bus-world-references] Rewrite these to 'page world' / 'shared world' / 'defensive double-install guard' — see #45 and the JSDoc in webview/packages/bus/src/bridge.ts for the canonical phrasing." >&2 + exit 1 +fi + +echo "[check-no-jarvis-bus-world-references] PASS (no tracked source references the retired named world)" diff --git a/webview/packages/bus/package.json b/webview/packages/bus/package.json index 31377a8..6769ddf 100644 --- a/webview/packages/bus/package.json +++ b/webview/packages/bus/package.json @@ -1,7 +1,7 @@ { "name": "@jarvis/bus", "private": true, - "version": "0.0.0", + "version": "0.0.1", "type": "module", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/webview/packages/bus/src/bridge.ts b/webview/packages/bus/src/bridge.ts index b633baf..9dbdcb5 100644 --- a/webview/packages/bus/src/bridge.ts +++ b/webview/packages/bus/src/bridge.ts @@ -57,27 +57,28 @@ export interface InstallOptions { * - `send(msg)` JSON-encodes and posts to `webkit.messageHandlers.jarvisBus`. * Callers await the returned Promise to observe Swift's reply. * - * ## WKContentWorld attach-to-existing path (H-02) + * ## Defensive double-install guard (H-02) * - * When the HUD bundle runs in the page's default JS world (via - * `