fix: select exact production route executable - #692
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🦋 Changeset detectedLatest commit: b66a8e4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…t-executable # Conflicts: # packages/agent-bundle/src/dev/routes/route-invocation-production.ts # packages/agent-bundle/src/dev/routes/route-invocation-service.ts
Owner-requested review — original #680 findings addressed at the inspected headReviewed The implementation now fixes the specific execution-selection defects from the earlier audit:
The changed compiled integration tests are materially stronger than manifest-only assertions: unrelated server import sentinels stay untouched; import/handler/preflight errors containing the old lookup-error wording do not select another worker; missing preparation/worker records fail; a Claude-only event submitted as Codex is rejected before route import/handler execution; valid denial does not reach the handler. The tests also preserve standalone and projected I found no additional concrete blocker in the inspected changes. Do not carry the original #680 probing/preflight findings forward as still open. Preserve the manifest-owned binding together with the bounded render forwarding adopted from main when subsequent branches are integrated. The updated PR body records the local gate against main This closes the old execution-selection slice; it does not by itself establish native conformance for the 21 newly researched hosts. |
Summary
routes.events[].executionfor shared, standalone, and standalone-fallback event workersDuplicate consolidation
Compared #684 and #692 symbol-by-symbol. #692 survives because it carries the manifest-owned binding from the leased parent into the child, has real compiled multi-server/import-sentinel coverage, and does not add the resolver abstraction prohibited by #680. #684 is closed as superseded and its remote branch is deleted. Ported its unique event execution-record semantics, shared hooks-worker path constant, missing preparation-export regression, standalone-fallback coverage, and bilingual Workbench wording.
Validation
Branch contains current
origin/main(3ddbead90e).pnpm build && pnpm typecheck && pnpm lint && pnpm test:unit— pass (4,425 tests; 4,419 passed, 6 skipped)pnpm test:integration:run packages/agent-bundle/tests/route-invocation-dev-server.test.ts— pass (5 tests)pnpm docs:site:build— pass (language parity; 0 broken links / 28,317 anchors)Review hygiene
Deslop: GPT-5.6 Sol, 5 edits — removed unrelated timeout increases; retained one 60 s child-process timeout only after the full unit gate reproduced the 30 s failure.
Self-review
Reviewer: Claude Fable 5.1 Thinking High.
AB8251: fixed with one manifest-selected direct binding and compiled coverage.routes.events[].execution; added standalone import sentinel and fallback coverage.cli:routes could reach a bin worker and failAB8236: fixed to failAB8251before the child, with a handler sentinel..tsMCP tools: corrected the extension check tocli:routes only and added a successful compiled projected-tool regression.b66a8e45fafound no concrete merge risks.Closes #680