diff --git a/.changeset/389-bin-claim-script-discovery.md b/.changeset/389-bin-claim-script-discovery.md new file mode 100644 index 000000000..971b38273 --- /dev/null +++ b/.changeset/389-bin-claim-script-discovery.md @@ -0,0 +1,5 @@ +--- +"agent-bundle": minor +--- + +Keep a conventional `src/scripts/.ts` module in script discovery when a `bin` config entry also references it, so `agent-bundle build` emits both `dist/bin/.js` and the artifact `scripts/.mjs` and `agent-bundle inspect --json` lists the module under both `packageBuild.bins` and `scripts` instead of silently dropping the script. Explicit `scripts`, `hooks`, `lib`, and `mcp` entries still claim the module they reference. Report `AB4737` when a `bin` entry points at a rendered `src/scripts/.tsx` script that does not export both its default Server Component and a named `main`, because the bin envelope calls `main` while the artifact script renders the component, and `AB4738` when it points at a plain `src/scripts/.ts` script that exports a `default` but no `main`, because the artifact script would ship inert while the bin runs the default export. (#413) diff --git a/docs/diagnostics.md b/docs/diagnostics.md index 1c134e17a..91f0ea35e 100644 --- a/docs/diagnostics.md +++ b/docs/diagnostics.md @@ -195,16 +195,27 @@ development-only fallback can never produce a release artifact, so | `AB4011` | warning | `package.json` is unusable — unparsable, not a JSON object, or symlinked outside the project root. | | `AB4013` | error (build) | `agent-bundle build` refuses a project with no release version: `plugin.version` is omitted and `package.json` declares no valid semantic version. | -## Migration nudges (`AB4730`–`AB4736`) +## Migration nudges and convention claims (`AB4730`–`AB4738`) The entry conventions and the framework-owned stdio lifecycle shell (RFC #50) replaced patterns consumers previously wrote by hand. When `validate`, `inspect`, `build`, or `dev` prepares project source and finds one of those pre-convention patterns, it reports a migration diagnostic. `AB4730`–`AB4735` -are **informational** nudges and never block anything. `AB4736` is an error: -the removed top-level authored-document locations are no longer discovered, -so the compiler refuses to omit them silently. The CLI prints these in human -`validate` output and includes them in every `--json` diagnostics array. +are **informational** nudges and never block anything. `AB4736`–`AB4738` are +errors: the removed top-level authored-document locations are no longer +discovered, and a conventional script whose `bin` entry would run an export +the artifact script ignores cannot ship on both surfaces, so the compiler +refuses to omit or misbuild them silently. The CLI prints these in +human `validate` output and includes them in every `--json` diagnostics array. + +Which explicit config keys *claim* a conventional module out of discovery is +tabulated in `docs/entry-conventions.md` ("Which config keys claim a +conventional module"). In short: `scripts`, `hooks`, `lib`, and `mcp` entries +claim the module they reference; a `bin` entry claims every conventional +module **except** a safely named direct `src/scripts/` child, which +keeps shipping as an artifact script beside the bin because the two outputs +are disjoint and both envelopes run the same `main`. That dual-surface shape +is intentional and raises no diagnostic. ### `AB4730` — self-connecting stdio MCP entry @@ -276,6 +287,52 @@ Recover: move the document under `src/skills/`, `src/commands/`, or `src/rules/`. Explicit `skills` paths remain valid anywhere. Published artifact paths remain `skills/`, `commands/`, and `rules/`. +### `AB4737` — rendered script claimed as a package bin entry lacks `main` or the component + +An explicit `bin` entry references a conventional rendered script +(`src/scripts/.tsx` or `.jsx`) that does not export **both** an async +default Server Component and a named `main`. The component check is the +route compiler's own static scan — the default export must be an async +function, so `export default {}` does not count; a default re-exported from +another module (`export { default } from './component.tsx'`) cannot be judged +statically and is accepted (the rendered worker still verifies it at run +time). A plain `src/scripts/.ts` module +ships happily on both surfaces — the npm bin envelope calls its `main(argv)` +and the artifact script is the same bundle — but a rendered script's default +export is an async Server Component the Agent renderer drives with +`{ argv, signal }` props. The bin envelope prefers a named `main` export and +only falls back to the default export, so without `main` it would call that +component as `main(argv)` and produce a bin that renders nothing; without the +default component, the bin works but `scripts/.mjs` fails at run time +with no component to render. The compiler refuses either shape instead of +emitting a broken surface beside a working one. A rendered script that +exports both serves both surfaces and is not gated. The message names every +`bin` entry referencing the module and which export is missing. + +Recover: export both an async default Server Component and a named +`main(argv)` from the module; point the `bin` entry at a plain module that exports `main`; +rename the script to `.ts` so one plain module ships as both the bin and the +artifact script; or prefix a path segment with `_` (`src/scripts/_name.tsx`) +to keep the module out of script discovery and bin-only. + +### `AB4738` — plain script claimed as a package bin entry runs only as the bin + +An explicit `bin` entry references a conventional plain script +(`src/scripts/.ts`) that exports a `default` but no named `main`. Both +the bin envelope and the artifact-script envelope wrap a `main(argv)` export +and bundle a self-executing module (no `main`, no `default`) byte for byte, +so those shapes run identically on both surfaces. Only the bin envelope falls +back to invoking a default export: the artifact `scripts/.mjs` would +merely define the function and exit, so a successful build would publish an +inert script beside a working bin. The detection is the same static export +scan the package build uses. The message names every `bin` entry referencing +the module. + +Recover: export a named `main(argv)` so both surfaces run the same entry; +make the module self-executing (drop the default export and run at top +level); or prefix a path segment with `_` (`src/scripts/_name.ts`) to keep +the module out of script discovery and bin-only. + ## Prebuilt payloads (`AB4740`–`AB4750`) The `payload` block and `{ prebuilt: ... }` entries (see diff --git a/docs/entry-conventions.md b/docs/entry-conventions.md index 006154c55..2396cb7cb 100644 --- a/docs/entry-conventions.md +++ b/docs/entry-conventions.md @@ -77,8 +77,8 @@ entries carry `provenance.kind: 'conventional'` in the normalized model. | `src/mcp/.ts` | Stdio entry for the declared MCP server `` that names no `entry`, `command`, or `url`. | Declare `entry` explicitly | | `src/mcp//{tools,resources,prompts}/*.{ts,tsx}` | Generated MCP server routes; path supplies identity and each executable module supplies static `config`, schemas, and one async default Server Component. | Set `routes.servers.` to `custom`, `command`, or `remote` | | `src/mcp//apps/*.{ts,tsx}` | Browser MCP App entry compiled to self-contained HTML and registered on the generated server; static `config.resourceUri` is required. An optional `config.template` HTML shell resolves relative to the route module like its imports (`'./dashboard.html'`); the legacy project-root-relative form is accepted only while unambiguous (`AB4827` otherwise). Tools, resources, and prompts reference the App from their own static `config` with `appResourceUri('')` from `agent-bundle/routes` or a shared `const` string literal instead of repeating the `ui://` literal. | Use a custom server or prefix the file with `_` | -| `src/scripts/.ts` | Plain script compiled to `scripts/.mjs` in every selected target artifact — the same pipeline explicit `scripts` entries use, with ordinary Node stdout/stderr semantics. A `scripts` entry that references the file claims it. Nested modules are hard errors (`AB4808`). | Prefix a path segment with `_`, or claim the file with an explicit `scripts` entry | -| `src/scripts/.tsx` | Rendered script: the async default component receives `{ argv, signal }` and renders through the Agent renderer with the CLI output contract (`--json`, `--ndjson`, TTY progress, piped Markdown). Compiles to `scripts/.mjs` plus a `scripts/-flight.mjs` react-server worker. The extension is the explicit, visible contract — plain `.ts` scripts are never wrapped in React behavior, and explicit `scripts` config entries stay plain regardless of extension. | Rename to `.ts`, prefix a path segment with `_`, or claim the file with an explicit `scripts` entry | +| `src/scripts/.ts` | Plain script compiled to `scripts/.mjs` in every selected target artifact — the same pipeline explicit `scripts` entries use, with ordinary Node stdout/stderr semantics. A `scripts` entry that references the file claims it. Nested modules are hard errors (`AB4808`). A `bin` entry that references the file does **not** claim it: the module ships as both the npm bin and the artifact script (see [Which config keys claim a conventional module](#which-config-keys-claim-a-conventional-module)); export `main` or make the module self-executing, because a `default`-only module would run as the bin but ship as an inert script (`AB4738`). | Prefix a path segment with `_`, or claim the file with an explicit `scripts` entry | +| `src/scripts/.tsx` | Rendered script: the async default component receives `{ argv, signal }` and renders through the Agent renderer with the CLI output contract (`--json`, `--ndjson`, TTY progress, piped Markdown). Compiles to `scripts/.mjs` plus a `scripts/-flight.mjs` react-server worker. The extension is the explicit, visible contract — plain `.ts` scripts are never wrapped in React behavior, and explicit `scripts` config entries stay plain regardless of extension. A `bin` entry that references a rendered script is `AB4737` unless the module exports both the default component (for the script) and a named `main` (for the bin envelope); with both, the module serves both surfaces. | Rename to `.ts`, prefix a path segment with `_`, or claim the file with an explicit `scripts` entry | | `src/cli/**/*.{ts,tsx}` | Routed CLI commands compiled into one collision-checked command graph and one generated package executable named after `plugin.name` (superseding the `src/cli.ts` bin convention for the project). Nesting is identity: `src/cli/library/audit.ts` runs as ` library audit`. Plain `.ts` commands execute directly and print one canonical JSON line; `.tsx` commands render through the dispatcher with the four output modes. | `bin: false`, `routes.cli: 'conventional'`, or prefix a path segment with `_` | | `src/events//.{ts,tsx}`, `src/events/stop.{ts,tsx}` | Semantic event route: the path is the canonical event family (`src/events/tool/after.tsx` is `tool/after`; `stop` is the one top-level family) and must be one of the admitted `canonicalAgentEvents`. The optional static `config` (`AgentEventRouteConfig`: `targets`, `tools`, `runtime: 'shared' \| 'standalone'`, `fallback`, `delivery`, `timeoutMs`) restricts hosts and selects the execution mode; the async default Server Component receives `AgentEventRouteProps` (`{ canonical, native, signal }`) and returns `Agent.*` output that the selected host adapter encodes into its native hook envelope. Application code never branches on host JSON or emits native hook documents; per-host support is a capability state (`supported`/`degraded`/`unavailable`/`prohibited`) surfaced by `inspect` and enforced at build time (`AB4817`, `AB4823`–`AB4825`). | Restrict `config.targets`, or prefix a path segment with `_` | | `src/state.ts` | Project state definition: default-exports `defineState({ ... })`; generated MCP, routed-CLI, and rendered-script request scopes mount `(await agent()).state` and `.notices`. | `state: false`, or rename the file to `_state.ts` | @@ -87,6 +87,36 @@ entries carry `provenance.kind: 'conventional'` in the normalized model. Route and package entry conventions match `.ts` and `.tsx` files exactly; the state convention is specifically `src/state.ts`. +### Which config keys claim a conventional module + +An explicit config entry that references a module under a conventional route +directory *claims* it: the module belongs to that declaration and leaves +conventional discovery. Claims are decided by the module path the entry +resolves to, so nothing is ever compiled twice into one artifact output. The +exception is `bin`: a bin compiles to `dist/bin/.js`, which is disjoint +from every artifact output, and the bin envelope and the artifact-script +envelope run the same `main`, so a direct `src/scripts/` child a `bin` +entry references stays a conventional script and ships on both surfaces. (A +nested `src/scripts//` module or one whose stem is not a safe route +identity — which the flat scripts artifact could not ship anyway — stays +claimed, so a bin-only entry there never turns into `AB4808` or `AB4803`.) +`inspect` shows such a module under both `packageBuild.bins` and `scripts`; no +diagnostic fires, because that is the intended "same entry, npm bin + hook +target" shape. `lib` is not an executable surface, so a `lib.entry` claims +its module like every other key. + +| Config key | Claims a module under | Effect on a `src/scripts/.ts` module it references | +| --- | --- | --- | +| `scripts.` | every route directory | Claimed: the explicit entry ships it as `scripts/.mjs`; the convention no longer applies. A *different* module under `src/scripts/` sharing the configured `` is `AB4809`. | +| `hooks.[].handler` | every route directory | Claimed: the module is a hook handler compiled under `hooks/`, not an artifact script. | +| `mcp.servers..entry`, `mcp.servers..apps..entry` / `.template` | every route directory | Claimed: the module is the server or App entry compiled under `mcp/` or `mcp-apps/`. | +| `lib.entry` | every route directory | Claimed: the module is the library entry compiled to `dist/.js`, not an artifact script. | +| `bin.` | `src/cli/**`, `src/events/**`, `src/mcp/**`, `src/providers/*`, and any `src/scripts/**` module that is nested or unsafely named — **not** a safely named direct `src/scripts/` child | Not claimed: the module ships as both `dist/bin/.js` and `scripts/.mjs`. The module must export `main` or be self-executing: a plain `default`-only module is `AB4738`, and a rendered `src/scripts/.tsx` must export both the default component and `main`, otherwise `AB4737`. | + +To ship a `src/scripts/` module as a bin only, prefix a path segment with `_` +(`src/scripts/_hauler.ts`): private segments opt the module out of discovery +while the `bin` entry still references it explicitly. + ### Config beside a route-generated MCP server A `mcp.servers.` block whose `` the route graph compiles in diff --git a/packages/agent-bundle/README.md b/packages/agent-bundle/README.md index b46f56646..3b89daed4 100644 --- a/packages/agent-bundle/README.md +++ b/packages/agent-bundle/README.md @@ -85,7 +85,13 @@ agent-bundle also owns the npm-facing package build: `bin` entries become self-e `dist/bin/.js` bundles (shebang, executable bit, generated `main(argv)` envelope) and the optional `lib` entry becomes `dist/.js` with declarations (resolving `typescript` from the project). The `src/cli.ts`, `src/index.ts`, and `src/mcp/.ts` conventions fill these in -when the config is silent; config always wins and `bin: false` / `lib: false` opt out. MCP server +when the config is silent; config always wins and `bin: false` / `lib: false` opt out. An explicit +`scripts`, `hooks`, `lib`, or `mcp` entry claims the module it references out of conventional route +discovery, but a `bin` entry does not claim a `src/scripts/.ts` module: the same file +ships as both `dist/bin/.js` and the artifact `scripts/.mjs` (the module must export +`main` or be self-executing: a `default`-only plain script is `AB4738`, and a rendered `.tsx` script +must export both its default component and `main` or it is `AB4737`; prefix a path segment with `_` +for a bin-only module). MCP server entries that default-export a server factory are wrapped in the framework stdio lifecycle shell (console-to-stderr guard with raw stdout restored for protocol frames, SIGINT 130 / SIGTERM 143, stdin-EOF exit 0, bounded shutdown, heartbeat), also available directly from diff --git a/packages/agent-bundle/src/config/validate.ts b/packages/agent-bundle/src/config/validate.ts index 69e524103..f702a6981 100644 --- a/packages/agent-bundle/src/config/validate.ts +++ b/packages/agent-bundle/src/config/validate.ts @@ -1,7 +1,7 @@ import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from 'node:fs'; import { basename, extname, isAbsolute, join, posix, relative, resolve, sep } from 'node:path'; -import { scanEntryExportsSource } from '../build/entry-exports.ts'; +import { type EntryExportScan, scanEntryExportsSource } from '../build/entry-exports.ts'; import { toPosixRelative } from '../core/paths.ts'; import { isPlainRecord, isRecord } from '../core/strict-json.ts'; import type { Diagnostic } from '../core/diagnostics.ts'; @@ -18,6 +18,7 @@ import { satisfiesGeneratedRuntimeFloor, } from '../core/runtime.ts'; import { canonicalHookEvents, isPrebuiltEntryInput, parseNativeHookToolSelector } from '../core/types.ts'; +import { type RouteModuleExports, scanRouteModuleExports } from '../routes/contract.ts'; import type { AgentBundleBinEntry, AgentBundleHookEntry, @@ -1865,21 +1866,111 @@ const validatePackageIdentity = (loaded: LoadedConfig, release: boolean): Diagno * resolution. Discovery is not a packaging choice - a route never * disappears silently. */ +/** The explicit `bin` names claiming each absolute entry source, tolerant of malformed config shapes. */ +const explicitBinNamesBySource = (loaded: LoadedConfig): ReadonlyMap => { + const bin = loaded.config.bin; + const names = new Map(); + if (!isRecord(bin)) return names; + for (const [name, declaration] of Object.entries(bin)) { + const entry = typeof declaration === 'string' + ? declaration + : isRecord(declaration) ? (declaration as AgentBundleBinEntry).entry : undefined; + if (!nonemptyString(entry)) continue; + const source = resolve(loaded.context.projectRoot, entry); + names.set(source, [...(names.get(source) ?? []), name]); + } + return names; +}; + +/** + * The build's own static export scan of one conventional script, so the + * dual-surface gates below agree with the bin envelope's export selection + * (`main` first, then `default`). Undefined when the module is unreadable. + */ +const scriptEntryExports = (source: string): EntryExportScan | undefined => { + try { + return scanEntryExportsSource(readFileSync(source, 'utf8')); + } catch { + return undefined; + } +}; + +/** + * The route compiler's static export scan of one rendered script, which + * judges the default export's component shape (an async function) rather + * than its mere presence. Undefined when the module is unreadable. + */ +const renderedScriptExports = (source: string, relativePath: string): RouteModuleExports | undefined => { + try { + return scanRouteModuleExports(readFileSync(source, 'utf8'), relativePath); + } catch { + return undefined; + } +}; + const validateConventionalScripts = ( loaded: LoadedConfig, discovered: DiscoveredProject, ): Diagnostic[] => { const diagnostics: Diagnostic[] = []; const configured = configuredScriptNames(loaded.config); + const binNamesBySource = explicitBinNamesBySource(loaded); for (const route of discovered.routeGraph?.scripts ?? []) { const relativePath = route.provenance.relativePath; const judgment = judgeScriptRoute(route, configured); + const binNames = binNamesBySource.get(route.source); + const binList = binNames?.map((name) => JSON.stringify(name)).join(', '); switch (judgment) { - case 'shippable': + case 'shippable': { + // A plain script a `bin` entry also names ships as both surfaces: the + // npm bin and the artifact script are disjoint outputs (#389). Both + // pipelines wrap a `main` export in the process envelope and bundle a + // self-executing module byte for byte, so those shapes agree. Only + // the bin envelope falls back to a default export; the artifact + // script would merely define it, so that shape is gated. + const exports = binNames === undefined ? undefined : scriptEntryExports(route.source); + if (exports !== undefined && !exports.hasMainExport && exports.hasDefaultExport) { + diagnostics.push({ + code: 'AB4738', + message: `Script ${relativePath} is also the entry of bin ${binList} and exports a default but no main; the bin envelope would run the default export while the artifact script would only define it.`, + recovery: 'Export a named main(argv) so both the bin and the artifact script run the same entry, make the module self-executing (no default export), or prefix a path segment with "_" to keep the module bin-only.', + severity: 'error', + sourcePath: route.source, + }); + } + break; + } // Rendered scripts ship through the Agent renderer pipeline (#102 // stage 3); AB4807 is retired and never reused. - case 'rendered': + case 'rendered': { + // The rendered-script default export is a Server Component the + // renderer drives. The bin envelope prefers a named `main` export and + // only falls back to the default export, so the two surfaces can share + // one module exactly when it exports both; the detection is the + // build's own export scan, so the gate and the envelope always agree. + if (binNames === undefined) break; + // `main` is judged by the bin envelope's own scan (which ignores + // type-only exports); the component by the route compiler's scan (an + // async default function, not mere default-export presence, since + // `export default {}` would build and fail at run time). A default + // re-exported from another module (`export { default } from`) cannot + // be judged statically and is accepted; the worker still verifies it. + const hasMain = scriptEntryExports(route.source)?.hasMainExport === true; + const routeExports = renderedScriptExports(route.source, relativePath); + const hasComponent = routeExports?.asyncDefault === true || routeExports?.named.has('default') === true; + if (hasMain && hasComponent) break; + const missing = !hasMain && !hasComponent + ? 'neither an async default Server Component nor a named main' + : hasMain ? 'no async default Server Component' : 'no named main'; + diagnostics.push({ + code: 'AB4737', + message: `Rendered script ${relativePath} is also the entry of bin ${binList} but exports ${missing}; the artifact script renders the default component and the bin envelope calls main(argv).`, + recovery: 'Export both an async default Server Component and a named main(argv) from the module, point the bin entry at a plain module that exports main, rename the script to .ts so one plain module ships as both the bin and the artifact script, or prefix a path segment with "_" to keep the module bin-only.', + severity: 'error', + sourcePath: route.source, + }); break; + } case 'nested': diagnostics.push({ code: 'AB4808', diff --git a/packages/agent-bundle/src/routes/contract.ts b/packages/agent-bundle/src/routes/contract.ts index 7d2adb71f..b5f7a292a 100644 --- a/packages/agent-bundle/src/routes/contract.ts +++ b/packages/agent-bundle/src/routes/contract.ts @@ -91,7 +91,11 @@ export const scanRouteModuleExports = ( continue; } if (ts.isExportDeclaration(statement) && statement.exportClause !== undefined && ts.isNamedExports(statement.exportClause)) { + // Type-only exports (`export type { X }`, `export { type X as default }`) + // emit no JavaScript binding, so they satisfy no runtime contract. + if (statement.isTypeOnly) continue; for (const element of statement.exportClause.elements) { + if (element.isTypeOnly) continue; const name = element.name.text; if (name === 'default' && statement.moduleSpecifier === undefined) { defaultIdentifier = element.propertyName?.text ?? name; diff --git a/packages/agent-bundle/src/routes/graph.ts b/packages/agent-bundle/src/routes/graph.ts index 73dbcb2b0..71e96e462 100644 --- a/packages/agent-bundle/src/routes/graph.ts +++ b/packages/agent-bundle/src/routes/graph.ts @@ -203,23 +203,43 @@ const claimedModuleEntry = (value: unknown): string | undefined => { return undefined; }; +interface ConfigClaimedSources { + /** + * Modules an explicit `scripts`, `hooks`, `lib`, or `mcp` declaration + * references. They belong to that declaration and never become + * conventional routes. + */ + readonly artifact: ReadonlySet; + /** + * Modules explicit `bin` entries compile. They leave route discovery too, + * except a direct `src/scripts/` child: `dist/bin/.js` and + * `/scripts/.mjs` are disjoint outputs and both envelopes + * run the same `main`, so one entry ships as both an npm bin and an + * artifact script instead of silently losing the script (#389). + */ + readonly bin: ReadonlySet; +} + /** * Absolute module paths explicit configuration already claims. Config always * wins — the rule the entry conventions established — so a module an explicit - * `scripts`, `hooks`, `bin`, `lib`, or `mcp` declaration references belongs - * to that declaration and never becomes a conventional route. Two shipped - * examples declare `scripts` entries under `src/scripts/`; this rule keeps - * their layouts route-free without a migration. + * `scripts`, `hooks`, `lib`, or `mcp` declaration references belongs to that + * declaration and never becomes a conventional route. Two shipped examples + * declare `scripts` entries under `src/scripts/`; this rule keeps their + * layouts route-free without a migration. `bin` claims are reported + * separately because a bin coexists with a conventional script. */ const configClaimedSources = ( projectRoot: string, config: Readonly, -): ReadonlySet => { - const claimed = new Set(); - const claim = (value: unknown): void => { +): ConfigClaimedSources => { + const artifact = new Set(); + const bin = new Set(); + const claimInto = (claimed: Set, value: unknown): void => { const entry = claimedModuleEntry(value); if (entry !== undefined && entry.trim().length > 0) claimed.add(resolve(projectRoot, entry)); }; + const claim = (value: unknown): void => claimInto(artifact, value); const scripts = configValue(config, 'scripts'); if (isRecord(scripts)) { for (const value of Object.values(scripts)) claim(value); @@ -232,9 +252,9 @@ const configClaimedSources = ( } } } - const bin = configValue(config, 'bin'); - if (isRecord(bin)) { - for (const value of Object.values(bin)) claim(value); + const bins = configValue(config, 'bin'); + if (isRecord(bins)) { + for (const value of Object.values(bins)) claimInto(bin, value); } claim(configValue(config, 'lib')); const mcp = configValue(config, 'mcp'); @@ -249,7 +269,22 @@ const configClaimedSources = ( claim(app.template); } } - return claimed; + return { artifact, bin }; +}; + +/** + * True for a direct child of the conventional scripts root whose stem is a + * safe route identity — the only shape the flat scripts artifact can ship. A + * nested or unsafely named module a `bin` entry names stays claimed: keeping + * it discovered would only turn a valid bin-only configuration into an + * AB4808 or AB4803 error. + */ +const isConventionalScriptPath = (relativePath: string): boolean => { + const segments = relativePath.split('/'); + return segments.length === 3 + && segments[0] === 'src' + && segments[1] === 'scripts' + && safeIdentitySegment.test(stemOf(segments[2]!)); }; interface RouteModeOverrides { @@ -530,8 +565,9 @@ export const compileRouteGraph = async ( const modulesById = new Map(); const providerModulesByKey = new Map(); for (const source of sources) { - if (claimed.has(source)) continue; + if (claimed.artifact.has(source)) continue; const relativePath = toPosixPath(relative(projectRoot, source)); + if (claimed.bin.has(source) && !isConventionalScriptPath(relativePath)) continue; if (isPrivateRoutePath(relativePath) || isProjectPathIgnored(rules, projectRoot, source)) continue; const module = classifyModule(source, relativePath); if ( diff --git a/packages/agent-bundle/tests/normalization.test.ts b/packages/agent-bundle/tests/normalization.test.ts index 9102676f5..9b5626776 100644 --- a/packages/agent-bundle/tests/normalization.test.ts +++ b/packages/agent-bundle/tests/normalization.test.ts @@ -1185,6 +1185,39 @@ it('gates nested and conflicting conventional script routes as AB4808/AB4809 and ]); }); +it('ships a bin-claimed plain conventional script as both surfaces and refuses a bin-claimed rendered one with AB4737 (#389)', async () => { + const root = '/workspace/project'; + const loaded = loadedProject({ + bin: { + hauler: './src/scripts/hauler.ts', + notes: { entry: './src/scripts/render-notes.tsx' }, + 'notes-again': './src/scripts/render-notes.tsx', + }, + plugin: { name: 'review-tools', version: '1.0.0' }, + }); + const discovered: DiscoveredProject = { + routeGraph: routeGraphWithScripts(root, ['src/scripts/hauler.ts', 'src/scripts/render-notes.tsx']), + skills: [], + }; + + // The fixture root has no files, so the export scan finds neither export and the gate fires. + const gate = validateSource(loaded, discovered, registry).filter(({ code }) => code === 'AB4737'); + expect(gate).toEqual([ + { + code: 'AB4737', + message: 'Rendered script src/scripts/render-notes.tsx is also the entry of bin "notes", "notes-again" but exports neither an async default Server Component nor a named main; the artifact script renders the default component and the bin envelope calls main(argv).', + recovery: 'Export both an async default Server Component and a named main(argv) from the module, point the bin entry at a plain module that exports main, rename the script to .ts so one plain module ships as both the bin and the artifact script, or prefix a path segment with "_" to keep the module bin-only.', + severity: 'error', + sourcePath: `${root}/src/scripts/render-notes.tsx`, + }, + ]); + + // The plain script stays in the model beside its bin; nothing about it is gated. + const model = await normalizeProject(loaded, discovered, registry); + expect(model.scripts.map((script) => script.name)).toEqual(['hauler', 'render-notes']); + expect(model.packageBuild?.bins.map((bin) => bin.name)).toEqual(['hauler', 'notes', 'notes-again']); +}); + it('normalizes rendered conventional script routes onto the renderer pipeline (#102 stage 3)', async () => { const root = '/workspace/project'; const model = await normalizeProject( diff --git a/packages/agent-bundle/tests/package-build.test.ts b/packages/agent-bundle/tests/package-build.test.ts index ad6fbb6f7..541aac033 100644 --- a/packages/agent-bundle/tests/package-build.test.ts +++ b/packages/agent-bundle/tests/package-build.test.ts @@ -115,6 +115,45 @@ describe('framework-owned package build', () => { expect(rebuilt.packageBuild?.files).toEqual(packageBuild!.files); }, 120_000); + it('ships a bin-claimed src/scripts module as both the npm bin and the artifact script (#389)', async () => { + const root = await fixtureRoot({ + 'agent-bundle.config.ts': [ + 'export default {', + " bin: { hauler: './src/scripts/hauler.ts' },", + " plugin: { name: 'package-build-fixture', version: '1.0.0' },", + " targets: ['portable'],", + '};', + '', + ].join('\n'), + 'package.json': '{"name":"package-build-fixture","type":"module","private":true}\n', + 'src/scripts/hauler.ts': [ + 'export const main = async (argv: readonly string[]): Promise => {', + " process.stdout.write(`hauled:${argv.join(',')}\\n`);", + ' return 0;', + '};', + '', + ].join('\n'), + }); + const result = await build({ output: 'artifact', packageOutputs: true, root }); + + // The same entry is visible on both surfaces of the inspect model, and + // the claim itself raises no diagnostic: this is the intended shape. + expect(result.diagnostics).toEqual([]); + expect(result.model.packageBuild).toMatchObject({ + bins: [{ name: 'hauler', provenance: { kind: 'config' }, source: join(root, 'src/scripts/hauler.ts') }], + }); + expect(result.model.scripts).toMatchObject([ + { name: 'hauler', provenance: { kind: 'conventional' }, source: join(root, 'src/scripts/hauler.ts') }, + ]); + + // Both outputs exist and run. + expect(result.packageBuild?.files.map((file) => file.path)).toContain('bin/hauler.js'); + await expect(execFile(join(root, 'dist', 'bin', 'hauler.js'), ['alpha'])).resolves.toMatchObject({ stdout: 'hauled:alpha\n' }); + expect(result.build.outputProvenance.map((record) => record.path)).toContain('portable/scripts/hauler.mjs'); + const script = join(root, 'artifact', 'portable', 'scripts', 'hauler.mjs'); + await expect(execFile(process.execPath, [script, 'beta'])).resolves.toMatchObject({ stdout: 'hauled:beta\n' }); + }, 120_000); + it('wraps factory-exporting MCP entries in the lifecycle shell and leaves self-connecting entries alone', async () => { const root = await fixtureRoot({ ...conventionFixture(), diff --git a/packages/agent-bundle/tests/route-graph.test.ts b/packages/agent-bundle/tests/route-graph.test.ts index f302afa02..50293ae78 100644 --- a/packages/agent-bundle/tests/route-graph.test.ts +++ b/packages/agent-bundle/tests/route-graph.test.ts @@ -258,6 +258,169 @@ it('never compiles a module explicit configuration claims: config always wins', expect('routeGraph' in discovered).toBe(false); }); +it('keeps a bin-claimed src/scripts module in script discovery while lib still claims (#389)', async () => { + const root = await createRoot(); + await writeTree(root, { + 'src/cli/doctor.ts': moduleSource, + 'src/cli.ts': moduleSource, + 'src/index.ts': moduleSource, + 'src/scripts/hauler.ts': moduleSource, + 'src/scripts/internal/tool.ts': moduleSource, + 'src/scripts/my tool.ts': moduleSource, + 'src/scripts/shared.ts': moduleSource, + }); + const graph = await compileRouteGraph(root, fixtureConfig({ + // The #389 shape: one entry is the npm bin and the artifact hook target. + bin: { + doctor: './src/cli/doctor.ts', + hauler: './src/scripts/hauler.ts', + main: './src/cli.ts', + spaced: './src/scripts/my tool.ts', + tool: './src/scripts/internal/tool.ts', + }, + lib: { entry: './src/scripts/shared.ts' }, + })); + + expect(graph.diagnostics).toEqual([]); + // A bin claim never removes a safely named direct src/scripts/ child: + // the bin and the artifact script are disjoint outputs running the same + // main, so both surfaces ship. The nested and the unsafely named modules + // stay claimed — discovering them would only turn a valid bin-only + // configuration into AB4808 or AB4803 — and a lib entry still claims its + // module: a library is not a script. + expect(graph.scripts.map((route) => route.id)).toEqual(['script:hauler']); + // Every other route kind still belongs to the claiming declaration. + expect(graph.cli).toBeUndefined(); +}); + +it('gates a bin-claimed rendered script with AB4737 only when it exports no main (#389)', async () => { + const project = await createInspectProject({ + 'agent-bundle.config.ts': [ + 'export default {', + ' bin: {', + " notes: './src/scripts/render-notes.tsx',", + " object: './src/scripts/render-object.tsx',", + " poster: './src/scripts/render-poster.tsx',", + " reexport: './src/scripts/render-reexport.tsx',", + " tool: './src/scripts/render-tool.tsx',", + " typed: './src/scripts/render-typed.tsx',", + ' },', + " plugin: { name: 'routes-fixture', version: '1.0.0' },", + " targets: ['portable'],", + '};', + '', + ].join('\n'), + // A default re-exported from a private sibling cannot be judged + // statically; it is accepted (the worker still verifies it at run time). + 'src/scripts/_component.tsx': 'export default async () => undefined;\n', + 'src/scripts/render-reexport.tsx': [ + 'export const main = async (argv: readonly string[]): Promise => argv.length;', + "export { default } from './_component.tsx';", + '', + ].join('\n'), + // main plus a type-only default alias of an async function binding: no + // JavaScript default export is emitted, so the rendered script has no + // component even though a same-named function exists. + 'src/scripts/render-typed.tsx': [ + 'const Component = async () => undefined;', + 'export const main = async (argv: readonly string[]): Promise => argv.length;', + 'export { type Component as default };', + '', + ].join('\n'), + // Exports both: main(argv) for the bin envelope, the component for the + // rendered script, so the module serves both surfaces. + 'src/scripts/render-notes.tsx': [ + 'export const main = async (argv: readonly string[]): Promise => argv.length;', + 'export default async () => undefined;', + '', + ].join('\n'), + // A default export that is not a component: present, but the rendered + // script would fail at run time, so presence alone is not enough. + 'src/scripts/render-object.tsx': [ + 'export const main = async (argv: readonly string[]): Promise => argv.length;', + 'export default {};', + '', + ].join('\n'), + // Component plus a type-only main: the bin envelope ignores type exports + // and would still call the component as main(argv). + 'src/scripts/render-poster.tsx': [ + 'export type main = (argv: readonly string[]) => Promise;', + 'export default async () => undefined;', + '', + ].join('\n'), + // main only: the bin works, but the rendered script has no component to render. + 'src/scripts/render-tool.tsx': 'export const main = async (argv: readonly string[]): Promise => argv.length;\n', + }); + + const result = await validate({ root: project }); + const gate = result.diagnostics.filter(({ code }) => code === 'AB4737'); + expect(gate.map((diagnostic) => diagnostic.sourcePath)).toEqual([ + join(project, 'src/scripts/render-object.tsx'), + join(project, 'src/scripts/render-poster.tsx'), + join(project, 'src/scripts/render-tool.tsx'), + join(project, 'src/scripts/render-typed.tsx'), + ]); + expect(gate[0]!.message).toContain('render-object.tsx is also the entry of bin "object" but exports no async default Server Component'); + expect(gate[1]!.message).toContain('render-poster.tsx is also the entry of bin "poster" but exports no named main'); + expect(gate[2]!.message).toContain('render-tool.tsx is also the entry of bin "tool" but exports no async default Server Component'); + expect(gate[3]!.message).toContain('render-typed.tsx is also the entry of bin "typed" but exports no async default Server Component'); + expect(gate.every((diagnostic) => diagnostic.severity === 'error')).toBe(true); + // Every rendered script stays discovered beside its bin: the gate names + // the conflict instead of dropping a route. + const graph = await compileRouteGraph(project, fixtureConfig({ + bin: { + notes: './src/scripts/render-notes.tsx', + object: './src/scripts/render-object.tsx', + poster: './src/scripts/render-poster.tsx', + reexport: './src/scripts/render-reexport.tsx', + tool: './src/scripts/render-tool.tsx', + typed: './src/scripts/render-typed.tsx', + }, + })); + expect(graph.scripts.map((route) => route.id)).toEqual([ + 'script:render-notes', + 'script:render-object', + 'script:render-poster', + 'script:render-reexport', + 'script:render-tool', + 'script:render-typed', + ]); +}); + +it('gates a bin-claimed plain script with AB4738 only when its bin would run a default export the script ignores (#389)', async () => { + const project = await createInspectProject({ + 'agent-bundle.config.ts': [ + 'export default {', + ' bin: {', + " 'default-only': './src/scripts/default-only.ts',", + " hauler: './src/scripts/hauler.ts',", + " plain: './src/scripts/plain.ts',", + ' },', + " plugin: { name: 'routes-fixture', version: '1.0.0' },", + " targets: ['portable'],", + '};', + '', + ].join('\n'), + // The bin envelope would run this default export; the artifact script + // pipeline only wraps main, so scripts/default-only.mjs would be inert. + 'src/scripts/default-only.ts': 'export default async (argv: readonly string[]): Promise => argv.length;\n', + // main is wrapped by both envelopes; a self-executing module bundles + // byte for byte on both surfaces. + 'src/scripts/hauler.ts': 'export const main = async (argv: readonly string[]): Promise => argv.length;\n', + 'src/scripts/plain.ts': "process.stdout.write('plain\\n');\n", + }); + + const result = await validate({ root: project }); + const gate = result.diagnostics.filter(({ code }) => code === 'AB4738'); + expect(gate).toHaveLength(1); + expect(gate[0]).toMatchObject({ + message: expect.stringContaining('default-only.ts is also the entry of bin "default-only" and exports a default but no main'), + severity: 'error', + sourcePath: join(project, 'src/scripts/default-only.ts'), + }); + expect(result.diagnostics.filter(({ code }) => code === 'AB4737')).toEqual([]); +}); + it('errors with AB4800 when a declared entry, command, or url claims a routed server', async () => { const root = await createRoot(); await writeTree(root, { 'src/mcp/curator/tools/inspect.ts': moduleSource });