fix(supervisor-run): preserve recursive source semantics

[drewstone]

What changes

• Preserve stable workerId identity from Claude and loops journals so duplicate worker labels no longer merge unrelated branches.
• Compute retries and supervisor reactions within each worker's direct parent instead of contaminating sibling branches.
• Preserve explicit nested-supervisor roles, supervisor model/backend options, structured verdicts, and exact worker scores in rollout nodes and reports.
• Track manager-token and worker-token availability independently, and refuse accepted/empty-pass or cost-per-accepted claims when deliverables are unavailable.
• Keep runtime-event translation out of agent-eval; source readers consume their own journal contracts.

Consumer migration

Every custom SourceLimits object literal must add managerTokens and workerTokens. Set a field to null only when that role's aggregate token channel is complete; otherwise set a reason string. NO_SOURCE_LIMITS already includes both fields.

WorkerLogSource.workerId and spawned-event role remain optional for legacy sources. Duplicate labels require stable IDs for unambiguous joins. Report fields are additive: steer/per-worker rows now expose workerId, and per-worker rows expose role and score.

Proof

• Focused supervisor-run suite: 4 files, 64/64 tests passed.
• Full pnpm test: 312 files passed, 2 skipped; 4,222 tests passed, 3 skipped.
• pnpm typecheck: passed.
• pnpm typecheck:examples: passed.
• pnpm lint: 572 files checked, passed.
• pnpm build: package and OpenAPI builds passed.
• pnpm verify:package: skills check, publint, package exports, and accepted ESM declaration profiles passed.
• Built root and supervisor-run declarations export SupervisorRunNodeRole; built NO_SOURCE_LIMITS contains independent manager/worker token channels.
• git merge-tree --write-tree origin/main HEAD: succeeded (3995ad6ef1319f8e09c8753114777932ee90d0f2).
• Critical audit: APPROVE, 0 findings.

[tangletools]

✅ Auto-approved drewstone PR — 6f29632a

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-29T17:28:13Z}

[tangletools]

🟡 Value Audit — sound-with-nits

Verdict	sound-with-nits
Concerns	1 (1 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	126.1s (2 bridge agents)
Total	126.1s

💰 Value — sound-with-nits

Makes the supervisor-run analyzer correct for nested (supervisor-of-supervisor) trees by joining workers on stable IDs and scoping retry/reaction metrics per-parent, plus splits token/deliverable channels into independent unavailable-vs-zero reasons — coherent and in-grain; only nit is committed tra

• What it does: The analyzer previously assumed a flat one-root-spawns-workers tree. This change (1) joins worker logs by stable workerId with label only as a legacy fallback (analyze.ts:216-218, 328, 414), so duplicate human labels under different supervisors no longer overwrite each other's facts; (2) computes respawns, observeThenRespawn, respawnWithoutEvidence, and repeatedLabels grouped by direct
• Goals it achieves: Make reports correct for RECURSIVE runs the module already claimed to model (it had delegationDepth and the claude-code reader already discovered nested spawns at claude-code-reader.ts:386-390, but mis-analyzed them). After merge: identical worker labels in sibling branches stay distinct; 'retry'/'respawn' mean what one supervisor did, not what happened anywhere; an uncaptured token channel repo
• Assessment: Strong, in-grain correctness fix. It extends the module's load-bearing Measured<T> / unavailable-≠-zero discipline (types.ts:38-54) precisely where it was leaking fabricated zeros, and the parent-local grouping is the only semantically correct scope for retry/respawn — those are inherently per-supervisor decisions. It reuses the single parseSupervisorTree parser (one parse, two consumers: anal
• Better / existing approach: none — this is the right approach. I checked for an existing tree/grouping utility to reuse: src/rollout/mint.ts and schema.ts treat parent_rollout_id as a flat row field with no tree-traversal helper, and no shared parent→children index exists anywhere in src/. The childSpawnsByParent map (analyze.ts:490-496) is built inline and used once; extracting a 'tree node' abstraction would fight
• 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

Fixes two real correctness bugs in recursive supervisor-tree analysis (global respawn accounting contaminating sibling branches, and label-keyed worker joins merging unrelated workers) plus closes a fabricated-zero loophole by splitting manager/worker token channels in SourceLimits.

• Integration: Fully reachable and exercised. Both readers populate the new fields: loops-reader extracts workerId from the started event (loops-reader.ts:99-104) and declares workerTokens unavailable when the harness join fails (loops-reader.ts:186-189); claude-code-reader sets workerId=agentId per spawn (claude-code-reader.ts:567) and emits role on every journal line (claude-code-reader.ts:462). The analyzer (
• Fit with existing patterns: Matches the module's central patterns exactly. (1) The Measured unavailable-vs-zero discipline (types.ts:38-39) is the module's load-bearing invariant; adding managerTokens/workerTokens to SourceLimits extends it to token channels that previously fabricated zeros when a harness join failed — directly aligned with CLAUDE.md's 'No fallbacks. Fail loud.' doctrine. (2) The source-agnostic reader co
• Real-world viability: Holds up on the path it targets. The parent-local respawn accounting (analyze.ts:502-533) correctly scopes labelCounts, orderedSpawns, and directCloseTimes per-parent, so two supervisors each spawning 'same task' no longer cross-count — proven by analyze.test.ts:391-490. The workerId ?? label fallback (analyze.ts:216-218, rollout-nodes.ts:228) keeps legacy label-keyed stores working. readVerdict (
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

💰 Value Audit

🟡 Transient agent-run artifacts committed into the source tree [maintenance] ``

The diff adds .agent/critical-audit/2026-07-29T17-08-06Z/{findings.jsonl,manifest.json,summary.md}, .agent/skill-runs.jsonl. findings.jsonl is empty, the manifest has placeholder reviewers ["A","B","C"] and head "working-tree" — clearly throwaway run metadata, not package content. .gitignore does not exclude .agent/ (only .marketing-agent-runs/), so these will accumulate across PRs. Recommend adding .agent/ to .gitignore and dropping these files from the PR; they are not part of

---

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 · 20260729T173044Z}

[tangletools]

✅ No Blockers — 6f29632a

Review health 100/100 · Reviewer score 58/100 · Confidence 85/100 · 10 findings (1 medium, 9 low)

glm: Correctness 58 · Security 58 · Testing 58 · Architecture 58

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 18 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM manifest.json head is a non-resolvable 'working-tree' placeholder, not the commit SHA — .agent/critical-audit/2026-07-29T17-08-06Z/manifest.json

"head": "working-tree" while "base" is the real SHA f99f7cb.... Once committed, 'working-tree' resolves to nothing — a reader cannot checkout or diff the audited range, defeating the artifact's traceability purpose. The actual head is 6f29632 (confirmed via git log). Fix: replace "working-tree" with the resolved head SHA before/after commit so base..head is reproducible.

🟡 LOW findings.jsonl contains only a blank line — semantically correct but fragile for strict parsers — .agent/critical-audit/2026-07-29T17-08-06Z/findings.jsonl

The file is a single newline, representing '0 findings' — consistent with the summary. No current reader parses this path (verified), and the repo's own JSONL reader (FindingsStore.loadAll, src/analyst/findings-store.ts:56) skips blank lines, so no breakage today. Flagged only because a strict JSONL consumer added later would error on a non-JSON line. Optional: write '{}' is wrong (that's a finding); prefer an explicit empty file or a header comment convention if the format is meant to be machine-read.

🟡 LOW Audit scope (14 files) is 4 short of the 18-file PR and the exclusion is only partially disclosed — .agent/critical-audit/2026-07-29T17-08-06Z/manifest.json

manifest.json:5 records "files": 14 and summary.md:10 says 'n=14', but the PR diffstat is 18 files (+808 -205). The only documented exclusion (summary.md:14 'Runtime adapter implementation') does not enumerate the remaining excluded files (e.g. CHANGELOG.md, .agent/skill-runs.jsonl, src/index.ts). An APPROVE that silently covers 14/18 files is fine if disclosed; here disclosure is partial. Fix: list every excluded path in the 'Not inspected' row so the verdict's coverage is unambiguous.

🟡 LOW Load-bearing test-pass claim has no evidence pointer, violating the artifact's own self-gate — .agent/critical-audit/2026-07-29T17-08-06Z/summary.md

Line 3 asserts '64 of 64 focused tests pass' as the sole numerical basis for the APPROVE verdict, but supplies no log path, command output, or run id. The summary's own Self-gate (line 23, item 5) requires 'evidence is a pointer.' A verdict number with no pointer is exactly what the repo's no-fake-success doctrine flags. Fix: add the test command + a path to the captured log (or the skill-runs.jsonl entry that recorded the run).

🟡 LOW Local agent telemetry committed to repo; should be gitignored — .agent/skill-runs.jsonl

New file .agent/skill-runs.jsonl is 3 lines of local skill-run telemetry (skill name, timestamp, project, verdict) produced by the agent session that authored this PR. Nothing in the codebase reads this path: src/analyst/kinds/skill-usage.ts only consumes .evolve/skill-runs.jsonl, and .evolve/ is already in .gitignore alongside .runs/, .calibrate/, .evolve/ as ephemeral run state. .agent/ is missing from .gitignore, so the file leaked in. Impact: repo pollution only — no runtime, no secrets, no logic change. Fix: git rm .agent/skill-runs.jsonl and append .agent/ to .gitignore (next to the other run-ephemeral entries). Alternatively, if .agent/ is intended to be tracked product state, document why it diverges from the .evolve/ convention.

🟡 LOW Breaking SourceLimits change not flagged with repo's BREAKING marker — CHANGELOG.md

The new fields SourceLimits.managerTokens and SourceLimits.workerTokens are declared readonly ...: string | null (required, not optional) in src/supervisor-run/types.ts:101-104, and NO_SOURCE_LIMITS was updated to include them. Any external SupervisorRunSources reader that constructs a SourceLimits object literal will fail to compile under tsc --noEmit until it adds both fields. The CHANGELOG entry documents the requirement in prose but files it under plain ### Changed, whereas this file's established convention for compile-breaking API changes is ### Changed — BREAKING (see CHANGELOG.md:331, 392, 474). Recommend retitling the subsection ### Changed — BREAKING (or adding a one-line note that the field addition is required) so consumers scanning the diff don't miss the m

🟡 LOW Legacy no-workerId path may regress per-worker usd attribution for shared labels — src/supervisor-run/analyze.ts

usd: usdLimit !== null || spawn === null ? null : (workerUsdById.get(spawn.id) ?? null) — when a worker source has no workerId, spawnForSource falls back to firstSpawnByLabel.get(w.label) (line 391). If two sibling spawns share that label, the lookup always returns the FIRST one, so any worker source after the first lands on the wrong spawn.id and now reads null usd (the previous code did workerUsdByLabel.get(w.label) and would have summed across same-label closes). This is documented as the legacy 'labels not unique' footgun in rollout-nodes.ts:224-225, and the loops-reader at loops-reader.ts:99-104 does extract workerId when present, so real loops dat

🟡 LOW Token limit cast assumes deliverables string when only deliverables is set — src/supervisor-run/analyze.ts

gap('accepted', verdictLimit ?? (src.limits.deliverables as string)) — the as string cast is sound because the enclosing condition (verdictLimit !== null || src.limits.deliverables !== null) guarantees at least one is non-null and ?? returns the deliverables branch only when verdictLimit is null. Readable but relies on a non-local invariant; a named const like const acceptedLimit = verdictLimit ?? src.limits.deliverables (narrowed above) would remove the cast and the multi-line ternary entirely.

🟡 LOW accepted and emptyPass gated identically but only accepted records a gap — src/supervisor-run/analyze.ts

Both accepted (line 627) and emptyPass (line 639) are gated on verdictLimit !== null || src.limits.deliverables !== null, but accepted uses gap(...) (appends to the report's gaps[] array) while emptyPass uses unavailable(...) only. A reader scanning report.gaps for the deliverables limit will see accepted: ... but not emptyPass: ..., even though both are unavailable for the same reason. Pick one: either both should record a gap (preferred, since both are first-class decision metrics) or neither should. The test at

🟡 LOW SourceLimits adds two required fields — breaking change for external literal constructors — src/supervisor-run/types.ts

managerTokens and workerTokens are newly required (non-optional) on the exported SourceLimits interface. Any out-of-tree consumer that constructs SupervisorRunSources.limits as an object literal (rather than spreading NO_SOURCE_LIMITS) will fail to typecheck after upgrade. The public NO_SOURCE_LIMITS constant is updated, so callers using it are unaffected. Within this repo only claude-code-reader.ts and tests construct limits and both are updated. Worth a CHANGELOG / minor-version bump note: this is a substrate package (CLAUDE.md:11) and other repos depend on it.

---

_{tangletools · 2026-07-29T17:41:35Z · trace}

[tangletools]

✅ Approved — 10 non-blocking findings — 6f29632a

Full multi-shot audit completed 5/5 planned shots over 18 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-29T17:41:35Z · immutable trace}

[tangletools]

✅ Auto-approved drewstone PR — e1f37fd4

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-29T18:48:26Z}

[tangletools]

🟢 Value Audit — sound

Verdict	sound
Concerns	0 (none)
Heuristic	0.0s
Duplication	0.0s
Interrogation	103.0s (2 bridge agents)
Total	103.0s

💰 Value — sound

Fixes the supervisor-run analyzer so recursive (multi-level) supervision trees are accounted correctly: parent-local retry/repeated-label counting, stable workerId joins, structured verdict preservation, and explicit independent token-availability declarations — all additive and in-grain.

• What it does: Extends the supervisor-run module to correctly model recursive supervision trees. (1) Worker logs now join to journal spawns by stable workerId first, falling back to label only when no id exists (workerSourceKey, analyze.ts:216-218), so two workers sharing a label under different supervisors no longer merge. (2) Respawns, repeatedLabels, observeThenRespawn, and respawnWithoutEvidence are now co
• Goals it achieves: Correctness for real recursive supervision trees where supervisors spawn sub-supervisors that spawn workers and the same task label recurs across branches — the prior global accounting fabricated retries and repeated-labels that were cross-branch coincidences, not real supervisor decisions. Secondary goal: align the module with the repo's fail-loud doctrine by replacing fragile null/0 inference (w
• Assessment: A coherent, well-grained fix that extends the existing architecture rather than restructuring it. The analyzer stays pure and source-agnostic; readers (loops-reader, claude-code-reader) populate the enriched contract. All contract changes are additive and backward-compatible: workerId and role are optional on WorkerLogSource/spawned-events for legacy sources, and NO_SOURCE_LIMITS defaults both new
• Better / existing approach: none — this is the right approach. I searched for existing verdict/role/token-availability handling to reuse: src/verdict.ts holds DefaultVerdict (a training-gate type, not journal parsing); src/rollout/reward.ts exports unscreenedRewardFields (already reused in rollout-nodes.ts:163,260); src/rollout/readers/claude-jsonl.ts owns transcript parsing (already reused via parseClaudeEntries in claude-c
• 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 correct, well-wired fix that makes recursive supervisor-tree accounting parent-local and adds independent token channels, immediately consumed by both readers and the rollout-row minter.

• Integration: Fully wired and reachable. Both production readers (loops-reader.ts:116-121, claude-code-reader.ts:454-466) populate the new fields (workerId, role, structured verdicts, managerTokens/workerTokens limits). The rollout-row minter (rollout-nodes.ts:243-272) consumes spawn.role, close.valid/score/rawVerdict. The new type is exported (src/index.ts:549) and the public contract is locked by index.test.t
• Fit with existing patterns: Fits the codebase's established patterns exactly. The SourceLimits extension (independent managerTokens/workerTokens) is the natural continuation of the existing spendUsd/workerVerdicts/deliverables model. The workerSourceKey = workerId ?? label preserves the legacy label-join as a documented fallback, matching the backward-compat posture already taken for WorkerLogSource.workerId being optional.
• Real-world viability: Holds up under the edge cases that motivated it. The parent-local respawn/label accounting is verified against fixtures with identical labels under two different supervisors (no false merge) and same-label retries under one parent (correctly counted). Structured verdict parsing handles string, object, and null inputs. The reader's worker-id recovery requires unanimous agreement across started rows
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅

---

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 · 20260729T185027Z}

[tangletools]

✅ No Blockers — e1f37fd4

Review health 100/100 · Reviewer score 86/100 · Confidence 85/100 · 3 findings (3 low)

glm: Correctness 86 · Security 86 · Testing 86 · Architecture 86

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 19 changed files. Global verifier still owns final merge decision.

🟡 LOW rejectedLimit computation is a 5-level nested ternary — src/supervisor-run/analyze.ts

The rejectedLimit chain (lines 641-650) nests 5 levels of ternary conditional to determine whether the rejected count should be unavailable. While correct — I traced all 6 branches (journalVerdictsComplete, sourceVerdictsComplete, settledCloses>0, verdictLimit non-null, no-journal-no-workers, null) — this is the single hardest-to-audit expression in the diff. A named helper like computeRejectedLimit(settledCloses, verdictLimit, journalComplete, sourceComplete, haveJournal, workers, gapReason) with early returns would make the verdict-unavailability logic reviewable without holding 5 conditions in working memory. Not blocking; the existing codebase uses neste

🟡 LOW sourceVerdictsComplete can count verdicts from workers with no matching settled close — src/supervisor-run/analyze.ts

sourceVerdictsComplete checks sourceVerdicts.length >= settledCloses.length. When src.workers contains entries that finished and reported pass/fail but have no corresponding journal settle (data inconsistency or still-running workers that reported early), those extra source verdicts are included in the rejected count via sourceVerdicts.filter(valid => !valid).length. This matches the old code's behavior (old code also counted any f.passed === false regardless of settle status), so it is not a regression. It is a deliberate fallback: when journal verdicts are incomplete, worker-log verdicts are the best available signal. Noted for transparency; no fix needed.

🟡 LOW gaps array length is not pinned by any assertion — tests/fixtures/supervisor-run/claude-code-session-report.json

The fixture test asserts only report.gaps.length > 0 (claude-code-fixture.test.ts:56), not the specific count. This shot ADDED a gap (7->8) and a future regression that silently drops the workers.tokensIn gap would still pass the suite. The core invariant IS covered by the tokensOut assertion at line 63-65, so impact is low. Optional: add expect(report.gaps.length).toBe(8) and/or assert report.gaps contains a string matching /workers.tokensIn: 33/52/ to lock the honesty contract for the newly-surfaced unavailability.

---

_{tangletools · 2026-07-29T18:57:45Z · trace}

[tangletools]

✅ Approved — 3 non-blocking findings — e1f37fd4

Full multi-shot audit completed 5/5 planned shots over 19 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-29T18:57:45Z · immutable trace}