Skip to content

fix(kernel): refuse an executor backend nothing implements - #956

Merged
drewstone merged 1 commit into
mainfrom
fix/refuse-unknown-executor-backend
Aug 21, 2026
Merged

fix(kernel): refuse an executor backend nothing implements#956
drewstone merged 1 commit into
mainfrom
fix/refuse-unknown-executor-backend

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Problem

snapshotExecutorConfig (src/runtime/supervise/runtime.ts:4550) switches over the seven ExecutorConfig arms and has no default. TypeScript accepts that, because the switch is exhaustive over the union — but the union is not what arrives. The type's own docstring says so: "the backend is DATA — the cost dial a profile, an experiment config, or a replay journal can name." A profile, a config file, or a journal line carries a string, not a discriminated union member.

Given a name outside the union, the switch returns undefined, createExecutor builds and returns a factory anyway, and the failure lands one call later:

$ createExecutor({ backend: 'bridge-worktree' } as never)
createExecutor RETURNED a factory: function
THREW: TypeError :: Cannot read properties of undefined (reading 'backend')

That TypeError comes from runtime.ts:4738, names no backend, and points at a line the caller did not write. The house rule is the opposite: no fallbacks, fail loud, typed errors.

I found this while checking whether bridge-worktree is an unowned eighth backend. It is not — it is a label on ExecutorMaterialization.backend, which is string by design because providers and BYO executors stamp their own names, and it has a producer (runtime.ts:4286), a consumer that names it by hand (stream-agent-turn.ts:319) and a passing test (tests/runtime/worktree-cli-executor.test.ts:709). The claim was wrong; the missing refusal it led me to is real, and applies to any unrecognized backend.

Change

snapshotExecutorConfig gains a default arm that throws ValidationError naming the value and the supported set:

ValidationError :: createExecutor: no backend named "bridge-worktree"; supported backends
are bridge, cli, cli-worktree, provider, router, router-tools, sandbox

The supported set is read off WORKER_TRACE_PROPAGATION (supervise/worker-trace.ts:81) rather than written out again. That table is satisfies Record<ExecutorConfig['backend'], boolean>, so the compiler already forces it to hold every arm — it is the one copy that cannot go stale, and reusing it means this change adds no eighth list.

The prose this exposed

Tracing the backend vocabulary turned up four descriptions of it, three of them stale, all corrected here:

Site Said Says
CLAUDE.md:67 router / router-tools / bridge / cli / sandbox — five of seven all seven
docs/architecture.md:456 "sandbox / cli-bridge / router / worktree-cli", citing runtime.ts:1517 all seven, citing runtime.ts:4735
ExecutorConfig doc comment runtime.ts:4522 six of seven names no list, so it cannot drift again
supervisor.ts:14 "A teardown failure is journaled as a cancelled event" teardown-unconfirmed, which is what supervisor.ts:749 writes

The last one is worth its own line: SpawnEvent.cancelled has eight readers and, in the product, zero writers, and that sentence is why it reads as an intended-but-missing writer. It is not. A cancelled node is journaled as settled with status: 'down' (scope.ts:1785) — the shape conformance/capabilities.json registers under cancellation-acknowledgement and tests/kernel/worker-cancellation.test.ts pins — plus a durable control record naming the effect (coordination-driver.ts:509). Driving a real cancel through the real runtime and dumping every journal line confirms it: eleven lines, one settled/down carrying the reason, cancelled events: 0. The member stays because it is the replay vocabulary for an external SpawnJournal implementor; deleting it produces 12 type errors in the replay and tree-view arms. The stale sentence was the whole defect.

docs/architecture.md's citation was also 3218 lines away from the symbol it named. The freshness gate (scripts/check-docs-freshness.mjs) checks that a cited file exists and never that the line is right, and CLAUDE.md is not in its curatedDocs list at all — so all three sites were green while wrong. I am not adding a gate class for that: per the standing rule, a gate whose defect has occurred once and whose invariant a type could carry is the wrong tool, and here the type already carries it (WORKER_TRACE_PROPAGATION). Naming the blind spot is the deliverable.

Proof

pnpm run lint                 615 files, no fixes
pnpm run typecheck            clean
pnpm run build                clean
pnpm run check:api-surface    2120 exports / 17 entry points, record current
pnpm run check:version-bump   consumer surface unchanged at 0.154.0
                              (a function body and prose; no declaration moved)
pnpm run docs:check           exit 0
pnpm test                     2805 passed / 170 failed across 20 files
  clean origin/main, same machine: 2797 passed / 171 failed across 21 files
  The 20 failing files are a strict SUBSET of the baseline's 21. Zero new failures.
  All are macOS git-worktree and process-spawn timeouts; CI on Linux is the authority.

The new test fails before the change with AssertionError: expected [Function] to throw an error — measured by reverting runtime.ts and re-running it.

Simplification

Simplification: the backend vocabulary now has one description in prose (all seven, everywhere) and one machine-checked list (WORKER_TRACE_PROPAGATION), which the refusal reads instead of restating; the ExecutorConfig doc comment stops carrying a list that can drift.
Net: +19 / -6 lines across 4 files plus one regenerated docs/api/runtime.md; 1 stale list deleted, 0 lists added.
Not done here: ExecutorMaterialization.backend stays string. It is written by providers and BYO executors with names this package cannot enumerate, and the one place it is load-bearing — assertExactExecutorDeclaration (stream-agent-turn.ts:283, :319) — has no test at all. That is a separate change with its own reproduction.

Tests: +1 (createExecutor refuses an unimplemented backend by name — the case that today returns a working-looking factory and fails one call later as an unattributed TypeError), -0 deleted.

Refs #954

snapshotExecutorConfig switched over the seven ExecutorConfig arms with no
default. The backend is data a profile, an experiment config, or a replay
journal can carry, so a name outside the union reaches the factory untyped: the
switch returned undefined, createExecutor handed back a working-looking factory,
and the failure landed one call later as a TypeError naming nothing.

It now refuses by name, listing the supported backends read off the
trace-propagation table, which is the one copy the compiler already forces to
hold every arm.

Three prose sites named a stale backend set and are corrected: CLAUDE.md listed
five of seven, docs/architecture.md paraphrased four and cited a line number
3218 lines away from createExecutor, and the ExecutorConfig doc comment listed
six. The supervisor's own header claimed a teardown failure is journaled as a
`cancelled` event; it is journaled per node as `teardown-unconfirmed`.

@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 — 3dd4e166

This PR was opened by the trusted drewstone account.

This approval is provisional and was applied by the local stand-in because the pr-reviewer webhook host is unreachable (2026-08-21). CI on this head is fully green. The full PR reviewer audit re-runs via the resweep when the service returns and will publish findings if it detects issues.

@drewstone
drewstone merged commit 406e6fa into main Aug 21, 2026
4 checks passed
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