Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
  james.maesandClaude Opus 4.7
|
2478eddcfd | ||
|
james.maesandClaude Opus 4.7
|
3f4c8e8fd9 | ||
|
james.maesandClaude Opus 4.7
|
dced4cb9a3 | ||
|
james.maes
|
00f1075ea5 |
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -0,0 +1,124 @@
|
||||
# Handoff — 2026-05-14: multi-runner shipped, v0.2 ready
|
||||
|
||||
**Session window:** 2026-05-13 afternoon (Grogu single runner setup) → 2026-05-14 (multi-runner expansion + first auto-Release)
|
||||
**Branch:** `develop`, clean
|
||||
**HEAD at write time:** `774e60a0` — `ci(actions): create GitHub Release directly in publish-tag job (#187)`
|
||||
**Open PR count:** 2 (#185, #191 — both with shepherds running)
|
||||
**Repo visibility:** PUBLIC
|
||||
|
||||
---
|
||||
|
||||
## TL;DR
|
||||
|
||||
- **3-runner self-hosted CI on Grogu is live** — `Grogu-jarvis`, `-2`, `-3`. Matrix parallelism observed (≥3 SPM jobs concurrent). Develop CI green in ~5 min vs. ~13 min single-runner.
|
||||
- **Auto-Release flow proven** — `v0.1.0-dev.64` has a real GitHub Release object, created by the new `gh release create` step in `publish-tag`. No more manual `gh release create` needed.
|
||||
- **Brainstorm → spec → plan → execute discipline established** via the Superpowers skills. Multi-runner work has a committed spec + plan under `docs/superpowers/{specs,plans}/`. Pattern to follow for v0.2 onward.
|
||||
- **Memory test flake** surfaced: `MemoryWiringEndToEndTests.testMemoryEnqueueReceivesNonNilUserAndAssistantText` fails intermittently on PR branches but passes on develop. Hypothesis: race in `TurnTranscriptStore` actor under concurrent-runner load. Pre-existing; not from any 2026-05-13/14 code change.
|
||||
|
||||
---
|
||||
|
||||
## What shipped today
|
||||
|
||||
### v0.1 mop-up + first dev tag
|
||||
- **PR #184** (squash `5fc2d3fa`) — Ollama model baseline reconciled to `qwen3.6:latest` (extractor) + `qwen2.5-coder:32b-instruct-q8_0` (agent fallback). Closes #58 (last v0.1 issue).
|
||||
- **v0.1.0-dev.50** — first release tag of the project. Release object created manually (workflow couldn't trigger downstream).
|
||||
- **v0.1.0-dev.64** — second release tag. Release object created **automatically** via the new inline `gh release create` step in `publish-tag`.
|
||||
|
||||
### Self-hosted runner saga
|
||||
- **PR #181** (`eef17252`) — Moved `swift-packages` matrix to `Grogu-jarvis`. DEVELOPER_DIR Xcode-select. Fork-PR gate.
|
||||
- **PR #183** (`81ce64c3`) — Orphan-process reaper at job start (closes #182).
|
||||
- **PR #189** (`4cdb3560`) — Reaper gets a `>=1h` age filter so sibling runners on Grogu don't kill each other.
|
||||
- **PR #187** (`774e60a0`) — `publish-tag` step creates the Release object directly instead of relying on `release.yml` triggering on tag push (GITHUB_TOKEN can't trigger downstream workflows).
|
||||
- **PR #190** (`a26f49fd`) — Multi-runner spec + plan committed to `docs/superpowers/`.
|
||||
- **PR #185** — open, AgentCore #23+#24 fix. Currently has a Memory-flake CI failure under shepherd watch.
|
||||
- **PR #191** — open, CLAUDE.md inventory update. Same Memory flake situation.
|
||||
|
||||
### Grogu state
|
||||
- 3 runners installed at `~/actions-runner-jarvis{,-2,-3}/`, each with `_work` symlinked to `/Volumes/HD-2/jarvis-runner-work{,-2,-3}/`.
|
||||
- LaunchAgents managed by `~/Library/LaunchAgents/actions.runner.KofTwentyTwo-Jarvis.Grogu-jarvis{,-2,-3}.plist`.
|
||||
- Keychain workflow: `~/Library/Keychains/jarvis-runner.keychain-db`, password in repo secret `JARVIS_RUNNER_KEYCHAIN_PASSWORD`. CI unlocks + makes default + prepends to search list.
|
||||
- Side effect: `jarvis-runner.keychain-db` is your user's default keychain after CI runs. Undo command in CLAUDE.md.
|
||||
|
||||
---
|
||||
|
||||
## Active background processes at handoff
|
||||
|
||||
| Task ID | What | Expected exit |
|
||||
|---|---|---|
|
||||
| `bny76q2ad` | Final shepherd watching PR #185 + #191 | When both PRs are handled (merged or marked `ci-failed`), or 60-min timeout |
|
||||
|
||||
**Log location:** `/tmp/final-shepherd.log`
|
||||
|
||||
Reading the log will tell you what the shepherd did. If it marked both PRs `ci-failed` because of the Memory flake, see "Memory flake disposition" below.
|
||||
|
||||
---
|
||||
|
||||
## Memory flake disposition (what to do when you pick up)
|
||||
|
||||
`MemoryWiringEndToEndTests.testMemoryEnqueueReceivesNonNilUserAndAssistantText` fails with:
|
||||
```
|
||||
XCTAssertEqual failed: ("0") is not equal to ("1") - enqueue was not called — BLOCKER-1 regression
|
||||
XCTAssertEqual failed: ("nil") is not equal to ("Optional("hi")") - userText empty/nil — Plan 4's submit-time append did not propagate via TurnTranscriptStore
|
||||
XCTAssertEqual failed: ("nil") is not equal to ("Optional("hello there")") - assistantText empty/nil — Plan 1's transcript subscriber did not propagate via TurnTranscriptStore
|
||||
```
|
||||
|
||||
**Evidence it's a flake, not a regression:**
|
||||
- PR #191 doesn't touch AgentCore at all (only `CLAUDE.md`) yet hits the same exact failure.
|
||||
- Develop's latest CI (`25832341662`, on `774e60a0`) passed Memory in 45s.
|
||||
- Both #185 and #191 are branched off slightly different develop SHAs — the test passes on the latest develop but fails on these branch HEADs after forward-merge.
|
||||
|
||||
**Recommended next steps:**
|
||||
1. Open a new issue: `bug,severity:medium,area:memory` — "MemoryWiringEndToEndTests intermittent failure under concurrent runner load". Describe the `TurnTranscriptStore` race hypothesis.
|
||||
2. Either rerun the failed jobs (CI may pass on next attempt) OR merge both PRs as-is since branch protection has 0 required checks and the test passes reliably on develop.
|
||||
3. Long-term: file the test as known-flaky in v0.2 milestone.
|
||||
|
||||
---
|
||||
|
||||
## v0.2 Wave 1 (next session start)
|
||||
|
||||
Tiny / doc-only / deterministic fixes — safe to land in a single session each:
|
||||
|
||||
| Issue | What | Effort | Why first |
|
||||
|---|---|---|---|
|
||||
| **#19** | MemoryStore.swift stale Phase 7 hedge doc-comments | ~5 min | Pure docs, zero risk |
|
||||
| **#45** | Bus 'JarvisBusWorld' docstring drift (20+ docstrings) | ~15 min | Pure docs, mechanical |
|
||||
| **#46** | Stale webview bundle artifacts in `App/Resources/webview/assets/` (~50MB) | ~10 min | File deletes |
|
||||
| **#37** | STTBackendSelector snake_case vs camelCase mismatch | ~15 min | Single rename |
|
||||
| **#178** | Add OpenSSF Scorecard workflow | ~15 min | One new `.github/workflows/` file |
|
||||
| **#47** | WebviewBridge.sendRaw NaN→null JSON guard | ~20 min | One-line bug fix |
|
||||
|
||||
Each gets brainstorm → spec → plan via the Superpowers skills (specs/plans templates established in `docs/superpowers/`). Brief specs (a few paragraphs) — these are "issue body IS the design" cases.
|
||||
|
||||
## v0.2 Wave 2 / Wave 3 — wait
|
||||
|
||||
Larger items that need real diagnostic work or design decisions:
|
||||
- #18 MemoryReplaySink error propagation — needs care
|
||||
- #23 + #24 already in PR #185 (held)
|
||||
- #44 HUD audioLevel consumer rewire — MED, design choice
|
||||
- #43 sessionHistory dead-letter — overlaps M-7, defer
|
||||
- #176 vitest 4.x major bump — needs eval
|
||||
- #26 confirmation panel multi-monitor — needs real hardware
|
||||
- Any M-0..M-7 issue — strategic, gets its own spec → plan
|
||||
|
||||
---
|
||||
|
||||
## Key references
|
||||
|
||||
- **Spec template:** `docs/superpowers/specs/2026-05-13-multi-runner-grogu-design.md`
|
||||
- **Plan template:** `docs/superpowers/plans/2026-05-13-multi-runner-grogu.md`
|
||||
- **Previous handoff:** `docs/HANDOFF-2026-05-13-v0.1-shipped.md` (now superseded for the multi-runner story but still describes v0.1 mop-up)
|
||||
- **Roadmap:** `.planning/ROADMAP-2026-05-12.md`
|
||||
- **Live state:** GitHub Issues + Milestones at https://github.com/KofTwentyTwo/Jarvis
|
||||
- **CI workflow:** `.github/workflows/ci.yml` — has the age-filtered cleanup + inline release create
|
||||
- **CLAUDE.md `Self-hosted CI runner (Grogu)`** section — operational reference for the 3-runner setup
|
||||
|
||||
## What this handoff deliberately doesn't relitigate
|
||||
|
||||
- The Memory flake's root cause — open a v0.2 bug and move on; develop's CI is reliably green so it's not blocking.
|
||||
- Whether to merge PR #185 / PR #191 with a known-flaky failure — branch protection allows it; user judgment.
|
||||
- Wave 2/3 v0.2 issues — they need design conversations; pick them up after Wave 1.
|
||||
- M-0..M-7 — strategic, gets its own session.
|
||||
|
||||
---
|
||||
|
||||
*Written 2026-05-14 at the natural session-end after multi-runner expansion shipped. Next session should start with `/clear`, read this doc, then read the latest `/tmp/final-shepherd.log` to see what the background shepherd did, then pick up Wave 1.*
|
||||
@@ -0,0 +1,114 @@
|
||||
# v0.2 Wave 1 sequencing and execution — design
|
||||
|
||||
**Status:** approved 2026-05-14 (autonomous execution authorized)
|
||||
**Authors:** brainstorm with James + Claude (Opus 4.7)
|
||||
**Scope:** Six tiny v0.2 issues (#19, #37, #45, #46, #47, #178) — sequenced into three PRs.
|
||||
|
||||
## Goal
|
||||
|
||||
Land all six Wave 1 issues against `develop` in three coherent PRs, in an order that minimises CI exposure to the known `MemoryWiringEndToEndTests` flake and keeps each PR's narrative reviewable as a single story.
|
||||
|
||||
## Why three PRs, not six and not two
|
||||
|
||||
Six individual PRs (the handoff's implicit default) maximises history granularity but burns 6× the SPM CI matrix and exposes us to the Memory flake on every branch. Two giant PRs maximise throughput but mix surfaces (build script + new workflow + bundle rebuild + Swift code) into a single review and a single revert unit.
|
||||
|
||||
Three PRs grouped by surface area give us:
|
||||
|
||||
- 3× CI runs instead of 6×.
|
||||
- Each PR has one coherent narrative (docs ⋄ infra ⋄ code).
|
||||
- Each PR can be reverted independently without untangling unrelated work.
|
||||
|
||||
## Composition
|
||||
|
||||
### PR 1 — "Doc drift cleanup" (`docs/v0.2-wave-1-doc-drift`)
|
||||
|
||||
**Issues:** #19 (MemoryStore stale Phase-7 doc-comments) + #45 (Bus JarvisBusWorld drift across 20+ docstrings post `fb41c5f`).
|
||||
|
||||
**Why grouped:** both are descriptive/textual changes to documentation that no longer matches the code. #45 already requires a `@jarvis/bus` bundle rebuild and a new grep-gate, so it warrants doc-class company rather than mixing with real code fixes. Both touch package-internal documentation only; no behavioural change.
|
||||
|
||||
**Deliverables:**
|
||||
|
||||
- `packages/Memory/Sources/Memory/MemoryStore.swift` — scrub "Plan 07-02 wires", "Plan 07-03 will replace", "future ops plan" hedges. Keep the 2026-05-11 comment at line 464–468 intact (it is correct).
|
||||
- Bus comments renamed across `WebviewBridge.swift`, `Injection.js`, `webview/packages/bus/src/bridge.ts`, `webview/packages/hud/src/main.tsx`, `App/AppDelegate.swift` — replace "JarvisBusWorld" / "WKContentWorld('JarvisBusWorld')" / cross-world-attach language with "page world" / "shared world" / "defensive double-install guard".
|
||||
- `bridge.ts:60–99` rationale rewritten — the duck-type `isJarvisBusShape` guard is now defensive against double-install (both `Injection.js`'s stub and the bundle's `installJarvisBus` running in the same page world), not cross-world attach. Comment must say this without misleading; do **not** delete the guard itself.
|
||||
- Bundle rebuilt: `webview/packages/bus/dist/bridge.{js,d.ts}` regenerated via `pnpm --filter @jarvis/bus build` (or equivalent).
|
||||
- `@jarvis/bus` `package.json` patch-version bumped to reflect comment changes in dist (matches existing project convention of patch-bumping when the published comments shift).
|
||||
- New grep-gate: `scripts/check-no-jarvis-bus-world-references.sh` — fails if any tracked file outside `.planning/`, `docs/`, or git-history references contains the literal `JarvisBusWorld`. Wired into the boundary-gates list in CI and in `CLAUDE.md`.
|
||||
|
||||
**Closes:** #19, #45.
|
||||
|
||||
### PR 2 — "Build hygiene" (`ci/v0.2-wave-1-build-hygiene`)
|
||||
|
||||
**Issues:** #46 (stale webview bundle artifacts) + #178 (OpenSSF Scorecard workflow).
|
||||
|
||||
**Why grouped:** both touch repo-level infrastructure (`scripts/` and `.github/workflows/`) and neither touches Swift packages, so the heavy SPM matrix runs but with low chance of new test failures. Quick CI; safe to land first.
|
||||
|
||||
**Deliverables:**
|
||||
|
||||
- `scripts/build-webview.sh` — before the existing `rsync -a ... dist/ -> App/Resources/webview/` at line 88, add an explicit cleanup of `App/Resources/webview/assets/*.js`, `*.js.map`, `*.css` (preserving `bus-harness.html`, the directory's `.gitignore`, and any other already-excluded files). Easiest mechanism: pass `--delete-excluded` plus an `--exclude` for each preserve target, or `rm -f` the asset glob before rsync. Either works; preference is `rm -f` of the explicit asset glob because it's grep-able.
|
||||
- `App/Resources/webview/assets/` — delete the 10 stale `index-*.js` files and their `.map` siblings in the same commit so the directory is clean post-merge.
|
||||
- `.github/workflows/scorecard.yml` — new file. Trigger `schedule: cron: "0 6 * * 1"` (weekly Mondays) + `push: branches: [develop]` + `workflow_dispatch`. Permissions: `id-token: write`, `contents: read`, `security-events: write`. Uses `ossf/scorecard-action@v2.4.0` (or latest at execution time). Upload SARIF to GitHub code-scanning.
|
||||
- `README.md` — add OpenSSF Scorecard badge near the top (under the title or alongside existing badges). Badge URL pattern is the standard `https://api.scorecard.dev/projects/github.com/KofTwentyTwo/Jarvis/badge`.
|
||||
|
||||
**Closes:** #46, #178.
|
||||
|
||||
### PR 3 — "Small code fixes" (`fix/v0.2-wave-1-code-fixes`)
|
||||
|
||||
**Issues:** #37 (STTBackendSelector snake_case vs call-site camelCase) + #47 (WebviewBridge NaN→null JSON guard for `audioLevel`).
|
||||
|
||||
**Why grouped:** both are genuine one-shot code fixes with a unit test each, in different packages (Voice/App + Bus). Landing them together gives one Swift-package-test cycle for both fixes.
|
||||
|
||||
**Deliverables for #37:**
|
||||
|
||||
- Decision: standardise on **camelCase** at call sites (matches existing identifier style elsewhere in the codebase). Update the constants in `STTBackendSelector` to camelCase string values, OR delete the unused constants in favour of an enum if call sites can adopt cheaply. Master agent picks the smaller diff after a quick read.
|
||||
- Single rename or constant-shape change; ensure `App/AppDelegate.swift:2028` (and any other call sites a grep reveals) is consistent.
|
||||
- Existing tests must still pass; if none cover the constant identity, add a small one in `packages/Voice/Tests/` asserting the chosen shape.
|
||||
|
||||
**Deliverables for #47:**
|
||||
|
||||
- First verify #47 is still latent or has become active post-#42: grep for the audioLevel consumer in `webview/packages/hud/` and check whether real decode errors are firing in any logged scenario. Either way, ship the clamp.
|
||||
- `packages/Bus/Sources/Bus/WebviewBridge.swift` — in `sendRaw` (or the `BusOutbound.audioLevel.encode` path, whichever is the cleanest place), replace non-finite `rms` floats with `0.0` before encoding. Inline fix only; do not refactor `AudioLevelEmitter` simultaneously (separate concern).
|
||||
- New unit test: `BusOutboundEncodingTests` (or extend the nearest existing suite) asserts `BusOutbound.audioLevel(rms: .nan)` decodes JS-side with `rms == 0.0`. Use the existing harness convention (`Mock*` only if recording, `Stub*` for canned data, `Fake*` for stateful) per CLAUDE.md.
|
||||
|
||||
**Closes:** #37, #47.
|
||||
|
||||
## Landing order
|
||||
|
||||
1. **PR 2 first** — lowest blast radius, lightest CI footprint, validates the multi-runner pipeline on something we can revert in seconds if it breaks.
|
||||
2. **PR 1 second** — purely descriptive, no behavioural change. CI surface is large (Bus + Memory packages, full webview build), but new tests should not fail because no logic moves.
|
||||
3. **PR 3 last** — real code touches. By the time we get here, the Memory flake is the only plausible failure mode, so if a job goes red we know it is the flake.
|
||||
|
||||
## Memory-flake disposition
|
||||
|
||||
Per `docs/HANDOFF-2026-05-14-multi-runner-shipped.md`: `MemoryWiringEndToEndTests` flakes intermittently on PR branches under concurrent-runner load. Branch protection has zero required checks, so flake-induced red is not blocking.
|
||||
|
||||
Master agent policy:
|
||||
|
||||
1. If a PR's CI fails on Memory only, rerun the failed jobs once.
|
||||
2. If they pass on rerun, merge.
|
||||
3. If they fail a second time, leave the PR open with the Memory failure visible and open `bug,severity:medium,area:memory` issue documenting the test as known-flaky in v0.2. Do not merge without user sign-off.
|
||||
4. If any *non-Memory* check fails, stop and report back — do not rerun, do not merge.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Do not touch PRs #185, #191, #192. They are tracked in a separate disposition.
|
||||
- Do not open or address Wave 2/3 issues (#18, #23/#24, #43, #44, #176, #26, M-series epics).
|
||||
- Do not refactor `AudioLevelEmitter` while fixing #47.
|
||||
- Do not change branch protection settings.
|
||||
- Do not bypass pre-commit hooks (`--no-verify`).
|
||||
- Do not force-push.
|
||||
- Do not rebase #185/#191/#192 onto the new tip of develop.
|
||||
|
||||
## Success criteria
|
||||
|
||||
- Issues #19, #37, #45, #46, #47, #178 all closed via merge.
|
||||
- `develop` HEAD's CI is green after all three merges.
|
||||
- `App/Resources/webview/assets/` contains only the active bundle (one `index-*.js` + `.map` + `.css`) after PR 2 merges.
|
||||
- OpenSSF Scorecard workflow appears in Actions → Workflows and has at least one successful run (or a clearly-named permission failure to address in a follow-up).
|
||||
- Grep for `JarvisBusWorld` outside `.planning/`, `docs/`, and committed history returns nothing.
|
||||
- `swift test --package-path packages/Bus` passes including the new NaN-clamp test.
|
||||
- No new test doubles violate the `Mock*`/`Stub*`/`Fake*` convention.
|
||||
|
||||
## Effort budget
|
||||
|
||||
~90 min of coding across the three PRs, plus CI wait time. Master agent should target completion in 90 min wall clock; if it falls behind, it must report partial progress rather than rush.
|
||||
@@ -43,6 +43,20 @@ public struct CacheHints: Sendable, Equatable {
|
||||
/// Once the system prompt grows past the threshold (e.g. memory
|
||||
/// hydration adds context, presence enrichment lengthens the prompt)
|
||||
/// the hint resumes automatically.
|
||||
///
|
||||
/// **Current production state (as of v0.1):** `selfAwarePreamble` is
|
||||
/// ~700 chars and the orchestrator is constructed with an `.empty`
|
||||
/// initial context, so the threshold is never crossed on a healthy
|
||||
/// boot — `eligibleForSystemPrompt` returns `nil` on every turn and
|
||||
/// the prompt cache is effectively dormant. This is intentional
|
||||
/// cost-acceptance: padding the preamble to ≥4096 chars would
|
||||
/// require real prompt-engineering work and risks bleeding
|
||||
/// irrelevant context into the model. Cache hits resume naturally
|
||||
/// once Memory M-4 (v0.8 milestone) lands `searchFacts`-driven
|
||||
/// hydration that grows the prompt past 1024 tokens organically;
|
||||
/// see #23 for the audit context. Until then, every turn pays full
|
||||
/// input-token cost — a ~30–80% cost reduction is on the table once
|
||||
/// M-4 ships.
|
||||
public static func eligibleForSystemPrompt(
|
||||
_ systemPromptText: String,
|
||||
ttl: CacheTTL = .extended1h
|
||||
|
||||
@@ -508,6 +508,15 @@ public actor AgentOrchestrator {
|
||||
// for the current pass. Reset on each pass restart (initial entry +
|
||||
// T1→T2 escalation re-enter via `continue outer`).
|
||||
var assistantTextSoFar: String = ""
|
||||
// #24: text accumulated since the most recent toolUse append (or
|
||||
// since the start of the round if no toolUse yet). Distinct from
|
||||
// `assistantTextSoFar` which stays cumulative for D-02 vision
|
||||
// routing on endTurn. This buffer feeds the assistant message
|
||||
// history alongside each toolUse block so the model sees its own
|
||||
// narration ("Let me check..." → tool_use(get_time)) on subsequent
|
||||
// round-trips; previously the text was dropped entirely from
|
||||
// history and the model would deny ever saying it.
|
||||
var assistantTextSinceLastFlush: String = ""
|
||||
// D-02: escalation gets ONE shot. The second low-confidence outcome
|
||||
// stays on T1 (the user got the response we have). This mirrors the
|
||||
// AGENT-09 retry budget but is independent of it.
|
||||
@@ -575,6 +584,10 @@ public actor AgentOrchestrator {
|
||||
// can hand it to VisionRouter.evaluatePostResponse.
|
||||
// Reset on `continue outer` from the escalation arm.
|
||||
assistantTextSoFar += s
|
||||
// #24: also accumulate into the per-tool-use flush
|
||||
// buffer so the next toolUseRequested attaches the
|
||||
// preceding narration to its assistant message.
|
||||
assistantTextSinceLastFlush += s
|
||||
await replayLog.record(.textDelta(s), for: currentTurnId)
|
||||
await events.send(.tokenDelta(turnId: currentTurnId, text: s))
|
||||
|
||||
@@ -601,9 +614,29 @@ public actor AgentOrchestrator {
|
||||
// history so providers that require the original
|
||||
// tool_use round-trip (Anthropic) can resolve the
|
||||
// tool_use_id when the next tool_result lands.
|
||||
messages.append(LLMMessage(role: .assistant, content: [
|
||||
.toolUse(id: req.id, name: req.name, argsJSON: req.argsJSON),
|
||||
]))
|
||||
//
|
||||
// #24: bundle any text the model emitted since the
|
||||
// last flush — Opus 4.7 commonly narrates ("Let me
|
||||
// check the time...") before calling a tool. Without
|
||||
// this the text was dropped from history and the
|
||||
// model would deny ever saying it on a follow-up.
|
||||
// Trim whitespace-only text to avoid emitting empty
|
||||
// text blocks (which Anthropic rejects with a 400).
|
||||
var assistantContent: [LLMMessage.ContentBlock] = []
|
||||
let pendingText = assistantTextSinceLastFlush
|
||||
.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
if !pendingText.isEmpty {
|
||||
assistantContent.append(.text(pendingText))
|
||||
}
|
||||
assistantContent.append(
|
||||
.toolUse(id: req.id, name: req.name, argsJSON: req.argsJSON)
|
||||
)
|
||||
messages.append(LLMMessage(role: .assistant, content: assistantContent))
|
||||
// Consumed — the next toolUseRequested in the same
|
||||
// round (rare but possible) gets only the text that
|
||||
// came BETWEEN the two tool_use blocks. D-02's
|
||||
// cumulative `assistantTextSoFar` is untouched.
|
||||
assistantTextSinceLastFlush = ""
|
||||
|
||||
do {
|
||||
let resultData = try await toolDispatcher.dispatch(toolUse: req)
|
||||
|
||||
@@ -0,0 +1,210 @@
|
||||
import XCTest
|
||||
import AgentCore
|
||||
import Config
|
||||
import Replay
|
||||
@testable import AgentOrchestrator
|
||||
|
||||
/// **#24 — assistant text blocks alongside tool_use must not be dropped.**
|
||||
///
|
||||
/// Opus 4.7 frequently narrates before invoking a tool ("Let me check the
|
||||
/// time for you..." → `tool_use(get_time)`). Before this fix, the
|
||||
/// orchestrator appended only the `.toolUse` block to `messages`, so on
|
||||
/// the next round-trip the model had no record of saying anything — a
|
||||
/// user follow-up like "what did you say before checking the time?"
|
||||
/// would get a denial.
|
||||
///
|
||||
/// These tests assert the assistant message at the round-trip boundary
|
||||
/// contains BOTH the preceding text and the tool_use block. D-02's
|
||||
/// cumulative `assistantTextSoFar` is preserved separately; only the
|
||||
/// per-flush buffer is consumed.
|
||||
@MainActor
|
||||
final class AssistantTextWithToolUseTests: XCTestCase {
|
||||
|
||||
var tempHome: TempReplayHome!
|
||||
|
||||
override func setUp() async throws {
|
||||
try await super.setUp()
|
||||
tempHome = TempReplayHome.make()
|
||||
}
|
||||
|
||||
override func tearDown() async throws {
|
||||
tempHome?.cleanup()
|
||||
try await super.tearDown()
|
||||
}
|
||||
|
||||
private func buildOrchestrator(
|
||||
provider: MockLLMProvider,
|
||||
dispatcher: any ToolDispatcher
|
||||
) async throws -> AgentOrchestrator {
|
||||
let replay = try ReplayLog(databaseURL: tempHome.dbURL)
|
||||
let session = try await replay.beginSession(appVersion: "test", buildSHA: "deadbeef")
|
||||
return AgentOrchestrator(
|
||||
configStore: makeConfigStore(),
|
||||
providerFactory: { _ in provider },
|
||||
toolDispatcher: dispatcher,
|
||||
replayLog: replay,
|
||||
sessionId: session,
|
||||
systemPrompt: "you are jarvis",
|
||||
availableTools: []
|
||||
)
|
||||
}
|
||||
|
||||
private func collectUntilIdle(orch: AgentOrchestrator, timeout: TimeInterval = 5.0) async {
|
||||
let deadline = Date().addingTimeInterval(timeout)
|
||||
for await event in orch.events {
|
||||
if case .stateChange(.idle) = event { return }
|
||||
if Date() > deadline { return }
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - ATU-1
|
||||
|
||||
/// Single text-then-tool sequence: model emits "Let me check..." then
|
||||
/// `tool_use(get_time)`. The assistant message in the second provider
|
||||
/// call must contain a `.text` block AND a `.toolUse` block.
|
||||
func test_ATU1_textBeforeToolUseBundledIntoSameAssistantMessage() async throws {
|
||||
let toolReq = ToolUseRequest(id: "tu1", name: "get_time", argsJSON: Data("{}".utf8))
|
||||
let mock = MockLLMProvider(scripts: [
|
||||
.init(events: [
|
||||
.messageStart(LLMMessageStart(messageId: "m1", model: "x", usagePrefix: nil)),
|
||||
.textDelta("Let me check the time for you... "),
|
||||
.toolUseRequested(toolReq),
|
||||
.stopReason(.toolUse),
|
||||
.messageStop,
|
||||
]),
|
||||
.init(events: [
|
||||
.messageStart(LLMMessageStart(messageId: "m2", model: "x", usagePrefix: nil)),
|
||||
.textDelta("It's 12:34."),
|
||||
.stopReason(.endTurn),
|
||||
.messageStop,
|
||||
]),
|
||||
])
|
||||
let dispatcher = StubToolDispatcher(dispatch: { _ in Data("12:34".utf8) })
|
||||
|
||||
let orch = try await buildOrchestrator(provider: mock, dispatcher: dispatcher)
|
||||
_ = await orch.submit(.text("what time"))
|
||||
await collectUntilIdle(orch: orch)
|
||||
|
||||
let calls = await mock.getRecordedCalls()
|
||||
XCTAssertEqual(calls.count, 2, "expected 2 provider calls (tool-use loop)")
|
||||
|
||||
// Find the assistant message containing the tool_use block in the
|
||||
// second call's history.
|
||||
let secondCall = calls[1].messages
|
||||
guard let assistantWithTool = secondCall.first(where: { msg in
|
||||
msg.role == .assistant && msg.content.contains(where: { block in
|
||||
if case .toolUse(id: "tu1", _, _) = block { return true }
|
||||
return false
|
||||
})
|
||||
}) else {
|
||||
return XCTFail("no assistant message with the tool_use block found in second call")
|
||||
}
|
||||
|
||||
// Block ordering: text first, then tool_use.
|
||||
let blockKinds = assistantWithTool.content.map { block -> String in
|
||||
switch block {
|
||||
case .text: return "text"
|
||||
case .toolUse: return "toolUse"
|
||||
case .toolResult: return "toolResult"
|
||||
}
|
||||
}
|
||||
XCTAssertEqual(blockKinds, ["text", "toolUse"],
|
||||
"assistant block sequence must be [text, toolUse] — got \(blockKinds)")
|
||||
|
||||
// The text content matches what the model streamed (trimmed).
|
||||
if case .text(let txt) = assistantWithTool.content[0] {
|
||||
XCTAssertEqual(txt, "Let me check the time for you...",
|
||||
"text block must contain the streamed narration (trimmed)")
|
||||
} else {
|
||||
XCTFail("expected first block to be .text")
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - ATU-2
|
||||
|
||||
/// If the model emits a tool_use with NO preceding text (Opus
|
||||
/// occasionally does this — straight to tool), the assistant message
|
||||
/// must contain ONLY the toolUse block (no empty `.text("")` block,
|
||||
/// which Anthropic rejects with a 400).
|
||||
func test_ATU2_noPrecedingTextOnlyToolUseBlock() async throws {
|
||||
let toolReq = ToolUseRequest(id: "tu2", name: "get_time", argsJSON: Data("{}".utf8))
|
||||
let mock = MockLLMProvider(scripts: [
|
||||
.init(events: [
|
||||
.messageStart(LLMMessageStart(messageId: "m1", model: "x", usagePrefix: nil)),
|
||||
.toolUseRequested(toolReq),
|
||||
.stopReason(.toolUse),
|
||||
.messageStop,
|
||||
]),
|
||||
.init(events: [
|
||||
.messageStart(LLMMessageStart(messageId: "m2", model: "x", usagePrefix: nil)),
|
||||
.textDelta("done"),
|
||||
.stopReason(.endTurn),
|
||||
.messageStop,
|
||||
]),
|
||||
])
|
||||
let dispatcher = StubToolDispatcher(dispatch: { _ in Data("12:34".utf8) })
|
||||
|
||||
let orch = try await buildOrchestrator(provider: mock, dispatcher: dispatcher)
|
||||
_ = await orch.submit(.text("time"))
|
||||
await collectUntilIdle(orch: orch)
|
||||
|
||||
let calls = await mock.getRecordedCalls()
|
||||
guard let assistantWithTool = calls[1].messages.first(where: { msg in
|
||||
msg.role == .assistant && msg.content.contains(where: { block in
|
||||
if case .toolUse(id: "tu2", _, _) = block { return true }
|
||||
return false
|
||||
})
|
||||
}) else {
|
||||
return XCTFail("expected assistant message with tool_use block")
|
||||
}
|
||||
|
||||
XCTAssertEqual(assistantWithTool.content.count, 1,
|
||||
"no preceding text — assistant message must contain only the toolUse block, got \(assistantWithTool.content.count) blocks")
|
||||
if case .toolUse = assistantWithTool.content[0] {
|
||||
// ok
|
||||
} else {
|
||||
XCTFail("expected sole content block to be .toolUse")
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - ATU-3
|
||||
|
||||
/// Whitespace-only preceding text must NOT be emitted as a `.text`
|
||||
/// block (Anthropic rejects empty/whitespace-only text blocks). The
|
||||
/// implementation trims whitespace before testing for emptiness.
|
||||
func test_ATU3_whitespaceOnlyTextNotEmitted() async throws {
|
||||
let toolReq = ToolUseRequest(id: "tu3", name: "get_time", argsJSON: Data("{}".utf8))
|
||||
let mock = MockLLMProvider(scripts: [
|
||||
.init(events: [
|
||||
.messageStart(LLMMessageStart(messageId: "m1", model: "x", usagePrefix: nil)),
|
||||
.textDelta(" \n\t "), // whitespace only
|
||||
.toolUseRequested(toolReq),
|
||||
.stopReason(.toolUse),
|
||||
.messageStop,
|
||||
]),
|
||||
.init(events: [
|
||||
.messageStart(LLMMessageStart(messageId: "m2", model: "x", usagePrefix: nil)),
|
||||
.stopReason(.endTurn),
|
||||
.messageStop,
|
||||
]),
|
||||
])
|
||||
let dispatcher = StubToolDispatcher(dispatch: { _ in Data("ok".utf8) })
|
||||
|
||||
let orch = try await buildOrchestrator(provider: mock, dispatcher: dispatcher)
|
||||
_ = await orch.submit(.text("hi"))
|
||||
await collectUntilIdle(orch: orch)
|
||||
|
||||
let calls = await mock.getRecordedCalls()
|
||||
guard let assistantWithTool = calls[1].messages.first(where: { msg in
|
||||
msg.role == .assistant && msg.content.contains(where: { block in
|
||||
if case .toolUse(id: "tu3", _, _) = block { return true }
|
||||
return false
|
||||
})
|
||||
}) else {
|
||||
return XCTFail("expected assistant message with tool_use block")
|
||||
}
|
||||
|
||||
XCTAssertEqual(assistantWithTool.content.count, 1,
|
||||
"whitespace-only preceding text must be dropped, leaving only the toolUse block")
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
+54
@@ -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)"
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
* `<script type="module" src="…">`), `webkit.messageHandlers.jarvisBus` is
|
||||
* NOT visible — Swift registered the handler `in: JarvisBusWorld` and
|
||||
* WebKit isolates message handlers per content world. However,
|
||||
* `window.jarvisBus` is a property on the (cross-world-shared) `window`
|
||||
* object, so the bus instance installed by `Injection.js` inside
|
||||
* `JarvisBusWorld` IS observable from the default world.
|
||||
* Both `Injection.js` (a `WKUserScript` injected at `.atDocumentStart`) and
|
||||
* this bundle's `installJarvisBus()` run in the SAME page world — the
|
||||
* default `WKContentWorld.page` since `fb41c5f` retired the prior
|
||||
* isolated named world. Injection.js fires first and stamps a
|
||||
* minimal `window.jarvisBus` with `protocolVersion`, `_handler`, plus the
|
||||
* `send` / `receive` / `onOutbound` surface needed for the handshake.
|
||||
* The bundle's installer fires later (top-level side-effect of `main.tsx`),
|
||||
* and if we naively overwrote `window.jarvisBus` we would orphan
|
||||
* Injection.js's queued early messages and any handshake state already
|
||||
* captured against the original closure.
|
||||
*
|
||||
* If we detect a pre-existing `window.jarvisBus` with the expected
|
||||
* `send` + `onOutbound` + `receive` shape, we attach our options to it
|
||||
* and return it verbatim rather than overwriting it. This routes every
|
||||
* outbound `send()` through the JarvisBusWorld-captured message handler
|
||||
* reference and keeps `onOutbound` hooked to the same `_handler` slot the
|
||||
* Injection.js bus exposes. Without this branch the default-world bundle
|
||||
* replaces `window.jarvisBus` with a bus whose `send()` cannot reach
|
||||
* `webkit.messageHandlers.jarvisBus` and every outbound frame — including
|
||||
* the first `uiReady` — is lost.
|
||||
* The shape check below (`isJarvisBusShape`) is therefore a defensive
|
||||
* double-install guard inside one page world, not a cross-world attach.
|
||||
* When we detect a pre-existing `window.jarvisBus` with the canonical
|
||||
* `send` + `onOutbound` + `receive` surface, we return early and leave
|
||||
* Injection.js's instance in place. Outbound `send()` calls still reach
|
||||
* `window.webkit.messageHandlers.jarvisBus` because that handler lives on
|
||||
* the same page world too.
|
||||
*
|
||||
* bus-harness.html (Phase 2) loads its marker script in a page where
|
||||
* `bus-harness.html` (Phase 2) loads its marker script in a page where
|
||||
* Injection.js has installed the same bus; the harness never called `send()`
|
||||
* so the regression wasn't observed until Phase 3. For that path this
|
||||
* function still returns the pre-installed bus, which is exactly what the
|
||||
@@ -93,10 +94,11 @@ export function installJarvisBus(options: InstallOptions = {}): () => void {
|
||||
console.error("[bus] decode failed:", error, raw);
|
||||
});
|
||||
|
||||
// H-02: attach-to-existing path. Injection.js in JarvisBusWorld installs a
|
||||
// minimal bus with the same surface; if it's already on window we MUST NOT
|
||||
// replace it (that would orphan webkit.messageHandlers.jarvisBus, which is
|
||||
// only visible inside JarvisBusWorld).
|
||||
// H-02: defensive double-install guard. Injection.js runs first (at
|
||||
// document-start in the same page world) and stamps a minimal bus with the
|
||||
// same surface; if it's already on `window`, we MUST NOT replace it — doing
|
||||
// so would orphan Injection.js's queued early messages and captured handshake
|
||||
// state. See the class doc above for the full rationale.
|
||||
const existing: unknown =
|
||||
typeof window !== "undefined"
|
||||
? (window as unknown as { jarvisBus?: unknown }).jarvisBus
|
||||
@@ -156,7 +158,7 @@ export function installJarvisBus(options: InstallOptions = {}): () => void {
|
||||
/**
|
||||
* Duck-type guard for a pre-existing `window.jarvisBus`. Matches the shape
|
||||
* installed by `packages/Bus/Sources/Bus/Resources/Injection.js` at
|
||||
* document-start in JarvisBusWorld. Intentionally narrow: if the shape
|
||||
* document-start in the page world. Intentionally narrow: if the shape
|
||||
* doesn't match (e.g. a left-over test stub), we fall through to the
|
||||
* fresh-install branch and overwrite it.
|
||||
*/
|
||||
|
||||
@@ -74,14 +74,13 @@ describe("installJarvisBus", () => {
|
||||
expect(b).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
// H-02: the HUD bundle (`main.tsx`) runs in the default JS world, but
|
||||
// Swift registered `webkit.messageHandlers.jarvisBus` in JarvisBusWorld,
|
||||
// so a default-world `send()` cannot reach the message handler. The
|
||||
// Injection.js-installed bus in JarvisBusWorld CAN reach it (it captured
|
||||
// the reference at document-start inside that world), and because
|
||||
// `window.jarvisBus` is a cross-world-shared property the default world
|
||||
// observes the Injection.js bus. Therefore `installJarvisBus()` must
|
||||
// attach to — not replace — a pre-existing bus.
|
||||
// H-02: both `Injection.js` (`WKUserScript` at document-start) and this
|
||||
// bundle's `installJarvisBus()` run in the SAME page world since `fb41c5f`
|
||||
// retired the prior isolated named content world. Injection.js fires first
|
||||
// and stamps a minimal `window.jarvisBus`; if `installJarvisBus()` naively
|
||||
// overwrote it, the early handshake state and queued messages captured
|
||||
// against the original closure would be orphaned. Therefore the bundle's
|
||||
// installer must attach to — not replace — a pre-existing bus.
|
||||
describe("H-02 attach-to-existing (pre-installed window.jarvisBus)", () => {
|
||||
it("returns the pre-existing bus unchanged when one is already installed", () => {
|
||||
const preExistingSend = vi.fn().mockResolvedValue(undefined);
|
||||
@@ -96,9 +95,10 @@ describe("installJarvisBus", () => {
|
||||
// @ts-expect-error — test shim: seed a pre-installed bus shape matching Injection.js
|
||||
globalThis.window = {
|
||||
jarvisBus: preExisting,
|
||||
// Note: webkit is intentionally absent — in real JarvisBusWorld the
|
||||
// pre-existing bus captured webkit at install time; the default world
|
||||
// never sees webkit. The fix MUST NOT touch the pre-existing bus.
|
||||
// Note: webkit is intentionally absent — Injection.js's pre-existing
|
||||
// bus captures a reference to `window.webkit` at install time, and the
|
||||
// shape guard must trust the pre-existing bus to route through that
|
||||
// captured reference rather than re-resolving `webkit` here.
|
||||
};
|
||||
|
||||
installJarvisBus();
|
||||
@@ -110,7 +110,7 @@ describe("installJarvisBus", () => {
|
||||
expect(window.jarvisBus.onOutbound).toBe(preExistingOnOutbound);
|
||||
});
|
||||
|
||||
it("routes send() through the pre-existing bus's send (reaches JarvisBusWorld-captured handler)", async () => {
|
||||
it("routes send() through the pre-existing bus's send (reaches Injection.js-captured handler)", async () => {
|
||||
const preExistingSend = vi.fn().mockResolvedValue({ ok: true });
|
||||
const preExisting = {
|
||||
protocolVersion: "2.0.0",
|
||||
|
||||
@@ -5,15 +5,15 @@ import { attachBus } from './bus/client'
|
||||
import './theme/tokens.css'
|
||||
|
||||
// `attachBus()` runs in the page's default JS world (this module loads via
|
||||
// `<script type="module" src="…">` in index.html). Swift installed the real
|
||||
// `window.jarvisBus` inside `JarvisBusWorld` via Injection.js — WebKit
|
||||
// content worlds isolate `webkit.messageHandlers`, so the default-world
|
||||
// bundle can't reach them directly. `window.jarvisBus` IS visible though,
|
||||
// because WebKit keeps `window` as one cross-world object (only *variables*
|
||||
// are world-scoped). The H-02 fix in `@jarvis/bus` detects the pre-existing
|
||||
// `window.jarvisBus` and attaches to it instead of replacing it, which
|
||||
// routes every outbound `send()` back through the JarvisBusWorld-captured
|
||||
// message handler reference. See `packages/bus/src/bridge.ts` for details.
|
||||
// `<script type="module" src="…">` in index.html). Swift's
|
||||
// `WebviewBridge.installScriptHandlers` (since `fb41c5f`) registers the
|
||||
// message handler on the same `WKContentWorld.page`, and Injection.js — also
|
||||
// in the page world — has already stamped a minimal `window.jarvisBus` at
|
||||
// document-start. `installJarvisBus()` here detects that pre-existing
|
||||
// instance via a duck-type shape guard and returns early, leaving the
|
||||
// Injection.js bus untouched so its captured handshake state and queued
|
||||
// early messages are preserved. See `packages/bus/src/bridge.ts` for the
|
||||
// full defensive-double-install rationale.
|
||||
attachBus()
|
||||
|
||||
const rootEl = document.getElementById('root')
|
||||
|
||||
Reference in new issue
Block a user