Repository navigation
✨ Install trusted Plugins before a document runs (#818) - #821
Merged
Merged
Conversation
XMD gains one generic, trusted Plugin boundary. A distribution supplies an ordered list of bundled Plugins and an operator appends package or filesystem Plugins with a repeatable `--plugin`. The complete list is loaded, admitted and installed before anything imports a root document. A Plugin is a plain structural value carrying a name and one optional `install(request)`. What it returns crosses at the boundary a trusted host already had — declared Markdown, structural syntax and its handler, journal admissions — and it composes a run through three stable, namespaced contextual APIs published from `@executablemd/core/api`: `Document`, `RootMetadata` and `ActivePlugins`. `@executablemd/runtime/api` publishes the existing `Config` Api a Plugin reads timeouts and verbosity through. Both root exports are unchanged. `Document`'s terminal answers with `<Document />`. Canonical core scans the composed envelope once and splices the already parsed, already target-selected root segments in at each placeholder, at their authored positions — so an unwrapped run is the run it always was, and an authored `<Document />` is an ordinary element that projects nothing. Execution, `inspectDocument` and document validation compose the root through one shared path. The #817 review graph becomes the first production Plugin: `@executablemd/code-review-agent` exports it as the package default, built from the existing `reviewComponentDeclarations()`/`useReviewComponents()` boundary with all thirty-five declarations, six reserved registrations, origins, digests and packaged assets unchanged. `packages/cli/src/bundled-plugins.ts` is now the only CLI module that names the package, and one installation per command serves ordinary execution, `xmd syntax`, structural validation and every `<Plan>` check. A selected Plugin is trusted executable code and is not sandboxed: loading one runs its module. Refusals — no usable default export, no name, a non-callable install, a duplicate Plugin name, a remote specifier — happen before any install and before the root is read.
Architecture review of b66a4af asked for eight corrections, and each of them sharpens a boundary the first commit left blunt. The three contextual APIs are now published under `executablemd.core.plugin.document`, `.root-metadata` and `.active-plugins` while keeping the public names `Document`, `RootMetadata` and `ActivePlugins`. The key is the whole of what two loaded copies agree on, so it carries the owning package rather than a bare word another package could pick — proved from both sides, including inside the compiled binary, where an impostor Api named `Document` composes nothing while a second copy at the published key composes. Module admission returns the admitted object instead of a copy: a Plugin keeps members this boundary does not read, and `install` keeps the receiver its own module gave it. Module loading goes back behind the host boundary. `host-plugin-modules.ts` owns the dynamic import and the package resolution, `runXmd` takes a `PluginModuleLoader`, and the Deno, Node, Bun and compiled entrypoints each supply it. Shared orchestration reaches no host API and detects no runtime. `@executablemd/core/api` is exactly the accepted surface — seven values plus the contract and contextual API types. `parsePluginValue` moves to `/host` beside `Markdown`, `sourceDigest` and `Structural`, which stay there. The review Plugin reads the workflow action as a positional rather than as the first recognized word, so `xmd workflow --plugin start list` is the management action it is. The frozen evidence is completed: one installation carrying every member, all three timeouts and verbosity during install with like-named props changing none of them, a bare package selected from the invocation directory under a Plugin name of its own, an installed package left inert, relative paths, and cancellation unwinding without moving the established teardown precedence. CLI help, the specification, architecture and source documentation now say what was accepted: selected Plugin code can execute whatever the surrounding runtime permits. And `bundled-plugins.ts` says what is true — bundled Plugins install first, so this build's middleware surrounds what a selection composes.
The review Plugin decides whether a `workflow` command line executes a document by finding the action, and the action is the first positional after `workflow` — the first token that is neither an option nor an option's value. The scan stepped over four options that take a separated value and missed two. `--output` and `--status` are the two, and `xmd workflow --output start export run-1` is what that costs: `--output` takes `start`, the action is `export`, and a scan that read the first recognized word installed the review graph for a command that executes no document. The list is now every non-boolean field of the command's own grammar — `--id`, `--at`, `--status`, `--artifact`, `--output` — plus the two options read out of argv before that grammar exists, `--plugin` and the aggregate and generated root properties. The boolean switches take no value and are skipped as the options they are. It is written out here because the CLI depends on this package rather than the other way round, and the comment names the grammar it mirrors. The regression walks every one of those options with an executing action's own name as its value and a management action after it, adds the assigned spelling, and keeps four executing classifications to show the same reading still finds a real action written after an option.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // No package-specific vocabulary is bootstrapped here. What a Plugin | ||
| // registers — the review graph's six reserved registrations among them — it | ||
| // registers in the command scope this one is entered inside, so a command | ||
| // that installed no Plugin describes exactly the engine's own language. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // that installed no Plugin describes exactly the engine's own language. |
| @@ -0,0 +1,2 @@ | |||
| // A default export whose `install` is not callable. | |||
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // A default export whose `install` is not callable. |
| // | ||
| // Both lines are markers a subprocess test reads. The first proves that | ||
| // selecting a module runs its top level — loading is running — and the second | ||
| // proves that its install ran, and what it was told about the command. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // proves that its install ran, and what it was told about the command. |
| @@ -0,0 +1,6 @@ | |||
| // Never selected by any row. Its presence beside the fixtures that are | |||
| // selected is what makes "an unselected module does nothing" observable: if | |||
| // anything discovered it, this line would be in the output. | |||
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // anything discovered it, this line would be in the output. |
| @@ -0,0 +1,12 @@ | |||
| // Composes the root's ordinary metadata and nothing else. | |||
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Composes the root's ordinary metadata and nothing else. |
| includes: ["components", "."], | ||
| // The distribution's own bundled Plugins, because a `host="run"` child | ||
| // gets what `xmd run` gets: the child installs them again in its own | ||
| // scope, with command `run`. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // scope, with command `run`. |
| // provider, spawns a process, reads a credential or reaches the network. | ||
| yield* useReviewComponents(); | ||
| // The thirty-five Markdown declarations travel by value, read from this | ||
| // build's own assets, exactly as they always have. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // build's own assets, exactly as they always have. |
| const parsed = yield* parseRootMarkdownDefinition("__root__", path, content, options.target); | ||
| const { definition } = parsed; | ||
| // The same composition a run performs, so what a caller is told a document | ||
| // declares is what the document will run as (spec §5.4). |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // declares is what the document will run as (spec §5.4). |
| ): Operation<Readonly<Record<string, unknown>>> { | ||
| return yield* scoped(function* () { | ||
| // The execution's own answer, installed nearest so a Plugin's ordinary | ||
| // middleware wraps it and reads the root's real metadata through `next()`. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware wraps it and reads the root's real metadata through `next()`. |
| yield* useRunProfileRegistry(); | ||
| // The bundled Plugins first, exactly as a `run` command installs them: a | ||
| // Plugin's registrations and the documentation describing them arrive | ||
| // together, so a build that shipped one without the other fails here. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // together, so a build that shipped one without the other fails here. |
PR #821: ✨ Install trusted Plugins before a document runs (#818)75 files, +4209 / -128 Scope🔴 PR has 4337 lines changed. Split into focused PRs. 🟡 4337 lines changed. PRs under 400 receive more thorough review. 🟡 75 files changed. Are all changes related? 🟡 Changes span 16 directories. 🟡 PR mixes config and source changes. 🟡 package.json changed without dependency justification. StructuralOxlint structural signals:
Slop
Oxlint slop signals:
Static AnalysisOxlint: 69 diagnostics across 10 files (15 rules) unbound-method (13): packages/cli/src/testing-host.ts, packages/cli/tests/support/run-markdown-tier.ts, packages/cli/src/cli.ts (+4) CorrectnessNo extraneous code patterns detected. |
taras
commented
Sep 15, 2026
| import reviewPlugin from "@executablemd/code-review-agent"; | ||
| import type { Plugin } from "@executablemd/core/api"; | ||
|
|
||
| export const BUNDLED_PLUGINS: readonly Plugin[] = Object.freeze([reviewPlugin]); |
Owner
Author
There was a problem hiding this comment.
why are we bundling this plugin?
The Product Owner amended #818: XMD has zero bundled or default Plugins. The code-review agent stays the first production Plugin and is activated only by an explicit `--plugin` selection. Three boundaries move with that. **Nothing is bundled.** `bundled-plugins.ts` is gone and the CLI installs exactly what the command line selected — no prefix, no default. The repeated `--plugin` occurrences, in written order, are the whole Plugin order. The package left the CLI's dependencies for its devDependencies: the suites select it the way an operator does, and the product neither imports it nor ships it. `EMBEDDED_PACKAGES` is empty, deliberately: the binary contains the program `xmd` is, and a Plugin is not part of it. `UNEMBEDDED_PACKAGES` names what the discovery sweep must not demand, and `packaged-document.test.ts` checks that claim rather than trusting it — a CLI module naming an excluded package, or the CLI manifest depending on one, fails there. This repository's own review paths now select explicitly: both workflows and the `review`, `review:local`, `analyze` and `analyze:ci` tasks. The graph itself is untouched — the same thirty-five declarations, six reserved registrations, origins, digests, documentation, packaged assets, command-aware activation, test root exclusion, nested `host="run"` installation and checkout anti-shadowing. **Both declaration arms reach every consumer.** The retained catalog carried only a Plugin's Markdown components, so `xmd syntax` and `<Plan>` validation could not see structural syntax a run expands — a Plan refused for writing syntax its own run supplies. `CommandPlugins.declarations` now carries both arms, from one installation, to execution, syntax, document validation, every Plan check and the nested run host. A structural-only Plugin is the regression, with the inverse row proving the same candidate is refused where nothing declared the construct. **`Document` is operation-valued.** `document(): Operation<string>`, because deciding what to wrap a document in is work: a Plugin may read configuration or ask where it is standing, and it delegates with `yield* next()`. The terminal still answers `<Document />`, the first selected Plugin is still outermost, `next()` may still be called more than once, and the private projection, the namespaced cross-copy keys, authored positions, root identity, target selection, props, returns and Output behavior are unchanged. The workflow valued-option correction is retained, `--status` and `--output` included.
The anti-shadowing case runs a document and then asks the same installation what it would let a document write. With no Plugin bundled, both halves have to name the Plugin: the run did and the description did not, so it described a vocabulary without the review graph and found the caller's own `Finding.md` where the declaration should have been. Selecting it for the description too is what makes the two halves one claim about one installation, which is what the case was always for.
Removing this suite's review-asset assertions took a neighbouring case with them: `publishes the /api subpath and loads a Plugin named on the command line`. The suite went on passing at three cases instead of four, which is what a green aggregate hides — the loss surfaced only by comparing the step names a run printed against the cases the file declares. It is restored as it stood at 82460bc, with one sentence added: reaching a Plugin through an explicit selection is now the only way this package reaches one at all, because it bundles none and depends on none. Nothing else changes. The whole diff is this case and its documentation.
The amended contract landed in the code and left claims behind in the prose. A reader was still told, in nine places, that the code-review graph is something XMD bundles, defaults to, or carries in its binary. Corrected: the loader no longer offers "or because this distribution bundles it"; the code-review package describes its default export as the Plugin a host installs once an operator has selected it; the Plan vocabulary is exactly what this invocation selected, with no review prefix; a nested `host="run"` child reinstalls the values the invocation selected and receives the review graph only when that Plugin was among them; the CI workflow section says the binary embeds none of the package and the workflow names it — keeping the no-include and anti-shadowing explanation, which is unaffected by how the Plugin is reached; and PL5 states that the repeated occurrences in written order are the complete order. Two definitions were also narrower than the code. `commandDeclarations()` carries every execution declaration — exact Markdown *and* structural syntax, under one discriminant — because a catalog holding only the Markdown half would describe a language a run does not have. The architecture glossary entry says the same. Comments and prose only: no line outside a comment changed in `packages` or `scripts`, and negative statements like "XMD bundles no Plugin" are correct and stay.
taras
marked this pull request as ready for review
September 15, 2026 03:57
Moving `@executablemd/code-review-agent` out of the CLI's dependencies left two generated records describing the old graph, and CI went red everywhere in under a minute. `pnpm-lock.yaml` still recorded the package under `packages/cli`'s `dependencies`. CI's `pnpm install` is frozen, so it failed before anything ran — which is why `lint`, `smoke`, `review`, every `test-node` shard and every `filesystem-contract` matrix entry failed in their install step with nothing else attempted. `.github/workflows/publish-packages.yml` is generated from the workspace manifests, and every `test-deno` shard regenerates it and diffs. The CLI no longer needs the code-review package, so that job leaves `cli`'s `needs:` and moves after it in the topological order. Both are regenerated, never hand-edited: `pnpm install --lockfile-only` and `deno task gen:publish-workflow`. No source changed.
…818) Two architecture blockers on PR #821. **A half-installation reached the commands that describe it.** A structural declaration and the handler that expands it are one installation, and canonical execution refuses either half alone before it reads a root document. But the Plugin host retained a *flattened* declaration list beside the installations, and `xmd syntax` and `<Plan>` validation consume that list through `admitExecutionDeclarations()`, which cannot see handlers and is not supposed to. So a Plugin that declared a construct and supplied no `expand` had its syntax described to a writer and accepted in a Plan, and was refused only when a run finally tried to expand it. The reverse half — a handler with nothing to expand — had the same shape. The rule now lives in one place. `incompleteStructural()` answers which half is missing; canonical `admit()` keeps its own wording for an execution, and the Plugin host holds every installation to the same answer where the assembly is built, naming the Plugin and what a command would otherwise have done with it. Generic declaration-only inspection is untouched: nothing new is required of `admitExecutionDeclarations()`. **A Windows path was refused as a remote URL.** `resolvePluginSpecifier()` tested the URI-scheme grammar before anything else, and `C:\plugins\review.mjs` matches it — `C:` reads as a scheme. Classification is now its own decision, made before resolution and in an order that reads a drive prefix first, so a path on an operator's own disk is a path. `file:` URLs, relative paths, POSIX absolute paths, UNC shares, bare packages and real remote schemes all keep the answers they had. Both regressions were checked by breaking the fix: removing the pair check fails five rows, including the two that prove `xmd syntax` advertised the construct and Plan accepted it; removing the drive-prefix rule fails three, reporting `C:\plugins\review.mjs: remote`. The Windows rows are classification rather than resolution, so they run on any host — no shard runs the CLI on Windows.
This was referenced Sep 15, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #818.
Why
XMD had one extension boundary, and it was not a boundary: the review component
graph reached the CLI by being imported at four separate call sites, and there
was no way for anyone else's code to compose a run at all. A distribution could
not state what it ships, and an operator could not add anything.
This adds one generic, trusted Plugin boundary, and moves the #817 review graph
onto it as its first production consumer — which is the only way to know the
boundary is the right shape.
What changes
Before:
packages/cli/src/cli.ts,syntax.ts,plan-component.tsandtesting-host.tseach imported@executablemd/code-review-agentand assembledthe graph themselves through
runProfileDeclarations()anduseRunProfileRegistry(). Nothing else could contribute to a run.After: an operator selects Plugins with a repeatable
--plugin <specifier>,and those occurrences in written order are the complete list — XMD ships none
and installs none for a command that named none. What was selected is loaded,
admitted and installed before anything imports a root document.
$ xmd run --plugin ./house-style.mjs --plugin @acme/reviews report.mdThe review graph is the first production Plugin and arrives the same way:
nothing in the CLI names the package, and this repository's own review paths
select it explicitly. One installation per command then serves ordinary
execution,
xmd syntax, structural validation and every<Plan>check.How it works
Documentisdocument(): Operation<string>, so composing an envelope may dowork and a wrapper delegates with
yield* next(). The three APIs are publishedunder namespaced keys (
executablemd.core.plugin.*) while keeping the publicnames, so a Plugin that resolved its own copy of core composes with the
canonical execution and an unrelated Api named
Documentcannot intercept it.A Plugin is a plain structural value carrying a name and one optional
install(request). What it returns crosses at the boundary a trusted hostalready had — declared Markdown, structural syntax and its handler, journal
admissions — converted to one
ExecutionInstallationat the CLI/core seam.It composes a run through three value-returning contextual APIs published from
@executablemd/core/api.Document's terminal answers with<Document />;canonical core scans the composed envelope once and splices the root's
already-parsed, already-target-selected segments in at each placeholder, at
their authored offsets, lines and paths. Splicing rather than nesting is what
keeps source positions, expansion identities, top-level
<Output>and a valueroot's
<Return>exactly as they were — and with no middleware the envelope isthe placeholder, so the definition that was imported is the definition that runs.
Review guide
Start with:
packages/core/src/plugin.tsandpackages/core/api.tsThen review:
packages/core/src/plugin-apis.ts— the three APIs and their namespaced keyspackages/core/src/root-composition.ts— the one composition path, shared byexecution,
inspectDocumentand document validationpackages/cli/src/plugin-{selection,loader,host}.tsandhost-plugin-modules.ts— grammar, admission, ordered installation, and thehost adapter the four entrypoints supply
packages/code-review-agent/src/plugin.ts— the first production PluginLook carefully at:
withPlugins()inpackages/cli/src/cli.ts: the command's whole lifetime isinside the scope the Plugins installed into, so a load or install failure
unwinds the earlier ones, reads no root and starts no document.
testing-host.ts: a nestedhost="run"child inherits nothing and installsthe same Plugin values again, with command
run.What must stay true
in
root-composition.tsand checked byroot-composition.test.tsRC1/RC3.namespaced keys (
executablemd.core.plugin.*) and checked byplugin-api.test.tsPA1b and, across real copies, byscripts/tests/plugin-compiled.test.ts.first selected Plugin outermost; two Plugins claiming one Plugin name,
component name or construct are refused at admission. Checked by
plugin-host.test.tsPH1/PH2 andplugin-cli.test.ts.--plugininstalls none, and the review graph is absent from the symbols and from a run
until it is selected. Checked by
plugin-host.test.tsPH6 andplugin-cli.test.tsPC9.execution,
xmd syntax, document validation and every Plan check from oneretained assembly. Checked by
plugin-host.test.tsPH7.invocation directory. Checked by
plugin-cli.test.tsPC4/PC7.registrations, same origins and digests. Checked by
scripts/tests/review-infrastructure.test.ts.root-import and declaration records are unchanged.
How to verify it
packages/core/tests/root-composition.test.tsproves an unwrapped run isunchanged and a wrapped one keeps the document's own contract — it fails if
composition renumbers segments, nests the body under an element, or lets an
authored
<Document />project anything.packages/cli/tests/plugin-host.test.tsproves install order, duplicaterefusal before any install, one installation carrying every member with its
bound
expand, cancellation unwinding, and the workflow-actionclassification — it fails if an option's value is read as the action.
packages/cli/tests/plugin-cli.test.tsdrives the real CLI: wrapper nesting,next()twice, declaration precedence over a checkout's own components,every refusal costing nothing, a bare package selected from the invocation
directory under a Plugin name of its own, and an installed-but-unnamed package
staying inert.
scripts/tests/plugin-compiled.test.tsasks the compiled binary — nocheckout, no
node_modules— to load an unembedded.mjs, and proves bothhalves of the cross-copy claim.
scripts/tests/cli-npm-bin.test.tsproves the emitted manifests publish./apiand the built bin loads an operator's module.Scope
Included
Plugincontract,Plugin(), and admission at the host boundary.Document,RootMetadata,ActivePluginsand the shared root composition.--plugingrammar, loading, ordered installation and lifetime.runProfileDeclarations()anduseRunProfileRegistry()removed, and nothing bundled in their place.@executablemd/core/apiand@executablemd/runtime/api.Intentionally unchanged
XMD bundles no Plugin and embeds none of the code-review package in the
binary;
EMBEDDED_PACKAGESis empty andpackaged-document.test.tschecksthat no CLI module or manifest reaches for an excluded package.
Every existing root export of
coreandruntime, and@executablemd/code-review-agent/review-components.Every durable encoding, failure precedence and replay rule.
The review graph's 35/6 counts, origins, digests and packaged assets.
scripts/lib/compile.ts's whole-package include — code selection and assetembedding stay separate decisions.
Ambient or document-declared Plugin discovery: V1 performs none. That is a
statement about V1, not a claim that it can never be added.
New abstractions
Plugin/PluginInstallationexist because an operator needs one way tocontribute to a run; consumers today are the code-review Plugin and the
fixture Plugins the suites select.
Document,RootMetadata,ActivePluginsexist because composing a documentacross two loaded copies of core needs stable named state rather than a
registry.
PluginModuleLoaderexists because reaching a module is a host act, and theshared CLI reaches no host API.
Risks and limitations
Loading one runs its module, and selected Plugin code can execute whatever the
surrounding runtime permits. The engine refuses a malformed Plugin before
anything installs; it constrains a well-formed one not at all. CLI help, the
specification and
architecture.mdall say so.<Output>decides what renders, so awrapper's own text around such a document renders nothing. That is the
document's contract winning, and
root-composition.test.tsRC4 states it.xmd workflow's valued-option list across a packageboundary; the spellings are exhaustively tested, and exporting the grammar was
considered and rejected as coupling.
Scope confirmation