💥 feat(runtime): resolve the xmd command through API.Env - #184
Merged
Conversation
The CLI reconstructed its own relaunch command by inspecting `basename(process.execPath)`. That guess had no Bun branch at all — under Bun it fell through to the compiled-binary case and produced `bun test-agent` instead of `bun <entry> test-agent` — and it could not be right in principle: a path names the executable but not how it was launched, and `deno run --allow-all <entry>` is not recoverable from "deno". `API.Env` gains `command(args?)`, which returns the complete invocation of this xmd with `args` appended. Its default handler fails explicitly rather than guessing. Each runtime-named entrypoint — `deno.ts`, `node.ts`, `bun.ts`, `compiled.ts` — installs its own adapter and takes the entry path from `import.meta.url`, so the path stays valid in the working directory the worker is spawned from. `cli.ts` is left runtime-neutral. `<TestAgent>` asks for the command when it provisions a provider, not when the components are installed, so a document that never mentions it runs even where no adapter exists. `installTestAgentComponents` therefore takes no argument and `TestAgentComponentsOptions` is gone. execute() no longer picks a compiler by testing `typeof globalThis.Deno`; installing one is the entrypoint's job. `API.Compiler`'s explicit "not installed" failure is unchanged, so a programmatic caller that runs eval blocks must now install compiler middleware itself. Documents without eval blocks are unaffected. The worker receives the installer rather than inheriting one, because the behavior-document profile has to wrap the compiler for its inline-only restriction to run. `useDenoCompiler` becomes `useDataUriCompiler` with no alias — nothing in it was ever Deno-specific — and its promise conversion moves to `until`. Refs #156, which keeps the wider production host-access migration and the lint rule that will enforce AGENTS.md rule 12.
PR #184: 💥 feat(runtime): resolve the xmd command through API.Env61 files, +974 / -355 Scope🔴 PR has 1329 lines changed. Split into focused PRs. 🟡 1329 lines changed. PRs under 400 receive more thorough review. 🟡 61 files changed. Are all changes related? 🟡 PR mixes config and source changes. 🟡 package.json changed without dependency justification. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static Analysis✅ Oxlint found no issues. CorrectnessNo extraneous code patterns detected. |
The spec listed the new entrypoint files without saying what they promise. Both contracts are observable and neither was written down. §4.2 now states compiler ownership: execute() neither detects the runtime nor installs a compiler; a document without eval blocks needs none; one with them requires the caller to install API.Compiler middleware, and gets an explicit failure rather than a guess when it has not; a caller's compiler is used rather than shadowed. The section no longer presents the data: URI as the only mechanism, since two implementations ship and neither is bound to the runtime its name once suggested. §9.6 now states the command contract: the complete invocation with arguments appended in the position that host needs, no inferred default, an absolute entry path from import.meta.url, and resolution deferred until <TestAgent> provisions a provider. Corrects the claim that cli.ts "detects nothing about the host". It makes no decision about re-invocation, compiler choice, or runtime detection — but it still reaches process.stdout and node:fs/promises directly, which is #156. Tier CB covers the compiler boundary: no eval blocks and no compiler, an eval block with no compiler, and a caller-installed compiler surviving execute().
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }, | ||
| }); | ||
| // Beneath the profile, so its inline-only check runs first and the | ||
| // passthrough for an allowed block reaches a real compiler. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // passthrough for an allowed block reaches a real compiler. |
`command` and `compile` are the same kind of thing: capabilities only the
entrypoint that knows the host can supply. Keeping them on separate Apis added
a boundary that bought nothing and cost an ordering workaround.
Both now live on API.Env, and each entrypoint installs them together with
`{ at: "min" }` — foundational providers that ordinary middleware wraps rather
than races. That deletes the workaround directly: the behavior-document policy
in the test-agent worker is plain "max" middleware again, validating a block
and delegating to whatever the entrypoint installed, with no need to be
installed in a particular order relative to it.
Removed with it: API.Compiler, XmdOptions.useCompiler, the compiler installer
threaded through runXmd() and runTestAgentWorker(), and the unused `next`
parameters that terminal providers had to declare. A terminal provider now
omits `next` entirely; only middleware that delegates declares it.
The compilers split into the operation and its installer: `compileDataUri` and
`compileTempFile` are what an entrypoint calls inside its `compile` provider,
while `useDataUriCompiler`/`useTempFileCompiler` remain for callers that want
only a compiler, and register at min.
Command construction is inline in each entrypoint's own middleware rather than
extracted to a shared module — that module made the entrypoints forwarding
shells and put the host-adapter boundary in the wrong place. Its unit tests go
with it; argument placement is covered where it is observable, by a worker
that has to actually launch.
| } | ||
| return mod.default; | ||
| } finally { | ||
| // Clean up temp file — don't await, fire and forget |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Clean up temp file — don't await, fire and forget |
| isolatedImports[`${name}/`] = `npm:${name}@^${siblingVer}/`; | ||
| } | ||
| // Preserve the manifest fields alongside the rewritten imports: packages/cli/src/cli.ts | ||
| // Preserve the manifest fields alongside the rewritten imports: the CLI |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // Preserve the manifest fields alongside the rewritten imports: the CLI |
The defect that motivated this branch is a wrong Bun worker command, and nothing verified the fix. `bun run test:bun` never loads packages/cli/src/bun.ts and never relaunches a worker, so its green result said nothing about the Bun adapter. The smoke document drives a <TestAgent> scenario, so the Bun parent has to relaunch bun.ts as `test-agent` through the command it builds. Reintroducing the original defect — dropping the entry module, as the old resolveWorkerCommand did under Bun — makes this step exit 1, so it discriminates. Also drops two comments that restate their code, and corrects the compilers' "Compiler API interface" wording now that compile lives on API.Env.
The header still said `execute` installs this automatically on Node and Bun, which the new contract makes false: `execute` installs no compiler at all. The Node entrypoint calls `compileTempFile` from its own `API.Env.compile` provider, and a programmatic caller that wants only a compiler installs `useTempFileCompiler()`. The "for Node and Bun" title went with it — this is the portable implementation, and which host uses it is the entrypoint's choice, not a property of the file.
EvalBlock named the generator that compilers happen to emit rather than what callers may do with it. That leaked an implementation detail into the contract, and eval-handler.ts paid for it twice with `as unknown as Operation<unknown>` casts to run a block at all. A compiled block now returns Operation<unknown>. Generated modules are still `function*` — a generator object already satisfies Operation — but that is how a block is built, not what the API promises. compileBlock() returns Operation<EvalBlock> instead of restating the concrete signature, and both casts are gone: a block runs with plain `yield*`. The type change immediately found every place that assumed otherwise. The compilation suites drove blocks by hand with .next() loops, which asserted mechanism rather than behavior; they now run the operation and assert what the block did. T17 no longer inspects an intermediate yield to prove `sleep` is in scope — it asserts the statement after the suspension ran, which is the observable claim. T23 covers a block's return value reaching the caller, which nothing asserted directly before. Enforcement is #185; this PR only avoids introducing what that rule will catch.
Per AGENTS.md rule 4, comments earn their place by describing surprising behavior. These described the line beneath them: an assertion restated in prose, four doc comments that repeated the names they sat on, and a `void next` for a parameter that only existed to be discarded — terminal middleware can omit `next` entirely. The --inspect rationale in node.ts was stated twice; it stays in the module documentation, where a reader meets it before the code. The constraints stay: why entrypoint providers install at min, why entry paths come from import.meta.url, why execute() installs no compiler, why command resolution is lazy, why XL3 does not assert exactly one request, why the static import anchors must survive `deno compile`, and why the Bun smoke exists separately from test:bun.
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
| // No compiler is installed here. Which one suits the host is the | ||
| // entrypoint's decision, and installing one from inside this task would | ||
| // shadow whatever the caller installed outside it. A document with no | ||
| // eval blocks never reaches API.Env.compile at all. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // eval blocks never reaches API.Env.compile at all. |
eval-handler.ts carried a comment saying the block compiles "via data: URI module import", which API.Env.compile makes false — the Node entrypoint uses the temp-file compiler — and a second that only narrated the yield* beneath it. The EvalBlock documentation claimed a generator object "already satisfies Operation". That is not true of an arbitrary Generator<unknown, unknown, unknown> under Effection's typed contract, and the point does not need it: a compiled block accepts the binding environment and returns an Operation, and callers do not depend on how a compiler builds one. Spec §4.2 now carries the public signature, so the contract is readable without opening apis.ts. The compilation tests lose their T-label comments and two that restated their assertions. The one explaining why T17's assertion proves the suspension resumed stays — that is the part a reader cannot see.
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
|
|
||
| await main(function* (args) { | ||
| // The base providers for this host. `at: "min"` puts them beneath ordinary | ||
| // middleware, so a policy installed later can wrap either one. |
There was a problem hiding this comment.
Redundant comment — restates what the code does.
Suggested change
| // middleware, so a policy installed later can wrap either one. |
4 tasks
4 tasks
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.
Why
Preparation for #144, and a slice of #156.
The CLI rebuilt its own relaunch command by inspecting
basename(process.execPath)(cli.ts,resolveWorkerCommand). That had areal bug — no Bun branch, so under Bun it fell through to the compiled-binary
case and produced
bun test-agentinstead ofbun <entry> test-agent— butthe shape was wrong regardless: an executable path says what is running, not
how it was launched.
deno run --allow-all <entry>cannot be recovered fromthe string "deno", and a compiled binary has no entry script at all. Only the
module that started the process knows.
execute()had the same problem in miniature, choosing a compiler withtypeof globalThis.Deno.What changes
Before: shared code guessed the host and reconstructed its own invocation.
After: the invocation is a contextual capability that runtime-named
entrypoints supply, and shared code asks for it.
How it works
API.Envowns both host capabilitiesIt returns the complete invocation with
argsappended, not a prefix forcallers to extend — argument placement differs per runtime and belongs to the
adapter. The default handler fails explicitly (
"xmd command not installed")rather than guessing from
process.execPath.command(how to re-invoke this xmd) andcompile(how this host loads agenerated module) are the same kind of thing — only the entrypoint knows
either. Each entrypoint installs both in one call with
{ at: "min" }, makingthem base providers that ordinary middleware wraps:
command(args)compiledeno.ts[execPath, "run", "--allow-all", entry, ...args]compileDataUrinode.ts[execPath, ...execArgv-minus-inspect, entry, ...args]compileTempFilebun.ts[execPath, entry, ...args]compileDataUricompiled.ts[execPath, ...args]compileDataUriArgument construction is inline in each entrypoint's own middleware — there is
no shared builder for them to forward to.
entrycomes from each module's ownimport.meta.url, neverprocess.argv[1]. The worker is spawned with the document's workingdirectory, so a path relative to the parent's would not resolve.
Lazy at the
<TestAgent>boundarycommandis requested when a provider is provisioned, not when components areinstalled — so a document that never mentions
<TestAgent>runs even where noadapter exists.
installTestAgentComponentstakes no argument now andTestAgentComponentsOptionsis gone.Compiler installation
execute()installs nothing; the entrypoint decides which compiler suits thehost.
API.Compileris gone —compilemoved ontoAPI.Envbesidecommand— and the explicit "not installed" failure moved with it.{ at: "min" }is what makes this work. An earlier revision installed thecompiler as ordinary middleware and hit a real ordering bug: the
behavior-document policy in
installWorkerProfilewrapscompileto rejectstatic imports, but a terminal provider installed earlier answered
compileitself and the restriction never ran — TV4 went green when it should have
stayed red. As a base provider the entrypoint's compiler is wrapped rather
than bypassed, and the policy is plain middleware that validates and
delegates. No installer threading, no ordering rule to remember.
The compiled-block contract
EvalBlocknamed the generator that compilers happen to emit rather than whatcallers may do with it, and
eval-handler.tspaid for that twice withas unknown as Operation<unknown>casts just to run a block. It is now:compileBlock()returnsOperation<EvalBlock>instead of restating theconcrete signature, both casts are gone, and a block runs with plain
yield*.Generated modules are still
function*— that is how a compiler builds one,not something callers depend on. Spec §4.2 carries the signature.
The type change found every consumer that assumed otherwise: the compilation
suites drove blocks by hand with
.next()loops, asserting mechanism ratherthan behaviour. They now run the operation and assert what the block did.
Rename
useDenoCompiler→useDataUriCompiler, no alias. Nothing in it was everDeno-specific — it builds a
data:URI, which Bun also loads and Node's tsxloader rejects. Its
call(() => import(...))moves tountilper rule 3.What must stay true
cli.tsandexecute.tsmake no decision about commandreconstruction, compiler selection, or runtime detection —
process.execPathappears only in the four entrypoints. They do still reach the host for
terminal and journal I/O; that is Route production host access through Env #156.
— checked by TV4 (
static import rejected).directory and requires the worker to launch.
Bun entrypoint smokestep in thetest-bunjob.How to verify it
Four repository checks with the CI-pinned Deno 2.9.1: lint 0 errors, typecheck
clean, 144 passed / 0 failed, JSR dry run
Success. Plus:deno task build→./dist/xmd test smoke-test/test-agent/README.mdexits 0with both assertions passing — the compiled entrypoint relaunching itself.
test-bunCI job:bun run packages/cli/src/bun.ts test smoke-test/test-agent/README.md --raw.bun run test:bunnever loadsbun.ts, so it could not have validated thevery defect this branch fixes. Verified discriminating: reintroducing the old
behaviour — dropping the entry module, as
resolveWorkerCommanddid underBun — makes the step exit 1.
scripts/tests/cli-npm-bin.test.tsbuilds the npm package from the newbin: ./src/node.tsand runs the emitted bin under Node, which relaunches itstest-agent worker. This is the regression gate for the entrypoint rewiring.
deno task gen:publish-workflowreports no drift.New coverage:
packages/cli/tests/command.test.ts(XC1–XC4 — the explicitno-adapter failure, argument appending, a max wrapper over the base provider,
and a worker relaunched from another working directory),
packages/test-agent/tests/command-lazy.test.ts(XL1–XL3), andpackages/core/tests/compiler-boundary.test.ts(CB1–CB3 — no eval blocks andno compiler; an eval block with no compiler; a caller-installed compiler
surviving
execute()).Both contracts are written down: spec §4.2 states compiler ownership and §9.6
states the command contract, rather than only listing the new files.
Scope
Included
API.Env.command, the four entrypoints, and the compiler-installationcontract.
scripts/build-npm.ts: an executable's entry now comes frompackage.jsonbinrather thandeno.jsonexports, so npm shipsnode.tswhile JSRgets the Deno entrypoint.
useTempFileCompiler()now thatexecute()does not.Intentionally unchanged
API.Envsurface, permission prompts, thetest-agent ACP server, and the Oxlint rule enforcing AGENTS.md rule 12.
temp-file-compiler.tsimplementation debt, tracked as Bring temp-file compiler in line with the Effection and filesystem rules #182.through a per-runtime launcher is the next PR.
Risks and limitations
install compiler middleware.
useDenoCompileris gone with no alias, andTestAgentComponentsOptionsis removed. Both specs are updated.absolute entry path it would also pass under the old
process.argv[1]implementation. The absolute-path contract is documented in spec §9.6 and in
each entrypoint rather than pinned by a discriminating test.
cli.tsstill reachesprocess.stdoutandnode:fs/promisesdirectly. Itmakes no decision about re-invocation, compiler choice, or runtime detection —
the claim is scoped to those three. The remaining host-boundary cleanup is
Route production host access through Env #156.
Scope confirmation