fix(routes): keep bin- and lib-claimed src/scripts modules in script discovery (#389) - #413
Conversation
🦋 Changeset detectedLatest commit: 2920a59 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 |
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. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/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: 5034adb7d4
ℹ️ 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".
commit: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb1b5e80a0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3573579e5d
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
0a134c1 to
b85e382
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b85e382d7c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddae532a81
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9507cea80a
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70f88130ff
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3f4aad6f8
ℹ️ 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".
a3f4aad to
331649c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 331649c221
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da1c6b800f
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
a842ecb to
0a981af
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a981afa3a
ℹ️ 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".
…discovery (#389) A bin (or lib) entry referencing a conventional src/scripts/<name>.ts module used to remove it from route discovery, so the artifact silently lost scripts/<name>.mjs while inspect still listed the bin. The package build writes to dist/, disjoint from every artifact output, so the same entry now ships on both surfaces. scripts, hooks, and mcp entries still claim the module they reference. A bin pointing at a rendered .tsx script is the new AB4737 error: the Server Component cannot double as a bin's main.
…cripts children A nested module a package-build entry names stays claimed; discovering it would only turn a valid package-only configuration into AB4808. Rewrite the changeset summary in the imperative user-facing format ending with the PR reference.
…exports no main The bin envelope prefers a named main export and only falls back to the default export, so a rendered script that also exports main serves both surfaces. Reuse the build's export scan so the gate and the envelope agree.
…ge entries A direct child whose stem is not a safe route identity cannot become an artifact script, so a bin or lib entry naming it stays claimed instead of surfacing AB4803 for a package-only configuration that built before.
…ip inert Both envelopes wrap a main export and bundle self-executing modules alike, but only the bin envelope falls back to a default export; the artifact script would merely define it. Gate that shape instead of publishing an inert scripts/<name>.mjs beside a working bin.
…claimed rendered script A rendered script that exports main but no default component would build a working bin beside an artifact script that fails at run time with nothing to render. AB4737 now names whichever export is missing.
…s claiming A library entry is not an executable surface, so a lib.entry under src/scripts/ has no second envelope to agree with and keeps removing the module from discovery exactly as before.
…ts now require consumer action
…h the route contract scan
A present but non-callable default export (export default {}) built a bin
that worked beside a rendered script that failed at run time. AB4737 now
uses scanRouteModuleExports so the default must be an async function.
…in envelope's scan so type-only exports do not count
…xport scan
export type { X } and export { type X as default } emit no JavaScript
binding, so they must not satisfy the component or named-export contract;
this closes the last dual-surface hole where a bin-claimed rendered script
passed AB4737 with a type-only default alias.
…d rendered script
0a981af to
2920a59
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
main (#413) made a bin entry stop claiming a direct src/scripts/<name> module, so the same file ships as the npm bin and the artifact script. The Scripts page now states that rule and the AB4737/AB4738 export requirements in both locales.
1 participant
Summary
Closes #389.
binentry that references a conventionalsrc/scripts/<name>.tsmodule no longer removes it from script discovery.configClaimedSources()inroutes/graph.tsnow reportsbinclaims separately from every other claim (scripts,hooks,lib,mcp); abinclaim on a directsrc/scripts/<name>child keeps the module a conventional script, so the same entry ships as bothdist/bin/<name>.jsand<target>/scripts/<name>.mjs, andinspectlists it under bothpackageBuild.binsandscripts. Every other route kind (src/cli/**,src/events/**,src/mcp/**,src/providers/*) and any nestedsrc/scripts/<dir>/<name>module still leaves discovery under any claim (so a package-only nested entry never regresses into AB4808), and an explicitscripts:claim still takes precedence over the convention (existing AB4808/AB4809 design untouched).AB4737error (verified unused onorigin/mainand in every open PR): abinentry pointing at a renderedsrc/scripts/<name>.tsx/.jsxscript that does not export both an async default Server Component (judged by the route compiler'sscanRouteModuleExports, soexport default {}and type-only aliases do not count, while a default re-exported from another module is accepted and left to the worker's run-time check) and a namedmain. The bin envelope prefersmainand only falls back to the default export, so withoutmainthe component would be called asmain(argv); without the component the rendered script fails at run time. The compiler refuses either shape instead of emitting a broken surface beside a working one, naming the missing export. A rendered script exporting both serves both surfaces and is not gated (detection reuses the build's ownscanEntryExportsSource, so gate and envelope agree). The message names everybinentry referencing the module and the recovery lists the ways out (exportmain, plain module for the bin, rename to.ts, or_-prefix for bin-only).AB4738error (also verified unused): abinentry pointing at a plainsrc/scripts/<name>.tsthat exports adefaultbut nomain. Both envelopes wrapmainand bundle self-executing modules identically, but only the bin envelope falls back to a default export — the artifactscripts/<name>.mjswould merely define it and ship inert. Rejected rather than changing every plain script's default-export semantics.lib.entrystill claims its module like every other key: a library is not an executable surface, so there is no second envelope for it to agree with.binclaims on a nested or unsafely namedsrc/scripts/**module (which the flat scripts artifact could not ship anyway) stay claimed, so a bin-only configuration never regresses into AB4808/AB4803.inspectalready shows both surfaces. The precedence is documented instead.routes/contract.ts:scanRouteModuleExportsnow skips type-only export declarations and specifiers, soexport type { main }/export { type X as default }satisfy no runtime contract (benefits every route contract check).docs/entry-conventions.mdgains a "Which config keys claim a conventional module" table (which keys claim which conventional directories;bin/libdo not remove asrc/scriptsmodule) and thesrc/scriptsrows reference it;docs/diagnostics.mddocumentsAB4737and the claim rule;packages/agent-bundle/README.mdstates the rule in the package-build paragraph. Changeset:.changeset/389-bin-claim-script-discovery.md(minorforagent-bundle: a previously bin-only default-exportsrc/scriptsmodule now needsmainor a_prefix, which is consumer action under the pre-1.0 rule).Not included (see issue comment):
simulateHook()per-call cost. Each call runstemporaryArtifact(), i.e. a fullbuild()into a temp directory that is removed afterwards — the ~7 s is the build, not a "prepare" step, and a memoization keyed on project root + config mtime would have to keep the temp artifact alive across calls and would go stale on any source edit, so it is not a straightforward api-layer cache.SimulateHookOptions.artifactalready lets a consumer build once and simulate many times against that artifact.Evidence
packages/agent-bundle/tests/route-graph.test.ts—keeps a bin-claimed src/scripts module in script discovery while lib still claims (#389): abinclaim on a directsrc/scripts/*child keeps the script route whilelib.entryon another still claims it; the samebinclaim onsrc/cli/doctor.tsstill removes the CLI route a nestedsrc/scripts/internal/tool.tsbin stays claimed (no AB4808), and a spaced-stemsrc/scripts/my tool.tsbin stays claimed (no AB4803).packages/agent-bundle/tests/route-graph.test.ts—gates a bin-claimed plain script with AB4738 only when its bin would run a default export the script ignores (#389): real files;default-only.tsgated,hauler.ts(main) and self-executingplain.tspass.packages/agent-bundle/tests/route-graph.test.ts—gates a bin-claimed rendered script with AB4737 only when it exports no main (#389): real files;render-notes.tsx(component +main) passes,render-poster.tsx(component only),render-tool.tsx(mainonly),render-object.tsx(main+ non-callable default), andrender-typed.tsx(main+ type-only default alias) are the four AB4737s with distinct messages;render-reexport.tsx(main+ re-exported default) passes; all six stay discovered.packages/agent-bundle/tests/normalization.test.ts—ships a bin-claimed plain conventional script as both surfaces and refuses a bin-claimed rendered one with AB4737 (#389): exact AB4737 diagnostic (two bins naming one rendered script), plain script present inmodel.scriptsbeside its bin inmodel.packageBuild.bins.packages/agent-bundle/tests/package-build.test.ts—ships a bin-claimed src/scripts module as both the npm bin and the artifact script (#389): real build withbin: { hauler: './src/scripts/hauler.ts' }; no diagnostics;model.packageBuild.binsandmodel.scriptsboth list it;dist/bin/hauler.jsandartifact/portable/scripts/hauler.mjsboth exist and execute.Gates (worktree, load average 70–100):
pnpm typecheck✅,pnpm lint✅,pnpm test:projection✅ (66/66),pnpm test:unit2729 passed / 2 five-second timeouts in unrelated files (inspect-state.test.tspasses in isolation;native-claude-contract.test.ts :: fails closed when …snapshots the real~/.claudehome six times and exceeds 5 s on this loaded machine in isolation too — environmental, untouched by this diff),pnpm test:route-unit36 passed / 1 five-second timeout (lifecycle-replay.test.ts, passes in isolation),pnpm build✅,pnpm test:integration:run— see CI.Test plan
rstest --config rstest.unit.config.ts packages/agent-bundle/tests/route-graph.test.ts packages/agent-bundle/tests/normalization.test.tsrstest --config rstest.integration.config.ts packages/agent-bundle/tests/package-build.test.ts(16/16)pnpm typecheck,pnpm lint,pnpm buildpnpm test:unit,pnpm test:route-unit,pnpm test:projection(flakes rerun in isolation as noted)