feat(runtime): type runtimeId off the union and guard the literals (warren-c80e) - #964
Conversation
…arren-c80e) Items 4 and 5 of jayminwest#846. Items 1 to 3 landed in jayminwest#887. RuntimeId cannot be the type at the dispatch seam, because WARREN_EXTRA_RUNTIME_IDS lets an operator register ids beyond the canonical list and the acceptance harness uses it for stub-shell. The documented widening is AcceptedRuntimeId, which moves from src/registry/schema.ts to the canonical wire home so src/runtime/ does not have to depend on src/registry/ for a type alias. wire.ts sits at its file-size budget, so the runtime-id vocabulary moves to src/core/wire-runtime.ts and wire.ts re-exports it, the same split already done for the actor, inbox and insight vocabularies. The budget drops from 627 to 606, and src/ui/tsconfig.app.json lists the new module alongside the other three. src/runs/stream/stats.ts declared its own union containing "claude", which is not a runtime id. It is read only into a log payload, so nothing branched on it, but filtering stream logs by runtime missed every claude-code row. The field is now RuntimeId and the three call sites are corrected. scripts/check-runtime-ids.ts fails lint when a runtime-id literal is written outside src/runtime/adapters/. The literal list is derived from KNOWN_RUNTIME_IDS at run time rather than hand-copied, and the allowlist is split between files that name a runtime on purpose and the older conditional code phase 2 absorbs. A stale-entry check keeps an exemption from quietly becoming a wildcard.
|
The
ALTER TABLE `runs` RENAME COLUMN "burrow_id" TO "sandbox_id";
ALTER TABLE `runs` RENAME COLUMN "burrow_run_id" TO "sandbox_run_id";The same commit updated the sqlite arm of expect(cols.has("burrow_id")).toBe(true);
expect(cols.has("burrow_run_id")).toBe(true);That arm is Run #282 on I can send the two-line fix as its own PR if you want it from me, or leave it with you. |
|
Heads up on the audit in 929a6b9: it closed
The PRs the audit lists (#947, #950, #955, #959, #963) are the burrow excision and the wire vocabulary rename, which is a different piece of work. Items 1 to 3 did land in #887, so I think the two got folded together by accident. I have rebased this onto One thing that is not from this PR: |
|
Correction to my last note: the rebase did not clear So the merge order that gets both green is #965 first, then this one. |
…5); ROADMAP: v0.18.0 campaign in flight (pl-a37b) The 'no bwrap => 503' test relied on the host platform: checkBwrap self-skips on non-Linux, so the assertion only held on Linux hosts and failed standalone on macOS. Pin platform: "linux" through the existing deps.platform seam. ROADMAP: record the v0.18.0 any-setup campaign (pl-a37b) as in flight, mark the AgentRuntimeAdapter phase-1 slice complete (PR #964). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Items 4 and 5 of #846, sent separately as agreed on the issue. Items 1 to 3 landed in #887.
Item 4: type
runtimeIdoff the unionRuntimeIdcannot be the type at the dispatch seam.WARREN_EXTRA_RUNTIME_IDS(warren-c4be) lets an operator register ids beyond the canonical list, and the acceptance harness uses it forstub-shell, so an unknown id has to keep flowing through. The documented widening the issue pointed at isAcceptedRuntimeId, which already existed insrc/registry/schema.ts.src/runtime/had no dependency onsrc/registry/, and adding one for a type alias reads backwards. So the alias moves to the canonical wire home instead, next toKNOWN_RUNTIME_IDS, andsrc/registry/schema.tsre-exports it.src/core/wire.tssits at its file-size budget, so the runtime-id vocabulary moves intosrc/core/wire-runtime.tsandwire.tsre-exports the module. That is the same split already done for the actor, inbox and insight vocabularies, andcheck:wire-typesfollowsexport *chains, so all four names stay enforced. The budget forwire.tsdrops from 627 to 606.Sites now typed
AcceptedRuntimeId:src/runtime/contract.ts, both k8s entrypoints, and the twosrc/runtime/local/profile.tsfunctions the issue did not list.The
"pi" | "claude"mismatchsrc/runs/stream/stats.tsdeclared its own union containing"claude", which is not a runtime id. The issue asks whether that masks anything. It does not: the field is only read into a log payload, atpersistInStreamUsage, and the four call sites pass a literal. Nothing branches on it.What it did cost is log correlation. Filtering the stream logs by runtime id missed every claude-code row, because the tag said
claudewhile the rest of the system saysclaude-code. The field is nowRuntimeIdand the three literals are corrected, so the tag agrees withruns.runtimeand with the event stream.Item 5: the lint guard
scripts/check-runtime-ids.tsfails thelintgate when a runtime-id literal is written outsidesrc/runtime/adapters/. It rides insidelintbecause thecheck:allmanifest is frozen, and also runs standalone asbun run check:runtime-ids.Two things follow
check-wire-types.ts:KNOWN_RUNTIME_IDSat run time. A second hand-copied list is the drift class the guard exists to prevent, and it would have gone stale already: sapling left the union while this issue was open.The allowlist is split in two, because the entries mean opposite things:
DELIBERATE, 11 files that name a runtime because that is the file's job. The seven built-in agent definitions declare the runtime they dispatch onto, the three per-runtime shape records are keyed by runtime id by design, andsrc/registry/schema.tsdeclaresDEFAULT_RUNTIME_ID.PENDING_ADAPTER, 5 files of runtime-conditional logic that predates the registry:usage-aggregate.ts,stream/bridge.ts,stream/budget.ts,local/drive.ts,local/profile.ts. These are phase-2 candidates, not endorsements. Freezing them means the residue can shrink but not grow.There is also a stale-entry check: an allowlisted file that no longer writes a literal fails the gate, so an exemption cannot quietly become a wildcard. It earned its place during the work, catching the
src/core/wire.tsentry that the vocabulary split had just made pointless.What I ran
bun run check:allgives 11 of 12 gates green, includinglintend to end with the new guard,typecheck,check:sizeon the lowered budget,check:bundle-size, andcheck:ci-parity. The twelfth ischeck:coverage, which also fails on6bd40e4untouched on this machine: I am on Windows, and the failures aremkdirand.shnot being executable, sqlite file modes, and path separators.6bd40e4fails two gates here, so the branch is not worse than the base on anything.The split needed one thing the type checker alone would not have caught:
src/ui/tsconfig.app.jsonis a composite project that lists the wire modules file by file, sowire-runtime.tshad to joinwire-inbox.ts,wire-insight.tsandwire-actor.tsthere.check:bundle-sizeis what caught it.Full suite, same machine, both trees: 5122 pass and 101 fail on the branch, 5106 pass and 102 fail on
6bd40e4. Comparing the failing test names between the two, no test fails only on the branch.Control for the new gate: appending
const TEMP_PROBE = "claude-code";tosrc/runs/reap/run.tsmakes it exit 1, naming that file and line as writing the literal, and makes the repo-level test fail. Reverting turns both green.scripts/check-runtime-ids.test.tscovers the derivation (including a runtime being removed, and a refusal to guess when the declaration is unreadable), comment lines, substring safety, the adapter and test exemptions, and both allowlist buckets, plus two tests that assert the real repo is clean.