Skip to content

feat(supervise): re-attach bridge sessions for continuity 'resume' - #865

Merged
drewstone merged 2 commits into
mainfrom
feat/chat-transport-resume-executor
Aug 15, 2026
Merged

feat(supervise): re-attach bridge sessions for continuity 'resume'#865
drewstone merged 2 commits into
mainfrom
feat/chat-transport-resume-executor

Conversation

@drewstone

@drewstone drewstone commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Phase 1 of #694: the chat-transport resume executor's bridge arm.

Problem

workerFromBackend refused every continuity: 'resume' spawn. The seam builds a NEW executor per spawn and had no session re-attachment, so accepting a resume would ledger continuity: 'resume' over a brand-new session — the exact false stamp #725 banned. That refusal blocked the revise-edge pattern (write fresh, revise by resuming the writer's session) for every consumer without a custom makeWorkerAgent.

Change

The bridge backend now honors continuity: 'resume' by real session re-attachment.

  • A bridge session id IS the harness conversation key. cli-bridge stores the CLI's own session id against it and passes --resume <id> on the next request with the same key.
  • The seam records the session id each supervised spawn was bound to, keyed by the worker id the Scope assigned. A resume spawn binds the prior worker's recorded session id (spawnContext.resume.ofWorker) instead of deriving a fresh one. A fresh → resume → resume chain stays on ONE conversation.
  • Every other backend keeps failing loud, and the message names the seam and the alternative.
  • Every refusal throws BEFORE a worker exists, so no continuity: 'resume' row is ever bound to a live worker that did not re-attach.

Also fixed here: the derived external session id now scopes by the spawning manager node. Assignment ordinals restart at ordinal:0 under every manager, so the unscoped digest mapped worker 1 of EVERY run — and of every sibling manager — to ONE bridge session id. On a bridge with a persistent session store a 'fresh' spawn then silently continued a foreign run's harness conversation.

Migration: a journaled run from an older version replayed under this version derives DIFFERENT session ids and will not re-attach its old harness sessions. Durable recovery WITHIN a run is unchanged — a replay re-issues the same manager node id and the same assignments.

Proof

Live cli-bridge resume against real claude-code / anthropic haiku. Turn 1 is told a random codeword; turn 2 asks for it back with no restatement. The assertion is on CONTENT, not on absence of error.

codeword: dolphin-08926c91
ledger: traversal=1 outcome=delivered continuity=fresh  workerId=live-resume-proof-1786776212885:s0
ledger: traversal=2 outcome=delivered continuity=resume workerId=live-resume-proof-1786776212885:s1
settled ...:s0: {"content":"stored.","promptCache":{"readTokens":18134}}
settled ...:s1: {"content":"dolphin-08926c91","promptCache":{"readTokens":32822}}
PASS: resumed worker recalled 'dolphin-08926c91' from the prior session turn

A different worker id returned context it was never re-told, and the cache read grew from 18134 to 32822 tokens as the conversation was replayed. That is re-attachment, not a stamp.

Corroborated by cli-bridge's own sessions.sqlite, which keys each external session id to the CLI's internal session id and counts turns:

external_id                        internal      turns  cwd
supervised-worker-610c30de...      e8dfbfe5-a95  6      .../scratchpad/workspace      <- PRE-FIX collision
supervised-worker-223a9970...      4a407475-e2f  1      /tmp/ar-resume-proof-POuumI
supervised-worker-fbc35b7b...      d3038eef-93f  1      /tmp/ar-resume-proof-aLRVFl
supervised-worker-f2b0b9e0...      c381bf7c-f41  2      /tmp/ar-resume-proof-XvMUvB   <- PASSING RESUME

Row 1 is the collision this PR fixes: ONE session id, turns: 6, accumulated across three separate runs that all derived ordinal:0. After the manager-scoping fix three runs produce three DISTINCT session ids. The last row is the resume: two worker spawns, one external session id, one internal claude session id, turns: 2.

Offline tests (5 added, tests/runtime/bridge-executor.test.ts) cover the fresh → resume → resume chain on one session id, manager-scoped isolation of equal assignment ordinals, refusal of a resume for an unrecorded worker, refusal on a resume-incapable backend, and a runGraph resume edge whose ledger stamps are witnessed by the wire.

Test Files  1 passed (1)
      Tests  5 passed | 43 skipped (48)

Gates

gate result
pnpm typecheck exit 0
pnpm run lint 592 files, no fixes applied
pnpm run docs:freshness OK — no drift
pnpm run check:testing-fixture fixtures are current
pnpm test 2704 passed, 6 failed

The 6 failures are host-load flakes, not regressions: all 6 are timeout or claim-window expiries in git-worktree-heavy suites, none import workerFromBackend or the bridge backend, and all 6 files pass serially on the same tree — Test Files 6 passed (6), Tests 72 passed (72).

Known gap, outside this repo

A resumed turn re-materializes the profile workspace, and cli-bridge refuses to replace an existing file: AgentProfile workspace materialization failed: Refusing to replace existing workspace file: CLAUDE.md. So a resume over cli-bridge fails on turn 2 whenever the profile plans workspace files with an unchanged cwd, even though the planned content is byte-identical (same workspacePlanDigest). That is a cli-bridge / agent-profile-materialize idempotency gap, not a Runtime one — Runtime correctly re-sends the profile as each turn's behavioral authority. Tracked for cli-bridge; the live proof above uses a profile that plans no workspace files.

Refs #694

workerFromBackend honors `continuity: 'resume'` on the bridge backend by session
re-attachment. A bridge session id is the harness conversation key, so a resume
spawn binds the prior worker's recorded session id instead of deriving a new one.
A fresh to resume to resume chain stays on one harness conversation.

Every other backend continues to refuse a resume spawn. Each refusal throws
before a worker exists, so the kernel never ledgers `continuity: 'resume'` over a
session that was not re-attached.

Scope the derived external session id by the spawning manager node. Assignment
ordinals restart at `ordinal:0` under every manager, so an unscoped digest mapped
worker 1 of every run to one bridge session id.

Refs #694

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 417622c4

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-15T06:49:47Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 417622c4

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-15T06:51:40Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 2 (2 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 257.5s (2 bridge agents)
Total 257.5s

💰 Value — sound-with-nits

Makes the default worker seam honor continuity 'resume' for the bridge backend by re-binding the prior worker's bridge session id (plus fixes a real cross-run session-id collision), implemented exactly in the grain the kernel's continuity contract documents; ship.

  • What it does: workerFromBackend now keeps a process-local map of Scope-assigned worker id → bound bridge session id (recorded in executorFactory at supervise.ts:202-209) and, on a 'resume' spawn, resolves the prior worker's recorded session via bridgeResumeSessionId (supervise.ts:241-271) and binds it with the existing bindReusableExecutorExecutionId instead of deriving a fresh one — so cli-bridge continues one
  • Goals it achieves: Unblocks the revise-edge pattern (write fresh, revise by resuming the writer's session) for every consumer that uses the default seam with a bridge backend, instead of requiring a custom makeWorkerAgent. Keeps the continuity ledger truthful per the #725 ban: a 'resume' stamp can only appear over a session that was actually re-attached, and every refusal throws before a worker exists. Fixes a real
  • Assessment: Good and in-grain. The codebase's own contract (coordination.ts:201-204) says the kernel owns identity/ordering/ledger and 'the seam owns the re-attachment itself — e.g. mapping ofWorker to a backend session id'; this PR is a literal implementation of that sketch for the bridge case. It reuses two existing mechanisms rather than inventing new ones: the bridge's session-id-keyed resume (runtime.ts:
  • Better / existing approach: none — this is the right approach. I searched for prior art: chatWorkerSeam/chat-transport-executor.ts:180-212 is the existing resume-capable seam but for router-tools, storing full message arrays (ChatSessionStore) because that backend is stateless — the bridge case needs only a session-id string, so a shared store abstraction would be over-engineering, not reuse. A transport-level '--resume' fla
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound-with-nits

This PR wires the already-shipped kernel continuity model (spawn_tool continuity, continuityByProfile, graph delegates-edge 'resume') into the one seam that refused it, using the bridge session id exactly as the existing execution-binding machinery intended — plus a real cross-run session-id collisi

  • Integration: Reachable immediately through both public entry points: runGraph defaults to workerFromBackend (src/runtime/supervise/graph.ts:615) and supervise builds its leaf seam from it (src/runtime/supervise/supervise.ts:1701). The consumer surface already exists in src: graph edges declare continuity:'resume' as data (graph.ts:104-112, threaded at 756-758), the spawn tool resolves resume lineage with ofWor
  • Fit with existing patterns: Follows the codebase's grain rather than inventing a mechanism: bindReusableExecutorExecutionId was built precisely to give stateful bridge backends a reconnectable external id (runtime.ts:4510-4525), the bridge executor documents session_id as the harness resume key and puts it on every turn POST (runtime.ts:1883-1885, 2390), and fail-loud-before-worker-exists matches the #725 false-stamp ban the
  • Real-world viability: Error paths behave loud and clean: unknown ofWorker, missing lineage, and non-bridge backends all throw ValidationError with actionable alternatives before any worker or ledger row exists (supervise.ts:241-271); the resume-while-live fence (coordination.ts:1129-1137) serializes per node, so two live workers cannot interleave turns on one harness conversation via resume. The session record is writt
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 Stale export comment now misstates workerFromBackend's resume role [maintenance] ``

src/runtime/index.ts:549-552 introduces the chat-transport exports with '(the resume consumer workerFromBackend refuses to be)'. After this PR workerFromBackend IS a resume consumer for the bridge backend (supervise.ts:145-153), so the parenthetical contradicts the new behavior. One-line doc fix in the same file; does not gate shipping.

🎯 Usefulness Audit

🟡 Stale export doc contradicts the new capability [ergonomics] ``

src/runtime/index.ts:549-552 still describes workerFromBackend as 'the resume consumer workerFromBackend refuses to be' when justifying chatWorkerSeam. After this PR workerFromBackend IS a resume consumer for bridge backends, so the comment on the public export surface now misdescribes the seam it contrasts with and could steer an adopter to the chat leaf unnecessarily. One-line doc fix; does not gate shipping.


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260815T070220Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 2 (2 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 367.0s (2 bridge agents)
Total 367.0s

💰 Value — sound

Implements the kernel's documented per-seam continuity-'resume' contract for the bridge backend via a process-local worker-id→session-id map, plus a real cross-run session-collision fix — exactly where the codebase says re-attachment belongs, with the ledger-truth invariant enforced by construction.

  • What it does: workerFromBackend (src/runtime/supervise/supervise.ts:155) previously refused every continuity:'resume' spawn. Now: (1) for the bridge backend it records the bridge session id each spawn was bound to, keyed by the Scope-assigned worker id (supervise.ts:207-209), and a resume spawn re-binds the prior settled worker's recorded session id via spawnContext.resume.ofWorker (bridgeResumeSessionId, super
  • Goals it achieves: First, unblock the revise-edge pattern (write fresh, revise by resuming the writer's session) for every bridge consumer that doesn't hand-roll a makeWorkerAgent — the bridge session id IS the harness conversation key (runtime.ts:323-326 documents cli-bridge resume off sessionId), so binding the prior id is real re-attachment, not a stamp. Second, keep the continuity ledger truthful (#725's ban on
  • Assessment: Good and in-grain. The kernel contract explicitly assigns re-attachment to the executor seam — 'the seam owns the re-attachment itself — e.g. mapping ofWorker to a backend session id' (coordination.ts:201-204) — and this change implements precisely that, at the seam the kernel's spawnContext reaches. The process-local Map matches the kernel's OWN resume boundary: coordination.ts:1062-1066 document
  • Better / existing approach: none — this is the right approach. Searched for alternatives: (a) durable session mapping via the spawn journal would exceed the kernel's process-local resume boundary (the kernel itself cannot resolve a cross-process ofWorker), so it would add durability nothing can consume; (b) deriving resume session ids deterministically from node identity instead of a Map would conflate fresh siblings — a fre
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound-with-nits

The bridge arm of the kernel's already-landed resume contract, built exactly in the grain of the existing chatWorkerSeam pattern and verified end-to-end (ledger stamp + wire session id) — it unblocks runGraph/supervise resume over cli-bridge without a hand-rolled seam, with one narrow fail-loud

  • Integration: Fully reachable now. The kernel side already exists: resolveContinuity stamps resume: { ofWorker, sequence } (src/mcp/tools/coordination.ts:1103-1156, threaded into WorkerSpawnContext at :1868-1878), and runGraph delegates edges already declare continuity: 'resume' (src/runtime/supervise/graph.ts:104-112, :756-758). Before this PR every one of those spawns hit workerFromBackend's blank
  • Fit with existing patterns: Matches the established pattern rather than competing: chatWorkerSeam (src/runtime/supervise/chat-transport-executor.ts:180-211) is the identical shape — session store keyed by worker nodeId, resume via spawnContext.resume.ofWorker — for the router-tools arm; this is the bridge arm of the same MakeWorkerAgent contract, which the type docs explicitly assign to the seam ('mapping ofWorker
  • Real-world viability: Holds on the main paths: the seam instance (and its session map) is created once per run for both the graph path (graph.ts:615) and plain supervise({ backend }) (supervise.ts:1701), so fresh→resume→resume chains stay on one conversation, and the process-local map matches the kernel's documented process-local resume boundary (coordination.ts:1062-1066). One narrow edge: when `opts.resolveDelivera
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

💰 Value Audit

🟡 bridgeSessionByWorker map is never pruned [maintenance] ``

The Map at supervise.ts:163 grows monotonically for the life of the workerFromBackend closure (entries survive worker settle/teardown). Bounded by per-process spawn count and holding two short strings per entry, so negligible in practice — a note only. If it ever mattered, prune in the same place the kernel settles a worker; not worth doing now.

🎯 Usefulness Audit

🟡 resolveDeliverable forks workerFromBackend per spawn, silently losing bridge resume [robustness] ``

supervise.ts:1788-1791 builds a new workerFromBackend instance (with a fresh empty bridgeSessionByWorker) whenever resolveDeliverable returns a deliverable different from the run-wide one, while the shared instance at supervise.ts:1701 (and graph.ts:615) holds the recorded sessions. A continuity: 'resume' spawn on that path always throws 'no recorded bridge session for worker ... this seam re-attaches only sessions it bound in this process' even when the prior worker ran and settled in-p


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260815T071931Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 417622c4

Review health 100/100 · Reviewer score 70/100 · Confidence 85/100 · 15 findings (2 medium, 13 low)

opencode GLM 5.2 opencode DeepSeek v4 Pro opencode DeepSeek v4 Flash aggregate
Readiness 70 82 74 70
Confidence 85 85 85 85
Correctness 70 82 74 70
Security 70 82 74 70
Testing 70 82 74 70
Architecture 70 82 74 70

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 5/5 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 9 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM resolveDeliverable + continuity:'resume' breaks: per-spawn workerFromBackend instance discards the bridge session map — src/runtime/supervise/supervise.ts

workerFromBackend keeps bridgeSessionByWorker as a closure-local Map (supervise.ts:163). When opts.resolveDeliverable returns a leaf deliverable, makeSelectedLeaf builds a fresh workerFromBackend(options.backend, leafDeliverable) PER SPAWN (lines 1788-1791; captureDeliverable always returns a new object so leafDeliverable === deliverable is never true in that branch). The prior worker's session was recorded in that spawn's discarded instance, so a later kernel-authorized resume of the node (resolveContinuity passes: the settled worker is in the ledger) reaches bridgeResumeSessionId with an empty map and throws "no recorded bridge session for worker

🟠 MEDIUM resolveDeliverable mints a fresh closure, dropping the bridge session record across a fresh→resume chain — src/runtime/supervise/supervise.ts

bridgeSessionByWorker is declared per workerFromBackend closure (line 163). Recording happens in the same closure (lines 207-208) and resume lookup reads that closure's map (line 262 via bridgeResumeSessionId). But supervise() creates a NEW workerFromBackend closure for every spawn whose resolveDeliverable returns a custom deliverable: [lines 1788-1791](https://github.com/tangle-ne

🟡 LOW Changelog skips 0.134.5 through 0.134.9 — CHANGELOG.md

Pre-base package.json was 0.134.9 but the changelog jumps from 0.135.0 straight to 0.134.4 (line 16); five released versions have no entries. Not introduced by this PR (verified via git show of the base), but the new 0.135.0 entry makes the gap adjacent and visible. Fix opportunistically: backfill entries for 0.134.5–0.134.9 or note the gap, so the migration warning culture this entry establishes is not undercut by missing history.

🟡 LOW Live-proof numbers in FIX paragraph are not reproducible in-repo — CHANGELOG.md

The FIX bullet cites a live measurement ('measured live: three separate runs shared one claude conversation, turns: 6') as evidence for the parentNodeId-scoping fix. The code fix and its unit test are present and correct, but the empirical claim has no reproduction script, run artifact, or PR link in the entry. Suggest citing the reproducing test (tests/runtime/bridge-executor.test.ts) or a run artifact so the claim is checkable; impact is documentation-only.

🟡 LOW API doc omits the session-id derivation migration note carried by the CHANGELOG — docs/api/runtime.md

Fresh bridge spawns now derive their external session id scoped by spawnContext.parentNodeId (supervise.ts:182-189), so a journaled run from a version before this commit replays under the new derivation and produces DIFFERENT session ids that will not re-attach its old harness sessions. The CHANGELOG (0.135.0) documents this migration caveat explicitly, but the runtime.md paragraph (which mirrors the source docstring) does not. Not wrong information — only an omission a doc-only reader would miss. Suggest adding one sentence to the docstring about the scope-qualified derivation so the generated API doc carries the caveat.

🟡 LOW Doc overstates session recording for non-Scope direct callers — docs/api/runtime.md

The paragraph says the seam 'records the session id each supervised spawn was bound to'. In source, recording is conditional on ctx.node?.nodeId !== undefined (src/runtime/supervise/supervise.ts:201-203); a direct caller that omits the Scope node context builds a bridge executor whose session is never recorded, and a later resume of that worker's id throws 'no recorded bridge session'. The following clause 'keyed by the worker id the Scope assigned' partially narrows the claim, and the fail-loud path is documented, so this is a precision nit mirrored from the source docstring rather than a code bug. Optional: qualify the sentence as 'each Scope-assigned supervised spawn'.

🟡 LOW Cross-version session re-derivation breaks re-attachment for live journaled runs — src/runtime/supervise/supervise.ts

Adding scope: parentNodeId to the externalExecutionId digest changes the derived bridge session id for EVERY fresh bridge worker, not just resumes. A journaled run started under the prior version and replayed under this one derives different session ids and will not re-attach its old harness conversations (CHANGELOG acknowledges this as a migration caveat). The new derivation is correct and the old one was buggy (foreign-run continuation), so this is an acceptable, documented breaking change — but worth flagging so any product holding long-lived durable runs plans the replay window. Could be mitigated by keying the scope to a run identity that is stable across versions, at the cost of reintroducing some collisions.

🟡 LOW bridgeSessionByWorker map grows without bound — src/runtime/supervise/supervise.ts

bridgeSessionByWorker is never pruned: one entry per bridge worker spawned in the process, keyed by node id, retained forever. A caller that constructs one workerFromBackend seam and reuses it across many supervise() runs (or a long-lived server) leaks one small (string,string) entry per worker. Bounded by total workers per seam instance, so low impact, but an explicit Map eviction policy (drop on settle/teardown) or a documented per-run lifetime would match the codebase's conservative memory posture. Note eviction must keep entries for workers referenced by resume.ofWorker until the resume chain is done.

🟡 LOW bridgeSessionByWorker never evicts settled workers — src/runtime/supervise/supervise.ts

Entries are added per spawned bridge worker and never deleted; a long-lived workerFromBackend closure (one per supervise() call, shared tree-wide) grows one ~150-byte entry per worker forever. Negligible at realistic scales, but pruning on settle (or capping to the node's latest chain entry) would keep long runs tidy. Nit only.

🟡 LOW New describe's afterEach resets fewer captured globals than the sibling describe — tests/runtime/bridge-executor.test.ts

The file-level mocks also capture lastBridgeUrl, lastBridgePostHeaders, and activeBridgePayload; the first describe's afterEach clears the first two while the new one clears only bridgeHttpHandler. Harmless today (the new tests never read those), but a later test in this file that does read them could see stale values from a prior failing test. Cosmetic consistency nit only.

🟡 LOW No wire-level test that a fresh spawn after a resume gets a new session id — tests/runtime/bridge-executor.test.ts

The chain test covers fresh→resume→resume, but never a resume→fresh sequence. The implementation derives a fresh session only when bridgeResumeSessionId returns undefined (supervise.ts:190-197); a regression that reuses bridgeSessionByWorker for fresh spawns would pass every existing test in this file. Add a spawn with continuity:'fresh' and a distinct ordinal after the resume chain and assert its session_id differs from sessions[0].

🟡 LOW Replay durability of derived session ids is documented but untested — tests/runtime/bridge-executor.test.ts

supervise.ts:186-188 claims a replay of the same run re-issues the same manager node id and assignments and therefore derives the same session id (durable recovery). The isolation test proves divergence across managers, but the convergence-under-replay half of the contract has no test: same parentNodeId + same assignmentId across two workerFromBackend products should yield identical session_id. Without it, a change to canonicalCandidateDigest inputs silently breaks replay recovery.

🟡 LOW Resume wire test cannot prove server-side re-attachment — tests/runtime/bridge-executor.test.ts

The new resume tests assert re-attachment on the REQUEST payload: the mock bridgeHttpHandler returns the same sse('ok',1,2) for every session id, so the suite proves the runtime sends the prior session_id on the wire but cannot prove cli-bridge actually continues the same harness conversation (versus opening a new one for a reused id). The seam's contract — 'a bridge session id IS the harness conversation key' — is reasonable and documented, but it is asserted, not verified. Acceptable for a runtime unit suite; a live bridge proof (the PR's own changelog cites a real measurement) is the right venue for server-side confirmation.

🟡 LOW Spawn context is cast as never, hiding field drift at typecheck — tests/runtime/bridge-executor.test.ts

spawnCtx('ordinal:0', { continuity: 'fresh' }) is passed as context as never (lines 1475, 1505, 1523, 1540) and assertions read request.session_id off Record<string, unknown>. If WorkerSpawnContext renames parentNodeId, continuity, or resume.ofWorker, these tests still compile and fail only at runtime. Not a correctness bug today — the behavioral assertions (identical session ids, distinct ordinal sessions, throw messages) would still detect the semantic regressions, and tsc/biome/tests all pass — but typing the context as WorkerSpawnContext instead of never would make the tests a compile-time contract on the public spawn shape. Cosmet

🟡 LOW afterEach resets only bridgeHttpHandler, not the other module-level mock state — tests/runtime/bridge-executor.test.ts

The new describe's afterEach sets only bridgeHttpHandler = null (line 1433-1435), while every other describe in the file also resets activeBridgePayload, lastBridgeUrl, lastBridgePostHeaders (lines 202-206). Inert today: this is the last describe in the file, tests run sequentially, and each new test assigns bridgeHttpHandler before any POST. But the asymmetry leaves activeBridgePayload/lastBridgeUrl/lastBridgePostHeaders stale if a future test in or after this block reads them before the first POST, an


tangletools · 2026-08-15T07:38:15Z · trace

tangletools
tangletools previously approved these changes Aug 15, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 15 non-blocking findings — 417622c4

Full multi-shot audit completed 5/5 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 9 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-08-15T07:38:15Z · immutable trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Coverage 2 of 2 lenses (value, usefulness)
Concerns 1 (1 weak-concern)
Heuristic 0.0s
Duplication 0.1s
Interrogation 467.2s (2 bridge agents)
Total 467.3s

💰 Value — sound

Makes the bridge backend honor continuity:'resume' by re-attaching the prior worker's cli-bridge session id (and fixes a real cross-run session-id collision), exactly as the kernel's documented seam contract prescribes — ship.

  • What it does: workerFromBackend (src/runtime/supervise/supervise.ts:155) previously threw on every continuity:'resume' spawn. Now, for backend:'bridge' only, it keeps a closure-scoped map of Scope-assigned worker id → bound bridge session id (recorded in executorFactory at supervise.ts:207-209) and a resume spawn re-binds the prior worker's recorded session via bridgeResumeSessionId (supervise.ts:241-271), so a
  • Goals it achieves: Unblocks the revise-edge pattern (write fresh, then revise by resuming the writer's session) for every supervise()/runGraph consumer that passes a bridge backend, without forcing a custom makeWorkerAgent. Keeps the continuity ledger truthful (#725's ban on false stamps): a 'resume' row can now only appear when the wire actually re-attached a session. Fixes the latent cross-run/cross-manager sessio
  • Assessment: Good on its merits. (1) It implements exactly the contract the kernel already documents: 'the seam owns the actual session re-attachment — e.g. mapping ofWorker to a backend session id' (src/mcp/tools/coordination.ts:201-204), and the process-local map matches the kernel's own documented resume boundary — 'All three reads are process-local by construction… a prior process's workers are not resume
  • Better / existing approach: none — this is the right approach. Searched for an existing worker→session mapping (rg 'ofWorker', 'sessionId', 'session_id' across src/): the only prior resume consumer is chatWorkerSeam (chat-transport-executor.ts), a different backend with a different value type (message arrays vs session ids) — no reuse possible without a speculative shared abstraction. Considered and rejected alternatives: (a
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error

🎯 Usefulness — sound-with-nits

Bridge session re-attachment for continuity 'resume' fills the real gap that kept the kernel's established resume contract off the most production-real backend, follows the existing chatWorkerSeam pattern exactly, and ships a measured cross-run session-id collision fix — with one residual default-ru

  • Integration: Directly reachable and immediately useful. workerFromBackend is THE default worker seam for supervise({backend}) (src/runtime/supervise/supervise.ts:1701) and runGraph({backend}) (src/runtime/supervise/graph.ts:615), exported from the main entry (src/runtime/index.ts:818). The kernel side already exists: continuityByProfile and per-call spawn_agent continuity resolve resume lineage and stamp spawn
  • Fit with existing patterns: In the grain. It mirrors chatWorkerSeam (src/runtime/supervise/chat-transport-executor.ts:180-212) — the codebase's established resume consumer for the chat-transport arm: session record keyed by kernel worker/node id, resume.ofWorker re-attach, fail loud when absent, process-local boundary. This is the bridge arm of the same kernel contract, not a competing design: a bridge session id IS the harn
  • Real-world viability: Holds beyond the happy path. Id spaces verified aligned: the record key ctx.node.nodeId (scope.ts:657) is the same id the kernel later returns as resume.ofWorker from the settled ledger (coordination.ts:1139-1154). Resume only targets SETTLED workers (resume-while-live refused at coordination.ts:1129-1138), and a settled worker necessarily had executorFactory invoked, which records the session bef
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🎯 Usefulness Audit

🟡 Session-id collision fix still collides across processes using default runIds on a persistent bridge store [robustness] ``

The derived session id is digest({assignmentId, scope: parentNodeId}) where parentNodeId is the scope root, i.e. the plain runId (supervisor.ts:636-637). Defaults are not process-unique: supervise() defaults to the constant 'supervise' (supervise.ts:1561) and runGraph defaults to a digest of the graph's node ids (graph.ts:571-573). Two ephemeral runs of the same shape against one cli-bridge with a persistent session store therefore still derive identical session ids for ordinal:0 — the exact sil


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260815T074917Z

@tangletools

Copy link
Copy Markdown
Contributor

⚠️ Review Incomplete — 417622c4

At least one required reviewer lane failed closed. No approval or request-changes review was published. This is a reviewer run failure, not a PR quality score.

Trigger a fresh review on the current PR head.

tangletools · 2026-08-15T08:16:13Z

@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — a0b5d5aa

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.

tangletools · auto-approval · reason: drewstone_author · 2026-08-15T08:51:50Z

@drewstone
drewstone merged commit 6fd900f into main Aug 15, 2026
4 checks passed
@drewstone
drewstone deleted the feat/chat-transport-resume-executor branch August 15, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants