Skip to content

fix: make compiled dispatch exhaustive over TOOLS so native review briefs route (#1070) - #1115

Merged
apackeer merged 8 commits into
mainfrom
fix/issue-1070-review-brief-dispatch
Sep 17, 2026
Merged

apackeer merged 8 commits into
mainfrom
fix/issue-1070-review-brief-dispatch

Conversation

@apackeer

@apackeer apackeer commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #1070. Native aidlc engine review-brief commands failed with aidlc-review-brief.ts does not export main(argv) even though the renderer exported the required entry point. The dispatcher declared the route but its compiled-mode delegate table omitted the tool, and the error text pointed at the wrong layer.

This is the same defect class as the earlier knowledge-verb miss documented in the route table: a tool that works when invoked as bun <tool>.ts (the copy channel every test and every e2e journey runs) but is unreachable through the compiled binary, which is the only path that consults the delegate table. Rather than add one arm, the PR closes the class: the table is typed so drift fails typecheck, t230 drives every delegate through the simulated compiled dispatcher so drift also fails a unit test, the packager's last hand-maintained tool list is derived from TOOLS, and the copy-channel CLI wrappers emit uncaught errors in the shape the compiled dispatcher already uses.

Changes

Dispatcher (core/tools/aidlc.ts)

  • Replace the loadDelegate switch with DELEGATES: Record<ToolFile, () => Promise<DelegateModule>>. DelegateModule.main is required, so a TOOLS entry without a loader or a delegate module without export main fails tsc under bun run check. Import specifiers stay literal so bun build --compile still bundles every delegate.
  • Split the error: an unrouted tool reports has no in-process delegate; DELEGATES in aidlc.ts is out of step with TOOLS instead of the misleading missing-export message.

Tests (tests/unit/t230-dispatcher-routes.test.ts)

  • Replace the hand-listed four compiled-vs-dev parity cases with a generator over TOOLS × ROUTES. For each of the 33 tools it picks the first route/verb whose resolveAction yields a delegate to that tool (two modes agreeing on a routing error can never pass vacuously), runs it in a fresh sandboxed project (AIDLC_INSTALL_ROOT, AIDLC_BIN_DIR, AIDLC_OFFLINE=1) through both viaDispatcher and viaImportedCompiledMain, and asserts identical output. 30 tools exercised; doctor, init, workspace-sync are excused by an explicit map quoting their non-forbidden networkPolicy; a completeness test fails if a tool is neither exercised nor excused. No bun build; ~12 s.
  • Add a regression that compiles the release projection (dist-release/claude/.claude/tools/aidlc.ts, the entry build-binaries.ts ships) into a real executable and exercises review, context, and summary from another cwd with an empty PATH.

Packager (scripts/package.ts)

  • delegateNames, the list driving the bare bun aidlc-<name>.ts native rewrite and its leftover check, had drifted to 23 of 33 tools (review-brief among the missing). It is now derived from TOOLS. Output-neutral: dist-release hashes identically before and after (the only bare form in any prose is aidlc-directive.ts, not a delegate); the existing leftover checks still reject a rewrite to a non-route.

Copy-channel CLI wrappers

  • aidlc-review-brief.ts, aidlc-orchestrate.ts, aidlc-init.ts, aidlc-lifecycle.ts: the import.meta.main catch now writes {"error":"<message>"} to stderr, the shape the compiled dispatcher's in-process catch and aidlc-lib die() already use, so bun <tool>.ts and aidlc engine <tool> agree. The parity test found the review-brief case on its first run (aidlc-review-brief: Error: Missing --stage <slug>. vs {"error":"Missing --stage <slug>."}); the other three had the same latent divergence. In-main diagnostics are untouched. t299 and t312 decoded that stderr as raw text and now read JSON.parse(stderr).error first; every state, rollback, and audit assertion in them is unchanged.

Docs

  • Document the native invocation in the tool reference.

Evidence the guards work

Rebasing onto main (which added attest to TOOLS in #1052) with the map lacking that entry:

core/tools/aidlc.ts(2020,7): error TS2741: Property '"aidlc-attest.ts"' is missing in type '{ ... }' but required in type 'Record<ToolFile, () => Promise<DelegateModule>>'.

Deleting only the "aidlc-review-brief.ts" line from DELEGATES and running bun test tests/unit/t230-dispatcher-routes.test.ts -t 'aidlc-review-brief.ts via':

Expected to not contain: "has no in-process delegate"
Received: "{\"error\":\"aidlc-review-brief.ts has no in-process delegate; DELEGATES in aidlc.ts is out of step with TOOLS\"}\n"

Both are the drift that shipped #1070, caught before a user could.

Test plan

  • Reproduced the reported error with a native executable compiled from unchanged main: exit code 1 and the exact missing-export message.
  • bash tests/run-tests.sh --debug -P 8 --unit --filter '^(t230-dispatcher-routes|t304-review-brief|t299-first-run-wizard|t312-orchestrate-session-binding|t215-bolt-dag-selfheal)[.]test[.]ts$' --no-llm: 215 passed, 0 failed (t230 147, t304 32, t215 15, t299 12, t312 9).
  • bun scripts/package.ts, bun scripts/package.ts --check (deterministic across two builds, all seven harnesses), bun run typecheck, bun run lint, git diff --check passed.

Native execution was verified on Linux. No live-model tests were needed for this deterministic dispatcher change.

Checklist

  • I have reviewed the contributing guidelines.
  • I have performed a self-review of this change.
  • Changes have been tested.
  • Changes are documented.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 10, 2026
Type the compiled delegate table as Record<ToolFile, () => Promise<DelegateModule>> so adding a TOOLS entry without a loader or a delegate without export main fails typecheck under bun run check. Keep every import specifier literal for native bundling and reject unknown names before awaiting a delegate.
@apackeer
apackeer force-pushed the fix/issue-1070-review-brief-dispatch branch from 6849b31 to f71ab9d Compare September 12, 2026 20:48
@apackeer apackeer changed the title fix: route native review briefs to their renderer fix: make the compiled delegate table exhaustive so native review briefs route Sep 12, 2026
The compiled dispatcher and aidlc-lib die() already emit JSON errors, and the generated parity test exposed the copy-channel mismatch.
…cher

Generate sandboxed dev/compiled parity cases from TOOLS and ROUTES, requiring every delegate to execute or carry an explicit network-policy exception. Assert delegate resolution and reject missing in-process loaders so a routed-but-unbundled tool like review-brief fails at runtime.
The hand-maintained list had drifted to 23 of 33 delegates; deriving it from TOOLS covers every bare invocation while the leftover checks still reject surviving invocations and non-route rewrites.
@apackeer apackeer changed the title fix: make the compiled delegate table exhaustive so native review briefs route fix: make compiled dispatch exhaustive over TOOLS so native review briefs route (#1070) Sep 12, 2026
@apackeer
apackeer force-pushed the fix/issue-1070-review-brief-dispatch branch from a42ffbf to 446dd4d Compare September 12, 2026 23:34
Following the review-brief precedent, the remaining copy-channel wrappers emit the compiled dispatcher JSON error shape while preserving their exit semantics.
Decode wrapper error messages before checking wizard recovery paths and session conflicts while preserving all state and audit assertions.
@apackeer
apackeer added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit be94bde Sep 17, 2026
24 of 29 checks passed
@apackeer
apackeer deleted the fix/issue-1070-review-brief-dispatch branch September 17, 2026 04:11
apackeer added a commit that referenced this pull request Sep 17, 2026
* origin/main:
  fix: make compiled dispatch exhaustive over TOOLS so native review briefs route (#1070) (#1115)
  test: skip POSIX gate-sensor fixture on Windows (#1208)
  fix: make /aidlc compose scopes durable across an engine reinstall (#1159)
  fix: refuse orphaned positionals at intent-create instead of storing a truncated description (#1114) (#1195)
  fix: route compiled gate-sensor dispatch through the engine namespace (#1166)
  fix: name the accepted values in review-path refusals (#1082) (#1194)
  fix: discover stage-level summary questions when units are skipped (#1110)
  fix: keep a JSON-scalar gate reply instead of parsing it away (#1186)
  fix: resolve the zero-Unit code-generation location in the traceability sensor (#1179)
  fix(config): one quiet line for --show, and honest source options on a copy-channel refresh (#1185)
  test(t238): normalize walkFiles separators in the invocation-surface selector (#1184)
  fix(config): Kiro provides its own model access, so the provider section has nothing to ask (#1183)
  chore: prepare 2.9.0 release (#1181)
  feat!: classic scope v1 parity with scope-owned ceremony switches (#1151)
  fix: publish changed previews without a daily cap (#1178)
  fix: separate Bun copy and native installer runtimes (#1174)
  fix: publish next-patch previews nightly (#1169)
  fix: keep review bookkeeping out of the artifact, and out of the reviewer's findings (#1160)
  fix: measure the review budget against the engine's own ordinal, not the caller's (#1158)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

engine review-brief is unroutable in v2.8.0: loadDelegate() has no case for reviewBrief

1 participant