fix(supervise): read the floor from the harness a root actually declared - #693
Conversation
The guard read only `spec.harness`, which is null whenever the harness rides in the backend config instead of the spec -- the path that actually spawns workers through the bridge. So it missed the exact case it was written for. Measured: discovery-lab run proof-bridge-20260801f. The root ran 14 turns successfully on pi through cli-bridge, then authored a child with 5 iterations / 6,000 tokens -- one fifth of the measured 31,211 floor. The guard never fired (the spawn event is in the journal), the child materialized on pi, and it produced nothing in 8 seconds. That is the seventh consecutive run to die of a below-floor child budget. An AgentProfile is where a root declares its child's harness, so it is the more reliable of the two sources. Falling back to it closes the path the bridge takes without changing the router/inline arms, which still resolve to no floor when neither source names a harness. Three tests, including the exact 6,000-token budget that slipped through. Full suite: 2259 passed / 6 skipped, exit 0.
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 5b139495
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-08-01T19:48:13Z
AgentProfile.harness is not exactly BackendType, so reading the floor from it failed the repo typecheck (which resolves examples against the built dist and is stricter than a bare tsc --noEmit). Widened to a bare string. An unrecognised name resolves to null -- no floor -- rather than throwing, so an unknown harness stays admissible instead of being refused on a name this table has not heard of.
❌ Needs Work —
|
| glm | deepseek | deepseek-flash | aggregate | |
|---|---|---|---|---|
| Readiness | 73 | 79 | 41 | 41 |
| Confidence | 70 | 70 | 70 | 70 |
| Correctness | 73 | 79 | 41 | 41 |
| Security | 73 | 79 | 41 | 41 |
| Testing | 73 | 79 | 41 | 41 |
| Architecture | 73 | 79 | 41 | 41 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision.
Blocking
🔴 HIGH Typecheck failure: HarnessType union (includes 'gemini') not assignable to workerTokenFloor(BackendType | null) — src/runtime/supervise/scope.ts
spec.harnessisBackendType | null;spec.profile.harnessisHarnessType | undefined(agent-interface 0.40.0AgentProfile.harness?: HarnessType).spec.harness ?? spec.profile.harness ?? nullhas typeBackendType | HarnessType | null, andHarnessTypeincludes'gemini', which is not a member of sandbox 0.15.2BackendType(14 values incl. forge/cursor, excl. gemini). Reproduced TS2345 (Type '"gemini"' is not assignable to type 'BackendType | null') using the exact pinned package .d.ts files and the repo's typescript 6.0.3 withstrict: true.workerTokenFloor's internalas Record<string, number | null>cast does not widen its parameter type. CI gatepnpm run typecheck=tsc --noEmitoversrc(ci.yml) will fail. Fix: narrow before calling, e.g. `workerTokenFloor(
Other
🟠 MEDIUM Missing type cast on HarnessType→BackendType when falling back to profile.harness — src/runtime/supervise/scope.ts
spec.profile.harness is HarnessType | undefined (includes 'gemini') but workerTokenFloor expects BackendType | null (excludes 'gemini' per sandbox-backend.ts:60-65). The identical fallback pattern at supervise.ts:336 uses
as BackendType | null. The runtime behavior is safe — workerTokenFloor does(WORKER_TOKEN_FLOOR as Record<string, number | null>)[harness] ?? nullso unknown strings return null (no floor). Fix: appendas BackendType | nullto the expression or wrap profile.harness with the same cast pattern.
🟠 MEDIUM profile.harness is a preference, not the effective harness; guard both misses and false-rejects — src/runtime/supervise/scope.ts
AgentProfile.harness(agent-interface 0.40.0) is documented as an optional PREFERENCE the executor MAY override (leaderboard harness sweeps, per-subtask harness picks, per-cell bridge backend). Effective harness is resolved from multiple sources the guard ignores:runtime.ts:2777spec.harness ?? captured.harness(config-named sandbox harness);bridgeCellModel/bridgeExecutorbackend?.type ?? agentHarness(profile.harness)wherebackend.typecomes fromctx.seams.createOptions.backend.typeper-cell override;sandbox-backend.ts:77resolveBackendType= override.type > metadata.backendType > profile.harness > default 'opencode'. Consequences: (a) UNDER-enforcement — a root that names the harness only in the config,metadata.backendType, or a bridge per-cell override still by
🟠 MEDIUM New 'regression' tests don't exercise the fix — they assert the ?? operator — tests/kernel/budget-floor.test.ts
The describe block is named for the bridge-path regression (proof-bridge-20260801f, 6,000-token child), but every case calls workerTokenFloor(specHarness ?? profileHarness) with locally-declared constants — re-implementing scope.ts:573's
spec.harness ?? spec.profile.harness ?? nullinline in the test. This asserts that JSnull ?? 'pi'yields 'pi' and workerTokenFloor('pi')===31_211; it never imports or calls createScope/spawn. Proof: grep shows workerTokenFloor has exactly ONE production caller (scope.ts:573) and the 'below-runtime-floor' rejection reason has ZERO test references anywhere in tests/. So reverting scope.ts:573 to the buggyworkerTokenFloor(spec.harness)leaves all three new tests green. The regression this PR exists to fix is therefore unguarded after the PR merges. Fi
🟠 MEDIUM New tests never exercise the code the PR changes — no regression protection — tests/kernel/budget-floor.test.ts
The PR's actual fix is src/runtime/supervise/scope.ts:573:
workerTokenFloor(spec.harness ?? spec.profile.harness ?? null). This test block re-implements the merge with local literals (specHarness ?? profileHarness, both typednull/'pi') and asserts onworkerTokenFloor, a pure function that is byte-identical in this PR (verified:git difffor budget-floor.ts is empty) and already covered by the pre-existing describe block. The file imports only from budget-floor ([line 8](https://github.com/tangle-network/agent-runtime/blob/5b139495bca6d7da8b7b832ad3b394ca6eea8e58/tests/kernel/budget-floor.test.ts#L8)) and never loads scope.ts, so a revert of the scope.ts line toworkerTokenFloor(spec.harness)leaves all 9 tests passing. The describe title asserts behavior ('the floor reads
🟡 LOW Added tests cover workerTokenFloor literals only; scope.spawn guard path untested — src/runtime/supervise/scope.ts
The new tests in tests/kernel/budget-floor.test.ts call
workerTokenFloor(specHarness ?? profileHarness)with string-literal/null inputs. They never construct anAgentSpecwithprofile.harnessset and assert thebelow-runtime-floorrejection fromscope.spawn, so (a) the?? nullunion type error at the real call site is invisible to them, and (b) the motivating bridge scenario (spec.harness null, harness riding in backend config/profile) has no integration coverage that would catch either finding above. An integration test that spawns through the bridge/sandbox path with a sub-floor budget on a pi profile and asserts{ ok: false, reason: 'below-runtime-floor' }would cover both the type contract and the effective-harness resolution.
🟡 LOW Comment says profile harness is 'the more reliable of the two' but ?? lets spec.harness win — src/runtime/supervise/scope.ts
The added comment (lines 567-572) states 'The AgentProfile is where a root declares its child's harness, so it is the more reliable of the two,' but the expression spec.harness ?? spec.profile.harness ?? null evaluates spec.harness FIRST. Operationally this is correct and matches runtime.ts:2777 (and requiredProviderProfileHarness throws when the two conflict, so divergence is impossible on the provider path), so for the bug case — spec.harness===null — profile.harness is used as intended. The wording just inverts the precedence the reader expects; tighten to note spec.harness wins when set and profile.harness is the fallback for the null/bridge path.
🟡 LOW No test coverage for the floor guard; PR adds none for the fixed bridge path — src/runtime/supervise/scope.ts
grep across src/**/*.test.ts for below-runtime-floor|workerTokenFloor|WORKER_TOKEN_FLOOR returns zero matches. The below-runtime-floor branch (lines 574-578) is untested both before and after this change. This PR broadens the guard to the spec.harness===null bridge path — exactly the path the comment says was broken (proof-bridge-20260801f) — yet adds no test asserting that a profile.harness='pi' spawn with maxTokens<31211 now returns {ok:false, reason:'below-runtime-floor'} while a spec.harness='pi' spawn still hits it. Without a test the fix can silently regress. Add a unit test in a scope spawn test that constructs an AgentSpec with harness:null + pro
🟡 LOW Type strictness of profile.harness -> BackendType unverified (no typecheck run) — src/runtime/supervise/scope.ts
workerTokenFloor is typed (harness: BackendType | null) (budget-floor.ts:52). spec.profile.harness is HarnessType|undefined. In agent-interface 0.36 HarnessType includes 'gemini' and omits 'forge'/'cursor', so it is NOT assignable to BackendType; the repo pins >=0.40 where the two may be aligned, but node_modules are absent in this worktree so tsc could not be run to confirm spec.harness ?? spec.profile.harness ?? null typechecks cleanly. Runtime is safe regardless (workerTokenFloor casts to Record<string,number|null> and returns null for unknown keys), so this is a strict-typecheck nit, not a behavior risk. Confirm
pnpm typecheckis green in CI.
🟡 LOW Floor value 31,211 hardcoded instead of read from the table — tests/kernel/budget-floor.test.ts
The measured floor is duplicated as a literal (also at lines 14, 28, 36, 57, 67) rather than derived from
WORKER_TOKEN_FLOOR.pi. Pre-existing style, but the new block adds a fourth copy. If the floor is ever re-measured, tests silently stale-document the old number while the table changes — the very drift the file's 'carries only measured numbers' test claims to prevent. Suggestexpect(workerTokenFloor(specHarness ?? profileHarness)).toBe(WORKER_TOKEN_FLOOR.pi).
🟡 LOW New test cases are largely redundant with existing tests — tests/kernel/budget-floor.test.ts
Tests 1 and 2 retest workerTokenFloor('pi') and workerTokenFloor(null) — both already covered in the first describe block at lines 11-14 and 17-19. Test 3 adds 6,000 to the budget-refusal check but the existing test at line 27-32 already covers a range of floor-violating budgets (2k/8k/12k/20k). The commentary adds documentation value but the assertions add one net-new numeric value (6,000) and zero new behavioral paths. The tests also exercise
??on local scalar variables rather than the `spec.harness ?? spec.prof
🟡 LOW No integration test for below-runtime-floor spawn rejection — tests/kernel/budget-floor.test.ts
The 9 unit tests in budget-floor.test.ts validate workerTokenFloor() and WORKER_TOKEN_FLOOR, including the null ?? 'pi' fallback pattern, but no test exercises the full scope.spawn() path to verify the guard actually returns { ok: false, reason: 'below-runtime-floor' } when a profile declares a pi harness and the budget is below 31,211. A comprehensive test would construct a minimal scope, spawn a child with a pi profile and 6,000-token budget, and assert the rejection.
🟡 LOW Third added test is fully redundant — tests/kernel/budget-floor.test.ts
expect(6_000).toBeLessThan(workerTokenFloor('pi')!)duplicates the pre-existing loop at line 27-32 which already asserts 2_000, 8_000, 12_000, 20_000 < floor; 6,000 is strictly between two already-covered values and cannot add information. It only adds narrative weight to the comment. Harmless, but combined with the first finding it compounds the false sense that the regression case is locked.
🟡 LOW Third test is fully redundant with the pre-existing floor test — tests/kernel/budget-floor.test.ts
Asserts 6_000 < workerTokenFloor('pi'). Lines 27-32 already assert [2_000, 8_000, 12_000, 20_000] are all < floor; since 6_000 < 8_000 < floor, this is subsumed and adds no boundary. It also tests workerTokenFloor('pi') directly (same as line 14), not the fix path. Either delete it or replace with the integration case described above; in its current form it guards nothing the surrounding file doesn't already cover.
tangletools · 2026-08-01T20:00:14Z · trace
tangletools
left a comment
There was a problem hiding this comment.
❌ 1 Blocking Finding — 5b139495
Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 2/2 planned shots over 2 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-01T20:00:14Z · immutable trace
…child budget (#695) A parent authors its child's budget blind: across a clean n=8 live sample, 5 of 6 authored child budgets were below the measured pi floor of 31,211 input tokens. #685/#693 made the runtime REFUSE such budgets (below-runtime-floor); nothing yet TOLD the root the floor first. Two telling surfaces in spawn_agent, both generated from WORKER_TOKEN_FLOOR so a newly measured harness appears with no prose edit: - The budget schema description now carries every measured floor at tool-discovery time, plus the refusal reason it maps to. - A below-runtime-floor refusal now carries a hint naming the declared harness's floor and the only fix — raise maxTokens to at least the floor, never retry smaller — matching the existing usd-unbudgeted hint pattern. With no harness on the profile the hint lists the whole measured census.
Follow-up to #685, which shipped a floor guard that misses the path workers actually take.
The miss
The guard read only
spec.harness. That isnullwhenever the harness rides in the backend config instead of the spec — which is exactly how workers spawn through cli-bridge. SoworkerTokenFloor(null)returnednulland admission proceeded.Measured
discovery-labrunproof-bridge-20260801f: the root ran 14 turns successfully on pi through cli-bridge (173,250 in / 11,800 out), then authored a child with:One fifth of the measured 31,211 floor. The guard never fired — the spawn event is in the journal — the child materialized on pi, and produced nothing in 8 seconds.
That is the seventh consecutive run to die of a below-floor child budget, and the first where the guard from #685 was present and simply did not apply.
The fix
An
AgentProfileis where a root declares its child's harness, so it is the more reliable of the two sources:The router and inline arms are unaffected — when neither source names a harness the floor is still
null, which is the pre-existing behavior.Checks
tsc --noEmit— 0 errors