Skip to content

feat(supervise): add recursive product control and durable identity - #657

Open
drewstone wants to merge 13 commits into
mainfrom
fix/supervise-root-handle-export
Open

feat(supervise): add recursive product control and durable identity#657
drewstone wants to merge 13 commits into
mainfrom
fix/supervise-root-handle-export

Conversation

@drewstone

@drewstone drewstone commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Run root and nested supervisors from complete AgentProfiles while binding product tools, spawn/message authorization, and awaited coordination transactions to trusted node identity.
  • Preserve durable execution, materialization, trace, accounting, assignment, and nested-tree evidence across recursive work and same-host restart.
  • Export createRootHandle from /kernel so products can inspect, steer, and cancel the live root through the documented public API.
  • Give every product-owned supervisor tool a frozen invocation context with the manager scope live AbortSignal, so caller, root, deadline, breaker, and recursive cancellation reach both router and external-MCP handlers.

Verification

  • pnpm test — 2016 passed, 6 skipped across 188 passing and 2 skipped files.
  • Cancellation-focused supervisor tests — 8/8 passed; router, external MCP, and recursive root-abort paths are covered.
  • pnpm run typecheck — passed, including examples.
  • pnpm run lint — 524 files checked.
  • pnpm run docs:api and pnpm run docs:check — passed with generated docs stable.
  • pnpm run verify:package — passed, including build, package fixtures, static imports, export checks, and edge-safety checks.
  • Fresh origin/main merge-tree — clean.

Known limits

  • Product-tool cancellation is cooperative: handlers must observe or pass context.signal; JavaScript cannot forcibly interrupt an arbitrary in-process promise.
  • Same-host restart restores committed work and records; it does not reattach a process that died in flight.
  • Live steering uses rootHandle.deliver(...) and cancellation uses abort(...); RootSignal pause/resume/ask remain non-operative legacy variants.
  • A remote sandbox manager still needs an explicit reachable coordination relay or tunnel.

tangletools
tangletools previously approved these changes Jul 29, 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.

✅ Auto-approved drewstone PR — 74b75602

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-29T19:08:15Z

@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

Verdict sound
Concerns 2 (1 low, 1 weak-concern)
Heuristic 2.0s
Duplication 0.0s
Interrogation 260.0s (2 bridge agents)
Total 262.0s

💰 Value — sound

A coherent, grain-aligned unification: the supervisor becomes a full AgentProfile (killing the narrow special-case type), and every executed node gains durable content-addressed identity, materialization receipts, live root control, structured traces, and product authorization — completing patterns

  • What it does: This PR (117 files, ~20.5k LOC, 0.109.2 → 0.110.0) does five concrete things. (1) SupervisorProfile collapses from a hand-rolled subset type ({ name, harness, model, systemPrompt }) into type SupervisorProfile = AgentProfile; the supervisor's brain now resolves from profile.harness exactly like a worker's executor (src/runtime/supervise/supervisor-agent.ts:69, `examples/supervise/supervi
  • Goals it achieves: Read from the code, the goal is to make the supervisor a first-class recursive agent — same profile, same materialization rule, same authorization surface as any worker — so product authority and traces can join a node to its backend across processes and hosts. This kills the parallel type surface (narrow SupervisorProfile vs full AgentProfile) that CLAUDE.md §1.5 explicitly outlaws, and it cl
  • Assessment: Sound and in the grain of the codebase. The unification is exactly what CLAUDE.md §1.5 (the 'AgentProfile law') prescribes — the supervisor stops being special. The materialization contracts are the structural counterpart to the already-shipped validateAgentProfileSecurity (security axes vs behavioral axes, same fail-loud-before-compute shape). The durable identity events are additive kinds on t
  • Better / existing approach: Searched for an existing equivalent or a simpler design before answering. (a) createRootHandle and the attach machinery already existed in src/runtime/supervise/supervisor.ts on main (line 356 in the pre-PR version) — this PR extends them (adds deliver, lease model, exports from /kernel), does not reinvent them. (b) validateAgentProfileSecurity already existed for security axes — the n
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

Makes the supervisor a complete AgentProfile (the codebase's stated 'agent = full profile' law), exports the root control handle for live steering, and adds durable identity for same-host restart — all wired through the central supervise() front door with fail-closed materialization gating.

  • Integration: Every new symbol has a caller. createRootHandle existed internally and is now exported from /kernel (src/runtime/index.ts:712); supervise() plumbs it via the rootHandle option (src/runtime/supervise/supervise.ts:1386) and attach() binds it to the live run. supervisorAgent() is constructed in exactly two places — root (supervise.ts:1341) and nested (supervise.ts:1281) — no competing construction. T
  • Fit with existing patterns: Fits the grain exactly. CLAUDE.md §1.5 states 'an agent IS its full profile (prompt+skills+tools+mcp+subagents+hooks); you change behavior by AUTHORING the profile and letting the sandbox substrate materialize it — never write a verify-loop or harness-specific config.' Making SupervisorProfile a plain AgentProfile (supervisor-agent.ts:71) is the direct realization of that law. The root handle's de
  • Real-world viability: Robustness is addressed beyond the happy path. The root handle moved from bind/unbind to an acquire/bind/release lease (supervisor.ts:775-803) so early-failure teardown releases cleanly (finally blocks at supervisor.ts:692-694). Resume path validates budget + identity + materialization match before continuing (supervisor.ts:132-171). New tests cover global concurrency (supervise-global-concurrency
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added src/runtime/supervise/runtime.ts

  •            seam.checkTimeoutMs ?? seam.harnessTimeoutMs ?? bridge.timeoutMs ?? 5 * 60 * 1000,
    

💰 Value Audit

🟡 Local spend helpers duplicated across 4+ files [duplication] ``

zeroSpend is defined in src/runtime/supervise/{driver-executor.ts:417, runtime.ts:260, scope.ts:2106} and src/durable/spawn-journal.ts:845; addSpend in driver-executor.ts:421 and supervisor.ts:1086; isNonZeroSpend/isNonEmptySpend (same shape, two names) in driver-executor.ts:435 and supervisor.ts:1100. Each is 3-15 lines and this PR added more copies. The canonical spendFromUsageEvents already lives in src/runtime/supervise/budget.ts; these small helpers could be co-located there (or i


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 · 20260729T191254Z

@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 — 162fea5a

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.

tangletools · auto-approval · reason: drewstone_author · 2026-07-29T19:27:58Z

@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
Concerns 2 (1 low, 1 weak-concern)
Heuristic 2.2s
Duplication 0.0s
Interrogation 172.9s (2 bridge agents)
Total 175.1s

💰 Value — sound-with-nits

Upgrades the recursive supervisor into a product-control surface with trusted per-node identity, authorization hooks, durable/honest restart accounting, and provable execution-plan receipts — coherent and in the codebase's grain; one minor dead-interface wart.

  • What it does: Makes supervise() enforce and audit the repo's §1.5 'author the profile, the substrate materializes it' law. Every spawned node now carries a content-addressed NodeExecutionIdentity (profile+task+candidate digests via the shared canonicalCandidateDigest), a ProfileMaterializationReceipt (exact effective profile/backend/model/plan vs. what was authored), and per-attempt ExecutionBindingReceipts
  • Goals it achieves: Turn agent-authored recursive supervision into a trustworthy, auditable, product-controllable execution path: (1) model-authored metadata no longer decides driver-vs-leaf or spawn identity in isolation; (2) restart can't mint budget, slide deadlines, or silently treat unknown telemetry as zero; (3) a product can observe, steer, and cancel the live root and nested managers through one handle; (4) '
  • Assessment: Strong, and firmly in the grain. It extends — not forks — the existing seams: the nested-scope seam key is reused (now with partitioned child pools), contentAddress was extracted to src/durable/content-address.ts and shared by trace-evidence, the profile-materialization contract system is broadened (sandboxActProfileMaterialization now derives from fullProfileMaterialization), and candidat
  • Better / existing approach: none — this is the right approach. I checked for an existing equivalent to reuse: contentAddress is shared (not reimplemented), canonicalCandidateDigest/agentProfileSchema come from @tangle-network/agent-interface, the recursive driver-executor and nested-scope seam are extended in place, and loadSpawnForest is genuinely new (the old replaySpawnTree read only one tree). No partial dupl
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 2
  • Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound

A coherent, fully-wired capability that runs root and nested supervisors from complete AgentProfiles with durable identity and live root control — integrated into the main supervise() entry point and exercised by examples and ~8.6k lines of tests.

  • Integration: Fully reachable. supervise() (supervise.ts:1343) is the primary consumer: it calls supervisorAgent() for root and nested supervisors, threads rootHandle via supervisor.attach() (supervise.ts:1388), wires authorizeDownMessage, and validates per-backend materialization contracts. createRootHandle is exported from /kernel (runtime/index.ts:712) and exposed as the rootHandle option on SuperviseOptions
  • Fit with existing patterns: Follows the codebase grain precisely. supervisorAgent resolves the brain from profile.harness exactly mirroring createExecutor({backend}) resolving a worker (supervisor-agent.ts:495). The materialization contracts generalize the pre-existing per-path declaration pattern (sandboxActProfileMaterialization now aliases fullProfileMaterialization). Durable identity (rootIdentity, NodeExecutionIdentity,
  • Real-world viability: Holds up under realistic use. A lease prevents two concurrent runs from racing one handle (supervisor.ts:776-803); the handle fails loud when unbound. The deadline timer is armed from an absolute clock captured before any async journal work, so a slow begin cannot extend the run on restart (supervisor.ts: runStartedAtMs). The resume contract asserts identity/budget/materialization match before reh
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added src/runtime/supervise/runtime.ts

  •            seam.checkTimeoutMs ?? seam.harnessTimeoutMs ?? bridge.timeoutMs ?? 5 * 60 * 1000,
    

💰 Value Audit

🟡 Dead RootSignal (pause/resume/ask) surface coexists with the operative deliver()/abort() on one public handle [maintenance] ``

The PR adds the real steering verb SteerableRootHandle.deliver() and keeps abort(), but RootHandle.signal(msg: RootSignal) plus the pause/resume/ask variants remain on the public type with no handler anywhere (grep for case 'pause'|'resume'|'ask' in src/runtime/supervise/ returns nothing). The PR body's 'Known limits' acknowledges these are 'non-operative legacy variants,' yet the interface still advertises them to callers. Two control surfaces on one handle, only one operative, is


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 · 20260729T193107Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 162fea5a

Review health 100/100 · Reviewer score 14/100 · Confidence 95/100 · 20 findings (3 medium, 17 low)

glm: Correctness 14 · Security 14 · Testing 14 · Architecture 14

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

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

🟠 MEDIUM Legacy coordination-log answer records no longer fold to 'answered' on resume — src/runtime/supervise/coordination-log.ts

Answer folding now requires ev.down.delivered === true: if (prior && ev.down.delivered). Legacy records (written before this PR) lack the delivered field, so ev.down.delivered is undefined (falsy). Result: questions that were answered in a prior process reload as 'open' on resume, potentially re-blocking the resumed driver's stop policy. This is the correct conservative choice (can't prove delivery without the field) but silently changes behavior for existing durable runs. Impact: a resumed durable run with legacy answer records may see previously-resolved questions as re-blocking. Fix: consider treating undefined delivered as true for legacy records (pre-this-PR behavior was unconditional fold), or document the migration.

🟠 MEDIUM Overspend no longer clamped — external provider overshoot makes pool go negative instead of throwing — src/runtime/supervise/scope.ts

The old clampSpend(spend, opts.budget) that truncated over-reservation spend to the ceiling was removed. Now pool.reconcile(ticket, spend) receives raw spend. The budget pool (budget.ts) no longer throws on over-spend; instead freeTokens/freeUsd go negative, and the readout reports exhaustion. This is a deliberate philosophy change ('Telemetry is never clamped to make the bookkeeping look within budget') and is more honest. However, it means a misbehaving executor reporting more tokens than it reserved will silently make the pool negative rather than failing loud — the old fail-loud guard (ticket spent > reserved) is gone. The unknown-telemetry fail-closed path partially compensates, but a KNOWN overshoot (e.g., provider reports 110% of reservation with tokensKnown:true) now just goes

🟠 MEDIUM budgetExempt executors now hard-refused inside supervised scopes — breaking change — src/runtime/supervise/scope.ts

runChild throws ValidationError when executor.budgetExempt is true: 'does not report usage and cannot execute inside a budgeted supervisor'. Previously budgetExempt executors (cliExecutor, default worktree-cli) were admitted with their spend excluded from conserved Σk. Now they throw after the materialization journal record is already written (the budgetExempt check is after await executionReady). This means any existing caller spawning a raw CLI or unmetered worktree-CLI subprocess inside supervise() will get a typed-down child instead of execution. The cliExecutor (runtime.ts:901) still sets budgetExempt:true. This is intentional per the types.ts doc update but is a hard break with no migration path other than switching to codexReproducible or a metered bridge backend. Impact: existing

🟡 LOW AuthoredHarness.budgetExempt TSDoc summary inverts the field's meaning — docs/api/runtime.md

The TSDoc says 'Require measured usage from this leaf' for a field named budgetExempt where true=exempt/unmetered. The sibling CreateWorktreeCliExecutorOptions.budgetExempt (line 14846) correctly says 'Exclude this leaf's spend from accounting. Defaults to true.' A caller reading only the AuthoredHarness description could infer that setting budgetExempt=true requires measurement — the opposite of actual behavior. Fix: align the summary with the sibling, e.g. 'Exclude this leaf's spend from accounting (default true). Budgeted supervision refuses an unmetered leaf; set false only when the runner returns real usage.'

🟡 LOW contentAddress() TSDoc stripped of security-critical canonicalization invariant — docs/api/runtime.md

The old doc described sha256-over-sorted-JSON canonicalization and the fail-loud-on-mismatch verification that makes the replay invariant trustworthy (a forged outRef breaks replay). The new doc is just 'Stable content address shared by result and trace artifacts.' This weakens documentation of a security boundary — future readers won't know keys are sorted recursively or that stores re-derive on put to detect tampering. Since WorkerToolTraceArtifact now also uses contentAddress, preserving this invariant in the doc matters more, not less. Fix: restore the canonicalization + fail-loud paragraph.

🟡 LOW prepareJsonlAppend reads the entire file on every append (O(n) per write) — src/durable/jsonl-file.ts

fs.readFile(path) reads the full journal into memory and bytes.lastIndexOf(0x0a) scans it on every single appendRecord call. For a journal with N events this is O(N) per append, O(N^2) overall. At typical supervision-tree scale (hundreds of events) this is negligible, but a long-running durable run accumulating 10K+ events would read ~2MB per write. Impact: performance degradation on long-lived journals, not correctness. Fix: stat the file size and read only the last chunk (e.g. last 64KB) to find the final newline boundary, or cache the committed byte length in the FileSpawnJournal instance.

🟡 LOW beginTree/appendEvent read-check-write is outside the serialized append chain — src/durable/spawn-journal.ts

beginTree calls loadTreeBegin (full file read) and appendEvent calls loadTree (full file read + assertSeqUnique) BEFORE entering the appendTail promise chain. Two concurrent appendEvent calls for the same root could both load the same event snapshot, both pass assertSeqUnique, then serialize on appendRecord — defeating the uniqueness guard at write time. The corruption IS caught on the next loadTree read (which re-runs assertSeqUnique on replay), so data integrity is preserved, but the error surfaces as read-time corruption rather than write-time rejection. In practice the supervisor processes settlements sequentially via scope.next(), so same-root concurrent appends do not occur by contract. The InMemorySpawnJournal has the same pattern with no serialization at all, so this is consistent

🟡 LOW replaySpawnTree eagerly validates trace blobs via discarded workerTraceAnalysisStore calls — src/durable/spawn-journal.ts

Lines 604 and 632 call await workerTraceAnalysisStore(trace, blobs) which fetches the trace blob, validates schema/span-count, and builds a full TraceAnalysisStore — but the return value is discarded. The sole purpose is eager integrity validation (fail loud if a settlement references a missing/invalid trace blob). This is intentional and correct, but adds serial blob-get + parse latency to every replayed settlement with an available trace. Not a bug; noting as a design observation since the validation could be done more cheaply (blobs.get existence check + parseWorkerToolTraceArtifact without building the analysis store).

🟡 LOW renderTrace async overload breaks callers passing TraceAnalysisStore to synchronous consumers — src/mcp/tools/checks.ts

renderTrace now has overloads: TraceAnalysisStore → Promise, unknown → string. The implementation returns string | Promise. runCheck handles this with await Promise.resolve(renderTrace(trace)) (line 253), which works. But any external caller that imported renderTrace and treats it as synchronous (the old signature) would get a Promise when passed a TraceAnalysisStore and silently embed '[object Promise]' or fail to await. The overload makes this a compile-time error for direct callers, but makeCheckRunner (line 305) pa

🟡 LOW Event-bus subscriber failures now hide events from queue, history, and stats — src/mcp/tools/coordination.ts

The event-bus change (in event-bus.ts, consumed here) moved subscriber delivery BEFORE queue.push/log.push. A throwing onEvent subscriber now leaves the event invisible — not in bus.pull(), not in bus.history(), not in bus.stats(). The staged WeakMap retry only works when the caller retries with the SAME event object. flushPendingSettlement does this correctly (retains pending.event). But raiseFinding (line 1485) creates an inline event object and does not retry on failure — a subscriber error there permanently drops the online detector's 'worker is looping' finding. Impact: any onEvent subscriber bug or transient infrastructure failure silently swallows find

🟡 LOW authorizeDownMessage throws on missing identity while non-authorized path returns delivered:false — src/mcp/tools/coordination.ts

authorizeInstruction reads workerIdentity only from opts.scope.view.nodes (line 635), not from opts.scope.resume?.view.nodes. When authorizeDownMessage is configured and answer_question targets a worker from a prior process (present in resume.view but not live view), the function throws 'cannot authorize ... without durable identity'. Without authorizeDownMessage configured, the same call proceeds to scope.send and cleanly returns { delivered: false, reason: 'unknown-worker' }. This creates two inconsistent error contracts for the same logical condition. Impact: a product using authorizeDownMessage gets an MCP tool error (throw) instead of a structured { deliv

🟡 LOW Removed per-harness MCP/hook/subagent field validation relies on materializer unsupported-list — src/mcp/worktree-harness.ts

The old code had explicit per-harness checks: mcp[server].headers rejected on codex, mcp[server].cwd rejected on opencode, hooks[x].env/.blocking rejected on claude, subagents[x].permissions/.maxSteps/.tools rejected. All removed in favor of assertProfileMaterialization with the worktreeCliProfileMaterialization contract, which supports mcp/hooks/subagents as axes but does NOT field-check harness-specific incompatibilities. The safety net is plan.unsupported at line 368, but only if the @tangle-network/agent-profile-materialize package flags the same field-level incompatibilities. If it doesn't, a profile with codex-incompatible MCP headers would be silently mat

🟡 LOW No dedicated tests for budget pool restore, scope identity, or materialization receipt paths — src/runtime/supervise/budget.ts

Only bridge-executor.test.ts exists in src/runtime/supervise/. The new budget restore logic (uncertainReservations, committed observe-with-swallowed-throw), scope.spawn keyed-identity comparison (sameNodeExecutionIdentity, key-conflict detection), materialization receipt construction (knownMaterializationReceipt with assertSafeDescriptor), and supervisor resume contract validation (assertResumeContract) have no dedicated unit tests. These are complex, high-stakes paths (budget conservation, durable identity, security descriptor sanitization). The bridge executor tests are excellent but cover only one executor. Impact: regressions in budget restore or identity checking could ship undetected. Fix: add tests for budget restore with uncertain reservations, keyed-spawn identity conflict, and ma

🟡 LOW parseSseFrame now throws on non-JSON data frames instead of silently skipping — src/runtime/supervise/runtime.ts

Previously JSON.parse failure in parseSseFrame returned undefined (silently skipping the frame). Now it throws ValidationError('bridge emitted a non-JSON SSE data frame'). Also, unnumbered data frames (no id: line) now throw. This is stricter protocol enforcement — correct for a well-behaved OpenAI-compatible bridge — but any bridge that sends non-JSON keepalive data frames or unnumbered events will now fail loud where it previously worked silently. Impact: bridges with non-standard SSE framing break; correct for standard bridges. Fix: ensure bridge compatibility is documented.

🟡 LOW No unit tests for detachedSnapshot or trajectory closes-filter expansion — src/runtime/supervise/snapshot.ts

The new detachedSnapshot utility (18 lines, used in 7 files across scope/supervise/materialization/supervisor-agent/spawn-journal) has no dedicated test — no coverage for the cycle-detection path, the structuredClone failure → ValidationError path, or the freeze-enforcement contract. Similarly, the trajectory.ts closes-filter expansion (adding materialized/execution-bound exclusions) has no test; the TypeScript compiler validates it indirectly (without the exclusion, .status access on materialized events is a type error), but a regression that re-adds those kinds to the filter would only be caught at compile time, not by a runtime test. Adding a small test for detachedSnapshot (clone independence, freeze enforcement, DataCloneError → ValidationError) would close this gap.

🟡 LOW deepFreeze does not traverse Map/Set/Date internals — freeze is shallow for rich types — src/runtime/supervise/snapshot.ts

Object.values() returns [] for Map/Set instances (their data lives in internal slots, not own enumerable properties), so deepFreeze freezes the wrapper object but not the entries inside a Map or elements inside a Set. A structuredClone'd Map/Set in a snapshot is detached from the caller's original (structuredClone copies the entries), so this is defense-in-depth only — no caller can mutate the original through the snapshot. But if a downstream consumer holds the snapshot's Map and calls .set() on it, the mutation succeeds silently despite the 'frozen' contract. Fix: add explicit handling for Map/Set (iterate entries/elements, freeze each value) if rich types are expected in snapshots. Low severity because structuredClone already provides the immutability guarantee that matters (caller deta

🟡 LOW defaultPerWorker iterations reduced from full pool to 1/4 — behavior change for existing callers — src/runtime/supervise/supervise.ts

defaultPerWorker now returns maxIterations: Math.floor(budget.maxIterations / 4) instead of budget.maxIterations. The old default was actually problematic (each worker reserved the full iteration pool, so only ~1 could spawn before iteration starvation), so this is a fix. But callers relying on the old default (a single worker getting all iterations) will see workers limited to 1/4. Also adds maxUsd: budget.maxUsd / 4 for usd-capped pools. Impact: positive change (enables ~4 workers by default), but changes the spawn capacity calculus for existing runs. Fix: no action needed; document as improvement.

🟡 LOW Unnecessary ArrayIterator type cast in coordination-log test — tests/kernel/coordination-log.test.ts

The .entries() as ArrayIterator<[number, CoordinationEvent]> cast is unnecessary in modern TypeScript — Array.prototype.entries() already returns the correct type. Harmless but could confuse readers about whether the cast is load-bearing.

🟡 LOW Real-timer timing coupling in deadline and partial-stream tests — tests/kernel/supervise-restart-resource-safety.test.ts

The settleWithin helper uses fixed timeouts (250ms for abort-ignoring executor, 750ms for non-acknowledging teardown) that are coupled to TEARDOWN_ACKNOWLEDGEMENT_MS=250 in src/runtime/supervise/deadline.ts:8. Similarly, supervise-full-profile-bridge.test.ts uses setTimeout(..., 10) for socket death. These passed in 16.8s but are the most likely source of CI flakiness under extreme contention. The vitest.config.ts comment already acknowledges 10x wall-time inflation under load. Consider documenting the coupling or using constants from the source.

🟡 LOW Spend ceiling arithmetic in resume-aware-driver test is underdocumented — tests/runtime/resume-aware-driver.test.ts

The assertions iterations=15, tokens={input:20_050, output:50} encode a specific ceiling-charging formula (5 workers × 3 attempts; 10 killed × 2000-token ceiling + 5 × 10 actual = 20050 input; output only from successful runs). The comment explains the concept ('charged at both declared ceilings and marked unknown') but not the arithmetic, so a future maintainer changing the test budget (currently maxTokens derived from the child script) must reverse-engineer the expected values. Consider adding the formula inline. Not blocking — the test is correct as-is.


tangletools · 2026-07-29T20:04:23Z · 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.

✅ Approved — 20 non-blocking findings — 162fea5a

Full multi-shot audit completed 8/8 planned shots over 117 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-07-29T20:04:23Z · immutable trace

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