diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9c5ee0350..88979aab69 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,3 +89,12 @@ jobs: # per-run session log named main-session-.jsonl. Assert one exists. ls .sessions/_no-cwd/main-session-*.jsonl >/dev/null rm -rf .sessions + + # The published `bin` is `lib/bin.js`, run under plain `node` by a real + # consumer — NOT the tsx dev path the demo smoke and demo:* scripts use. + # These keyless smokes boot the BUILT bins (this step runs AFTER the build) + # in a temp dir that mirrors a real install, catching a regression in the + # published artifact that tsx would mask. They self-skip if lib/ is absent, + # so the e2e job (which does not build) does not run them. + - name: Built-bin smoke test (published lib/bin.js under node) + run: pnpm exec vitest run --config vitest.e2e.config.ts packages/ui/stdio-agent/tests/built-bin.e2e.ts packages/ui/acp-agent/tests/built-bin.e2e.ts diff --git a/docs/architecture.md b/docs/architecture.md index 891e851337..b5beec1143 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -39,7 +39,7 @@ For a catalog of the **data structures** this architecture moves around — the └─────────────────────────────────────────────────────────────┘ ``` -Dependency rule: plugins depend on interface packages, never on `dsh-agent-loop`. The loop itself is swappable — UI/hook/tool plugins keep working against the `dsh-agent` vocabulary if the loop is replaced. +Dependency rule: **extension** plugins depend on interface packages, never on `dsh-agent-loop`. The loop itself is swappable — UI/hook/tool plugins keep working against the `dsh-agent` vocabulary if the loop is replaced. The one sanctioned exception is a **composition/bundle** package whose job IS to assemble the concrete spine: `dsh-agent-core` bundles `dsh-agent-loop` (and the other concrete spine plugins) by design, so it depends on the concrete loop on purpose. The rule constrains plugins that EXTEND the system, not the bundle that COMPOSES it — swapping the loop means publishing a different bundle, not rewiring every extension. ## Service map diff --git a/docs/rfc/implemented/architecture/2026-06-20-extract-example-app-packages.md b/docs/rfc/implemented/architecture/2026-06-20-extract-example-app-packages.md index 6ab2c6a15f..b6c30819dd 100644 --- a/docs/rfc/implemented/architecture/2026-06-20-extract-example-app-packages.md +++ b/docs/rfc/implemented/architecture/2026-06-20-extract-example-app-packages.md @@ -12,7 +12,7 @@ The deeper problem was a **coupled front-door cluster** that lived at the leaf w Each example is now **mostly an invocation of an app package**, splitting the wiring along the existing [interface / implementation / consumer seam](2026-06-13-capability-seams.md): the **app package owns the composition**, the leaf `cordis.yml` owns only the **swappable choices** (which LLM adapter, which bash executor, model, prompt, persistence root). -- **`@deepseek-ai/dsh-agent-core`** ([packages/core/agent-core](../../../../packages/core/agent-core)) — a Cordis bundle plugin for the providerless, executor-less, UI-less spine: `timer` + `llm` + sessions + system-prompt + tools + agents + invariants + `tool-bash` + `agent-loop`, mounted as child plugins inside its `apply(ctx)` via `ctx.plugin(...)`. This is the old `base-core.yml` **minus** `bash-local`, **plus** `timer` and the loop, as code instead of a YAML include. The bundle **forwards** `agent-loop`'s `agents` list as its own config (`export const Config = AgentLoop.Config`, default `[]`, the existing `AgentLoop.Config` shape in [packages/core/agent-loop/src/index.ts](../../../../packages/core/agent-loop/src/index.ts)) — so each app supplies its own pre-created agents. This is precisely the reason the old `base-core.yml` gave for keeping `agent-loop` *out* of the shared core ("the examples disagree — stdio needs a pre-created `main`, acp needs none"); forwarding the config dissolves that objection — the loop is shared, the agents list is per-app. The bundle children register into the root service store, so a leaf-mounted sibling (the adapter, the executor) sees them exactly as a nested `plugin-include` subtree's services were seen before. +- **`@deepseek-ai/dsh-agent-core`** ([packages/core/agent-core](../../../../packages/core/agent-core)) — a Cordis bundle plugin for the providerless, executor-less, UI-less spine: `timer` + `llm` + sessions + system-prompt + tools + agents + invariants + `tool-bash` + `agent-loop`, mounted as child plugins inside its `apply(ctx)` via `ctx.plugin(...)`. This is the old `base-core.yml` **minus** `bash-local`, **plus** `timer` and the loop, as code instead of a YAML include. The bundle **forwards** `agent-loop`'s `agents` list as its own config (`export const Config = AgentLoop.Config`, default `[]`, the existing `AgentLoop.Config` shape in [packages/core/agent-loop/src/index.ts](../../../../packages/core/agent-loop/src/index.ts)) — so each app supplies its own pre-created agents. This is precisely the reason the old `base-core.yml` gave for keeping `agent-loop` *out* of the shared core ("the examples disagree — stdio needs a pre-created `main`, acp needs none"); forwarding the config dissolves that objection — the loop is shared, the agents list is per-app. The bundle children register into the root service store, so a leaf-mounted sibling (the adapter, the executor) sees them exactly as a nested `plugin-include` subtree's services were seen before. Depending on the CONCRETE `dsh-agent-loop` (not just the `dsh-agent` interface) is deliberate and is the sanctioned exception to the "extension plugins depend on interfaces, never on the concrete loop" rule (packages/README.md, docs/architecture.md § Layering): the rule constrains plugins that EXTEND the system, whereas this bundle's whole job is to COMPOSE the concrete spine. Swapping the loop means publishing a different bundle, not rewiring every extension. - **`@deepseek-ai/dsh-stdio-agent`** ([packages/ui/stdio-agent](../../../../packages/ui/stdio-agent)) and **`@deepseek-ai/dsh-acp-agent`** ([packages/ui/acp-agent](../../../../packages/ui/acp-agent)) — app packages, each consuming `dsh-agent-core` and **baking in its coupled front-door cluster**: stdio = `ui-stdio` + console logger + a pre-created `main`; acp = the `acp` bridge + JSONL persistence + **no stdout logger** + no pre-created agents. The leaf no longer carries the cluster, so it has no logger entry to copy wrong by default — the common stdout-purity mistake loses its foothold. (A leaf can still *add* a sibling logger entry — a package cannot forbid what a leaf author writes — so the rule "never add a stdout logger to an ACP leaf" stays documented at the leaf; what changed is that the default leaf has nothing to get wrong.) They land under the existing `ui` group alongside `acp`, so no new package group (and no `tsconfig`/`packages/README` group plumbing) was needed. - **`start.ts` is gone.** Each app package exposes a `bin` (`dsh-stdio-agent` / `dsh-acp-agent`); the `demo:*` scripts invoke it (e.g. `dsh-stdio-agent ./cordis.yml`). The Loader-boot tail, `.env` loading, snapshot-mode selection, and stdin-dispose lifecycle moved into that bin, owned by the app. The `bin.ts` files are coverage-excluded (a self-executing CLI entry, like the old `start.ts`) and driven by the keyless Loader-path tests. - **Each leaf `cordis.yml` collapses** to backends + config: the LLM adapter (`llm-deepseek` with apiKey/models, or `llm-replay`), the bash executor (`bash-local`), `hmr` for the stdio demos (see the amendment below), and one app entry carrying the app's config (model, system prompt, persistence root — surfaced as the app package's own `Config`, which routes each value to wherever the app wires it: stdio onto its pre-created agent, acp onto the bridge plugin). diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index 039dbace8c..be6aabada0 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -134,19 +134,21 @@ describe('snapshot fixtures', () => { }) it('every registered scenario has its required fixture files', async () => { - // Required files are per-KIND. Every scenario has an input script and an - // stdout golden. Only model scenarios persist a session log, so only they - // require `session.jsonl` (the replay source AND expected-log artifact); - // a no-model scenario boots `llm-replay` with an empty script and needs no - // session fixture. Authored scenarios additionally ship the - // `replay.override.json` sidecar that drives their model behavior. + // Every scenario has an input script and an stdout golden. EVERY scenario + // also needs `session.jsonl`: the harness boots `llm-replay` with that path + // as the replay source for ALL scenarios (acp.snapshot.ts passes + // `fixtureFile: /session.jsonl` unconditionally), and `loadReplayScript` + // throws "fixture not found" when it is absent and no override replaces it. + // A no-model scenario ships a header-only `session.jsonl` (it derives to an + // empty script — no model call is made); a model scenario's fixture also + // doubles as the expected-log artifact the run is diffed against. An authored + // (non-`recorded`) model scenario additionally ships a `replay.override.json` + // sidecar for the throw/hang cases a derived script cannot express. for (const { name, hasModelTurn, recorded } of SCENARIOS) { const dir = join(SNAPSHOTS_DIR, name) expect(existsSync(join(dir, 'input.json')), `${name}/input.json`).toBe(true) expect(existsSync(join(dir, 'stdout.golden.jsonl')), `${name}/stdout.golden.jsonl`).toBe(true) - if (hasModelTurn) { - expect(existsSync(join(dir, 'session.jsonl')), `${name}/session.jsonl`).toBe(true) - } + expect(existsSync(join(dir, 'session.jsonl')), `${name}/session.jsonl`).toBe(true) if (hasModelTurn && !recorded) { expect(existsSync(join(dir, 'replay.override.json')), `${name}/replay.override.json`).toBe(true) } diff --git a/knip.json b/knip.json index e73d165138..b3ce7c1d4b 100644 --- a/knip.json +++ b/knip.json @@ -32,6 +32,10 @@ "packages/ui/acp-agent": { "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts"], "project": ["src/**/*.ts", "tests/**/*.ts"] + }, + "packages/ui/stdio-agent": { + "entry": ["tests/**/*.spec.ts", "tests/**/*.e2e.ts"], + "project": ["src/**/*.ts", "tests/**/*.ts"] } } } diff --git a/packages/README.md b/packages/README.md index d5e1f55fe6..037bb9c66c 100644 --- a/packages/README.md +++ b/packages/README.md @@ -40,7 +40,7 @@ dsh-stdio-agent ← dsh-agent-core, dsh-ui-stdio, dsh-session-persistence-json dsh-acp-agent ← dsh-agent-core, dsh-acp, dsh-session-persistence-jsonl (ACP server APP + bin) ``` -The rule: plugins depend on interfaces, never on the concrete loop. `dsh-agent-loop` is swappable — UI/hook/tool plugins keep working against the `dsh-agent` vocabulary if the loop is replaced. A swappable capability splits into interface / implementation / consumer packages (the bash trio is the template — see [capability seams](../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md)). +The rule: **extension** plugins depend on interfaces, never on the concrete loop. `dsh-agent-loop` is swappable — UI/hook/tool plugins keep working against the `dsh-agent` vocabulary if the loop is replaced. The sanctioned exception is a **composition/bundle** package like `dsh-agent-core`, whose whole job is to assemble the concrete spine: it depends on `dsh-agent-loop` (and the other concrete spine plugins) on purpose. The rule constrains plugins that EXTEND the system, not the bundle that COMPOSES it — swapping the loop means shipping a different bundle, not rewiring every extension. A swappable capability splits into interface / implementation / consumer packages (the bash trio is the template — see [capability seams](../docs/rfc/implemented/architecture/2026-06-13-capability-seams.md)). ## What goes where diff --git a/packages/ui/acp-agent/README.md b/packages/ui/acp-agent/README.md index d48ecaa2b3..3403ef2126 100644 --- a/packages/ui/acp-agent/README.md +++ b/packages/ui/acp-agent/README.md @@ -36,4 +36,6 @@ The leaf supplies the swappable backends: an LLM adapter (`llm-deepseek` for the - honors `DSH_SNAPSHOT=replay` by booting the sibling `cordis.snapshot.yml` (the keyless replay tree, `llm-replay` in place of `llm-deepseek`); - in a snapshot run, disposes the context on stdin EOF so the session log is fully flushed before exit. +Run it under `node --expose-internals`: the cordis Loader resolves the config's bare plugin specifiers through its internal module loader, active only under that flag. (`demo:acp` runs under tsx, whose tsconfig `paths` map resolves them instead.) + All diagnostics go to **stderr** — stdout is the protocol. diff --git a/packages/ui/acp-agent/src/bin.ts b/packages/ui/acp-agent/src/bin.ts index 00a8bfdec0..1352cde0ed 100644 --- a/packages/ui/acp-agent/src/bin.ts +++ b/packages/ui/acp-agent/src/bin.ts @@ -59,10 +59,70 @@ function loadEnv(): void { } /** - * Boot the Loader against `absoluteConfigPath`. `baseUrl` is pinned to the - * config's directory and the include gets only the basename, so the config's - * relative plugin/include paths resolve as the upstream `cordis` bin does. - * Returns the root context. + * Make a load failure fail loud with a clear message on stderr. Covers the + * failure path the entry-tree check below cannot: when the include's + * `[Service.init]` throws (e.g. a config FILE missing in a real directory), the + * cordis Loader surfaces it as an unhandled promise rejection AFTER `boot()` + * resolves — `loader.await()` does NOT rethrow it (`EntryTree.await()` uses + * `Promise.allSettled`, which swallows rejections). Node's default handler + * already exits non-zero on an unhandled rejection, so this does not change the + * exit code; it replaces the noisy stack dump with a single labelled line (on + * STDERR — stdout is the ACP JSON-RPC channel) and guarantees `process.exit(1)`. + * Install before `boot()`. + */ +export function installFailLoud(): void { + process.on('unhandledRejection', (err: unknown) => { + process.stderr.write(`dsh-acp-agent: fatal load failure: ${err instanceof Error ? err.stack ?? err.message : String(err)}\n`) + process.exit(1) + }) +} + +/** + * After the tree settles, assert every loader entry actually started. This is + * the load-bearing guard against the SILENT-exit-0 bug: a plugin module that + * fails to IMPORT (e.g. a config path in a non-existent directory) is caught and + * only LOGGED by the cordis Loader (`entry._init`), leaving the entry with no + * `fiber` and producing no rejection — so the process would otherwise exit 0. A + * started entry has a `fiber`; throw on any entry still missing one so `boot()` + * rejects. + */ +function assertEntriesLoaded(ctx: Context): void { + const failed = [...ctx.loader.entries()].filter(entry => entry.fiber === undefined) + if (failed.length > 0) { + const names = failed.map(entry => entry.options.name).join(', ') + throw new Error(`dsh-acp-agent: plugin(s) failed to load: ${names} (see the error(s) logged above)`) + } +} + +/** + * Boot the Loader against `absoluteConfigPath`. The include is handed the + * config's ABSOLUTE `file://` URL as its `path`, so resolution never depends on + * `ctx.baseUrl` (an absolute URL ignores the base) and can never fall back to + * the cwd. `baseUrl` is still pinned to the config's directory so the config's + * OWN relative plugin/include paths resolve against it. Returns the root context + * once the whole tree has settled. + * + * The `await ctx.loader.await()` is load-bearing: `loader.create()` returns once + * the include ENTRY is registered, but the include then loads its child plugins + * asynchronously. Without awaiting the tree, `boot()` would resolve while the ACP + * bridge is still mounting — the process would have no stdin handle attached yet + * and could exit 0 silently. Awaiting keeps the process alive until the bridge + * is up. + * + * `loader.await()` does NOT rethrow load errors (`EntryTree.await()` uses + * `Promise.allSettled`), so failures are surfaced two ways: a plugin that fails + * to IMPORT leaves an entry with no fiber, caught here by + * {@link assertEntriesLoaded} (this `boot()` rejects); a plugin whose init THROWS + * surfaces as an unhandled rejection caught by {@link installFailLoud} (installed + * by `main()` before this runs). Together any load failure exits non-zero. + * + * Bare plugin specifiers in the config (`@deepseek-ai/dsh-*`, npm packages) are + * resolved by the cordis Loader's internal module loader, which is only active + * under `node --expose-internals`. The `demo:acp` script runs under tsx (whose + * tsconfig `paths` map resolves the workspace plugins instead), but a consumer + * running the built bin under plain node must pass `--expose-internals` so the + * Loader resolves the config's plugins from the config directory rather than + * relative to its own module. */ export async function boot(absoluteConfigPath: string): Promise { const ctx = new Context() @@ -70,19 +130,23 @@ export async function boot(absoluteConfigPath: string): Promise { await ctx.plugin(Loader) await ctx.loader.create({ name: '@cordisjs/plugin-include', - config: { path: `./${basename(absoluteConfigPath)}` }, + config: { path: pathToFileURL(absoluteConfigPath).href }, }) + await ctx.loader.await() + assertEntriesLoaded(ctx) return ctx } /** - * Entry point. Selects the config (snapshot-aware), loads `.env` outside replay, - * boots, and — in a snapshot run — disposes the context on stdin EOF so the - * session log is fully flushed before exit and the harness's `waitForExit` - * resolves. In a normal editor session stdin stays open for the connection's - * lifetime (the editor kills the process), so the EOF handler never fires. + * Entry point. Installs the fail-loud guard, selects the config (snapshot-aware), + * loads `.env` outside replay, boots, and — in a snapshot run — disposes the + * context on stdin EOF so the session log is fully flushed before exit and the + * harness's `waitForExit` resolves. In a normal editor session stdin stays open + * for the connection's lifetime (the editor kills the process), so the EOF + * handler never fires. */ export async function main(argv: string[] = process.argv.slice(2)): Promise { + installFailLoud() const snapshotMode = process.env.DSH_SNAPSHOT const configPath = resolveConfigPath(argv[0] ?? './cordis.yml', snapshotMode) if (snapshotMode !== 'replay') loadEnv() diff --git a/packages/ui/acp-agent/tests/built-bin.e2e.ts b/packages/ui/acp-agent/tests/built-bin.e2e.ts new file mode 100644 index 0000000000..3793232bda --- /dev/null +++ b/packages/ui/acp-agent/tests/built-bin.e2e.ts @@ -0,0 +1,187 @@ +import { spawn } from 'node:child_process' +import { mkdtemp, mkdir, rm, symlink, writeFile, readFile } from 'node:fs/promises' +import { existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { + ClientSideConnection, + ndJsonStream, + PROTOCOL_VERSION, + type Agent as AcpAgent, + type Client, + type RequestPermissionRequest, + type RequestPermissionResponse, + type SessionNotification, +} from '@agentclientprotocol/sdk' +import { Readable, Writable } from 'node:stream' +import { afterEach, describe, expect, it } from 'vitest' + +/** + * BUILT-ARTIFACT smoke for the published `dsh-acp-agent` bin. `load-path.e2e.ts` + * boots `src/bin.ts` under tsx — but the package's `bin` field points at + * `lib/bin.js`, run under plain `node` by a real consumer. This runs the REAL + * `lib/bin.js` under `node` (NOT tsx) and asserts it answers an `initialize` + * JSON-RPC frame, so a regression in the published entry (a settle race that + * exits before the bridge attaches, a stdout logger leaking onto the protocol) + * fails here. + * + * It build-gates: SKIPS if `lib/bin.js` is absent (suite run without + * `pnpm run build`); CI runs it after the build step. Setup mirrors a real + * install (a temp dir whose `node_modules` symlinks the built packages) and runs + * `node --expose-internals` (the cordis Loader resolves bare plugin specifiers + * via its internal module loader, active only under that flag). KEYLESS: + * `initialize` never reaches the model; a dummy key lets `llm-deepseek` boot. + */ + +const repoRoot = fileURLToPath(new URL('../../../../', import.meta.url)) +const acpBin = join(repoRoot, 'packages/ui/acp-agent/lib/bin.js') + +const dshPackages = [ + 'core/agent-core', 'core/agent', 'core/session', 'core/system-prompt', + 'core/tools', 'core/agent-loop', 'llm/llm', 'llm/llm-deepseek', 'bash/bash', + 'bash/bash-local', 'bash/tool-bash', 'support/invariants', + 'session-persistence/session-persistence', + 'session-persistence/session-persistence-jsonl', 'ui/acp', 'ui/acp-agent', +] +const vendorPackages = [ + 'cordis', 'loader', 'include', 'timer', 'hmr', 'logger-console', + 'schemastery', 'cosmokit', +] +// Third-party deps the ACP bridge needs (resolved from the acp package's own +// node_modules and linked into the consumer so plain node finds them). +const npmDeps = ['@agentclientprotocol/sdk', 'zod'] + +async function pkgName(absDir: string): Promise { + const json = JSON.parse(await readFile(join(absDir, 'package.json'), 'utf8')) as { name: string } + return json.name +} + +async function link(target: string, name: string, nm: string): Promise { + const dest = join(nm, name) + await mkdir(dirname(dest), { recursive: true }) + await symlink(target, dest) +} + +/** Build a temp consumer dir + a minimal acp `cordis.yml`. Returns the dir. */ +async function makeConsumer(): Promise { + const dir = await mkdtemp(join(tmpdir(), 'acp-built-bin-')) + const nm = join(dir, 'node_modules') + for (const rel of dshPackages) { + const abs = join(repoRoot, 'packages', rel) + await link(abs, await pkgName(abs), nm) + } + for (const v of vendorPackages) { + const abs = join(repoRoot, 'vendor', v) + await link(abs, await pkgName(abs), nm) + } + for (const dep of npmDeps) { + const resolved = fileURLToPath(import.meta.resolve(`${dep}/package.json`)) + await link(dirname(resolved), dep, nm) + } + await writeFile(join(dir, 'cordis.yml'), [ + '- id: llm-deepseek', + ' name: \'@deepseek-ai/dsh-llm-deepseek\'', + ' config:', + ' apiKey: !!js process.env.DEEPSEEK_API_KEY', + ' models: [deepseek-v4-flash]', + '- id: bash', + ' name: \'@deepseek-ai/dsh-bash-local\'', + '- id: acp-agent', + ' name: \'@deepseek-ai/dsh-acp-agent\'', + ' config:', + ' model: deepseek-v4-flash', + ' systemPrompt: \'test agent\'', + '', + ].join('\n')) + return dir +} + +let consumer: string | undefined +let child: ReturnType | undefined + +afterEach(async () => { + if (child !== undefined) { child.kill('SIGKILL'); child = undefined } + if (consumer !== undefined) await rm(consumer, { recursive: true, force: true }) + consumer = undefined +}) + +describe.skipIf(!existsSync(acpBin))('dsh-acp-agent BUILT bin (node lib/bin.js, no tsx)', () => { + it('boots the published bin and answers an initialize JSON-RPC frame on stdout', async () => { + consumer = await makeConsumer() + child = spawn(process.execPath, ['--expose-internals', acpBin, './cordis.yml'], { + cwd: consumer, + // Dummy key: initialize never reaches the model, so it is never used. + env: { ...process.env, DEEPSEEK_API_KEY: process.env.DEEPSEEK_API_KEY ?? 'sk-dummy-for-boot' }, + stdio: ['pipe', 'pipe', 'pipe'], + }) + const stderr: string[] = [] + child.stderr!.setEncoding('utf8') + child.stderr!.on('data', (c: string) => stderr.push(c)) + // Tee raw stdout for a protocol-purity check, and feed it to the SDK client. + const rawOut: string[] = [] + const passthrough = new Readable({ read() {} }) + child.stdout!.on('data', (buf: Buffer) => { rawOut.push(buf.toString('utf8')); passthrough.push(buf) }) + child.stdout!.on('end', () => passthrough.push(null)) + const stream = ndJsonStream( + Writable.toWeb(child.stdin!) as WritableStream, + Readable.toWeb(passthrough) as ReadableStream, + ) + const makeClient = (_a: AcpAgent): Client => ({ + sessionUpdate(_p: SessionNotification): Promise { return Promise.resolve() }, + requestPermission(_p: RequestPermissionRequest): Promise { + return Promise.resolve({ outcome: { outcome: 'cancelled' } }) + }, + }) + const client = new ClientSideConnection(makeClient, stream) + + const init = await client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + // A response at all proves the built bin booted the bridge (the settle-race + // regression would exit before answering); loadSession proves the real app + // mounted, not a collapsed export shape. + expect(init.agentCapabilities?.loadSession).toBe(true) + expect(stderr.join('')).not.toContain('without inject') + // stdout purity: every emitted line is a JSON-RPC frame, no logger leak. + for (const line of rawOut.join('').split('\n').filter(l => l.trim().length > 0)) { + expect(() => JSON.parse(line) as unknown).not.toThrow() + } + }, 30_000) + + it('fails LOUD (non-zero exit + stderr) on a config whose directory does not exist', async () => { + // A typo'd config path must fail clearly, not exit 0. The include plugin + // itself cannot be imported from a non-existent dir; the Loader logs that and + // leaves the entry with no fiber, which boot()'s entry-load check throws on. + const { code, stderr } = await runBinExpectingExit('/nonexistent/dir/cordis.yml') + expect(code).not.toBe(0) + expect(stderr).toContain('failed to load') + }, 30_000) + + it('fails LOUD (non-zero exit + stderr) on a missing config file in a real directory', async () => { + // The directory exists (the include imports), but the file does not — the + // include's init throws "config file not found", which surfaces as an + // unhandled rejection the fail-loud guard turns into a non-zero exit. + consumer = await makeConsumer() + const { code, stderr } = await runBinExpectingExit('./does-not-exist.yml', consumer) + expect(code).not.toBe(0) + expect(stderr).toContain('config file not found') + }, 30_000) +}) + +/** Spawn the built acp bin against `configArg` and resolve with its exit code + stderr. */ +function runBinExpectingExit(configArg: string, cwd: string = tmpdir()): Promise<{ code: number; stderr: string }> { + return new Promise((resolve, reject) => { + const proc = spawn(process.execPath, ['--expose-internals', acpBin, configArg], { + cwd, + env: { ...process.env, DEEPSEEK_API_KEY: process.env.DEEPSEEK_API_KEY ?? 'sk-dummy-for-boot' }, + stdio: ['pipe', 'pipe', 'pipe'], + }) + child = proc + let stderr = '' + proc.stderr.setEncoding('utf8') + proc.stderr.on('data', (c: string) => { stderr += c }) + const timer = setTimeout(() => { proc.kill('SIGKILL'); reject(new Error(`bin did not exit within 25s. stderr:\n${stderr}`)) }, 25_000) + proc.on('exit', (code) => { clearTimeout(timer); resolve({ code: code ?? -1, stderr }) }) + proc.on('error', (err) => { clearTimeout(timer); reject(err) }) + proc.stdin.end() + }) +} diff --git a/packages/ui/stdio-agent/README.md b/packages/ui/stdio-agent/README.md index 286fe6a85b..a78df0fa72 100644 --- a/packages/ui/stdio-agent/README.md +++ b/packages/ui/stdio-agent/README.md @@ -31,7 +31,7 @@ The leaf `cordis.yml` supplies only the **swappable backends** — an LLM adapte ## The bin -`dsh-stdio-agent [path-to-cordis.yml]` (default `./cordis.yml`) loads a gitignored `.env` from the cwd (`DEEPSEEK_API_KEY` / `DEEPSEEK_BASE_URL`), then drives the cordis Loader against the config — the boot glue the `examples/*/start.ts` files once each duplicated. The `demo:echo` / `demo:coding` scripts invoke it. +`dsh-stdio-agent [path-to-cordis.yml]` (default `./cordis.yml`) loads a gitignored `.env` from the cwd (`DEEPSEEK_API_KEY` / `DEEPSEEK_BASE_URL`), then drives the cordis Loader against the config and awaits the whole plugin tree before returning. Run it under `node --expose-internals`: the cordis Loader resolves the config's bare plugin specifiers (`@deepseek-ai/dsh-*`, npm packages) through its internal module loader, which is only active under that flag. The `demo:echo` / `demo:coding` scripts invoke it that way. ## Example leaf `cordis.yml` diff --git a/packages/ui/stdio-agent/src/bin.ts b/packages/ui/stdio-agent/src/bin.ts index 015a462f52..dae0299f06 100644 --- a/packages/ui/stdio-agent/src/bin.ts +++ b/packages/ui/stdio-agent/src/bin.ts @@ -13,7 +13,7 @@ */ import { pathToFileURL } from 'node:url' -import { basename, dirname, resolve } from 'node:path' +import { dirname, resolve } from 'node:path' import { Context } from 'cordis' import Loader from '@cordisjs/plugin-loader' @@ -38,10 +38,72 @@ function loadEnv(): void { } /** - * Boot the Loader against `configPath` (resolved from the CWD). `baseUrl` is - * pinned to the config's directory and the include is handed only the basename, - * so the config's relative plugin/include paths resolve exactly as the upstream - * `cordis` bin does. Returns the root context (the process owns its lifetime). + * Make a load failure fail loud with a clear message on stderr. Covers the + * failure path the entry-tree check below cannot: when the include's + * `[Service.init]` throws (e.g. a config FILE that does not exist in a real + * directory), the cordis Loader surfaces it as an unhandled promise rejection + * AFTER `boot()` has resolved — `loader.await()` does NOT rethrow it, because + * `EntryTree.await()` uses `Promise.allSettled`, which swallows rejections. + * Node's default handler already exits non-zero on an unhandled rejection, so + * this does not change the exit code; it replaces Node's noisy stack dump with a + * single labelled line and guarantees `process.exit(1)`. Install before `boot()`. + */ +export function installFailLoud(): void { + process.on('unhandledRejection', (err: unknown) => { + process.stderr.write(`dsh-stdio-agent: fatal load failure: ${err instanceof Error ? err.stack ?? err.message : String(err)}\n`) + process.exit(1) + }) +} + +/** + * After the tree settles, assert every loader entry actually started. This is + * the load-bearing guard against the SILENT-exit-0 bug: when a plugin module + * fails to IMPORT (e.g. a config path in a non-existent directory, so the include + * plugin itself cannot be resolved), the cordis Loader catches the import error + * and only LOGS it (`entry._init`), leaving the entry with no `fiber` and + * producing no rejection — so the process would otherwise exit 0 with a usable + * config typo reported only as a log line. A started entry has a `fiber`; an + * entry with `fiber === undefined` after the tree settled never loaded. Throw on + * any such entry so `boot()` rejects (and the top-level `await` fails the process + * non-zero) instead of returning a half-empty context. + */ +function assertEntriesLoaded(ctx: Context): void { + const failed = [...ctx.loader.entries()].filter(entry => entry.fiber === undefined) + if (failed.length > 0) { + const names = failed.map(entry => entry.options.name).join(', ') + throw new Error(`dsh-stdio-agent: plugin(s) failed to load: ${names} (see the error(s) logged above)`) + } +} + +/** + * Boot the Loader against `configPath` (resolved from the CWD). The include is + * handed the config's ABSOLUTE `file://` URL as its `path`, so resolution never + * depends on `ctx.baseUrl` (an absolute URL ignores the base) and can never fall + * back to the cwd. `baseUrl` is still pinned to the config's directory so the + * config's OWN relative plugin/include paths (e.g. `./src/mock-llm.ts`) resolve + * against it. Returns the root context once the whole tree has settled. + * + * The `await ctx.loader.await()` is load-bearing: `loader.create()` returns once + * the include ENTRY is registered, but the include then loads its child plugins + * asynchronously. Without awaiting the tree, `boot()` (and `main()`) would + * resolve while the app plugins — the stdin reader, the agent loop — are still + * mounting, and a CLI process with no attached handles yet exits 0 silently. + * Awaiting the tree keeps the process alive until the app's handles are attached. + * + * `loader.await()` does NOT, however, rethrow load errors (`EntryTree.await()` + * uses `Promise.allSettled`), so failures are surfaced two ways: a plugin that + * fails to IMPORT leaves an entry with no fiber, caught here by + * {@link assertEntriesLoaded} (this `boot()` rejects); a plugin whose init + * THROWS surfaces as an unhandled rejection caught by {@link installFailLoud} + * (installed by `main()` before this runs). Together they make any load failure + * exit non-zero with a clear message. + * + * Bare plugin specifiers in the config (`@deepseek-ai/dsh-*`, npm packages) are + * resolved by the cordis Loader's internal module loader, which is only active + * under `node --expose-internals` (the flag the `demo:echo`/`demo:coding` scripts + * pass). Without it the Loader falls back to resolving relative to its own module + * and cannot find the config's plugins, so a consumer running the built bin must + * pass `--expose-internals` (or install the plugins where node hoists them). */ export async function boot(configPath: string): Promise { const absolute = resolve(process.cwd(), configPath) @@ -50,17 +112,20 @@ export async function boot(configPath: string): Promise { await ctx.plugin(Loader) await ctx.loader.create({ name: '@cordisjs/plugin-include', - config: { path: `./${basename(absolute)}` }, + config: { path: pathToFileURL(absolute).href }, }) + await ctx.loader.await() + assertEntriesLoaded(ctx) return ctx } /** - * Entry point: load `.env`, then boot the config named on argv (default - * `./cordis.yml`). Awaited at the module top level by the published bin - * (`#!/usr/bin/env node` shebang via the package's `bin` field). + * Entry point: install the fail-loud guard, load `.env`, then boot the config + * named on argv (default `./cordis.yml`). Awaited at the module top level by the + * published bin (`#!/usr/bin/env node` shebang via the package's `bin` field). */ export async function main(argv: string[] = process.argv.slice(2)): Promise { + installFailLoud() loadEnv() await boot(argv[0] ?? './cordis.yml') } diff --git a/packages/ui/stdio-agent/tests/built-bin.e2e.ts b/packages/ui/stdio-agent/tests/built-bin.e2e.ts new file mode 100644 index 0000000000..cd6bfd6475 --- /dev/null +++ b/packages/ui/stdio-agent/tests/built-bin.e2e.ts @@ -0,0 +1,166 @@ +import { spawn } from 'node:child_process' +import { cp, mkdtemp, mkdir, rm, symlink, writeFile, readFile } from 'node:fs/promises' +import { existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { afterEach, describe, expect, it } from 'vitest' + +/** + * BUILT-ARTIFACT smoke for the published `dsh-stdio-agent` bin. The other smokes + * boot `src/bin.ts` under tsx — but the package's `bin` field points at + * `lib/bin.js`, run under plain `node` by a real consumer. tsx masks two failure + * modes the built bin had: (1) `boot()` returned before the loader tree settled, + * so the process exited 0 with no output and load errors surfaced as unhandled + * rejections AFTER boot; (2) config-path resolution could fall back to the cwd. + * This test runs the REAL `lib/bin.js` under `node` (NOT tsx) and asserts the + * banner + echo round-trip, so a regression in the published entry fails here. + * + * It build-gates: if `lib/bin.js` is absent (suite run without `pnpm run build`) + * the test SKIPS with a note. CI runs it after the build step. Setup mirrors a + * real install: a temp dir whose `node_modules/@deepseek-ai/*` (and the vendored + * `cordis`/`@cordisjs/*`) are symlinked to the built packages, a `cordis.yml` + * that loads the app + the example's mock backend, and `node --expose-internals` + * (the cordis Loader resolves bare plugin specifiers via its internal module + * loader, active only under that flag — the same flag `demo:echo` passes). + */ + +const repoRoot = fileURLToPath(new URL('../../../../', import.meta.url)) +const stdioBin = join(repoRoot, 'packages/ui/stdio-agent/lib/bin.js') + +// Workspace packages the stdio app's tree needs, by repo-relative path. Each is +// symlinked into the temp consumer's node_modules under its package name, so +// plain `node` resolves the bare `@deepseek-ai/dsh-*` specifiers in cordis.yml +// to the built `lib/` (package.json `main`), exactly as an installed dep would. +const dshPackages = [ + 'core/agent-core', 'core/agent', 'core/session', 'core/system-prompt', + 'core/tools', 'core/agent-loop', 'llm/llm', 'bash/bash', 'bash/bash-local', + 'bash/tool-bash', 'support/invariants', 'support/ui-stdio', + 'session-persistence/session-persistence', + 'session-persistence/session-persistence-jsonl', 'ui/stdio-agent', +] +const vendorPackages = [ + 'cordis', 'loader', 'include', 'timer', 'hmr', 'logger-console', + 'schemastery', 'cosmokit', +] + +async function pkgName(absDir: string): Promise { + const json = JSON.parse(await readFile(join(absDir, 'package.json'), 'utf8')) as { name: string } + return json.name +} + +/** + * Build a temp consumer dir: `node_modules` with the workspace + vendor packages + * symlinked in, a `src/` carrying the example mock backend, and a `cordis.yml` + * that wires them onto the stdio app. Returns the dir (caller removes it). + */ +async function makeConsumer(welcome: string): Promise { + const dir = await mkdtemp(join(tmpdir(), 'stdio-built-bin-')) + const nm = join(dir, 'node_modules') + for (const rel of dshPackages) { + const abs = join(repoRoot, 'packages', rel) + const name = await pkgName(abs) + const target = join(nm, name) + await mkdir(dirname(target), { recursive: true }) + await symlink(abs, target) + } + for (const v of vendorPackages) { + const abs = join(repoRoot, 'vendor', v) + const name = await pkgName(abs) + const target = join(nm, name) + await mkdir(dirname(target), { recursive: true }) + await symlink(abs, target) + } + // The example's mock model + echo tool are example-local TS plugins (Node 24+ + // strips types natively, so plain `node` loads them); they import the workspace + // packages the symlinked node_modules now provides. + await cp(join(repoRoot, 'examples/echo-agent/src'), join(dir, 'src'), { recursive: true }) + await writeFile(join(dir, 'cordis.yml'), [ + '- id: mock-llm', + ' name: \'./src/mock-llm.ts\'', + '- id: echo-tool', + ' name: \'./src/echo-tool.ts\'', + '- id: bash', + ' name: \'@deepseek-ai/dsh-bash-local\'', + '- id: stdio-agent', + ' name: \'@deepseek-ai/dsh-stdio-agent\'', + ' config:', + ' model: mock-echo', + ' systemPrompt: \'demo\'', + ` welcome: '${welcome}'`, + '', + ].join('\n')) + return dir +} + +/** Run the built bin in `cwd` against `configArg` with one stdin line; resolve with stdout/stderr + exit code. */ +function runBuiltBin(cwd: string, configArg: string, line: string): Promise<{ stdout: string; code: number; stderr: string }> { + return new Promise((resolve, reject) => { + // --expose-internals: the cordis Loader resolves bare plugin specifiers via + // its internal module loader (active only under this flag); demo:echo passes + // it too. NO tsx — this is the published `node lib/bin.js` path. + const child = spawn(process.execPath, ['--expose-internals', stdioBin, configArg], { + cwd, + // Mock model: never calls the network, so no key needed. + env: { ...process.env }, + stdio: ['pipe', 'pipe', 'pipe'], + }) + let stdout = '' + let stderr = '' + child.stdout.setEncoding('utf8') + child.stdout.on('data', (c: string) => { stdout += c }) + child.stderr.setEncoding('utf8') + child.stderr.on('data', (c: string) => { stderr += c }) + const timer = setTimeout(() => { + child.kill('SIGKILL') + reject(new Error(`built bin did not exit within 25s. stdout:\n${stdout}\nstderr:\n${stderr}`)) + }, 25_000) + child.on('exit', (code) => { clearTimeout(timer); resolve({ stdout, code: code ?? -1, stderr }) }) + child.on('error', (err) => { clearTimeout(timer); reject(err) }) + child.stdin.write(`${line}\n`) + child.stdin.end() + }) +} + +let consumer: string | undefined + +afterEach(async () => { + if (consumer !== undefined) await rm(consumer, { recursive: true, force: true }) + consumer = undefined +}) + +describe.skipIf(!existsSync(stdioBin))('dsh-stdio-agent BUILT bin (node lib/bin.js, no tsx)', () => { + it('boots the published bin, prints its banner, and runs the echo tool round-trip', async () => { + consumer = await makeConsumer('BUILT-BIN-OK ready.') + const { stdout, code, stderr } = await runBuiltBin(consumer, './cordis.yml', 'echo hi') + expect(stderr).not.toContain('UNHANDLED') + expect(stderr).not.toContain('without inject') + // The banner proves boot() awaited the tree (the settle-race regression would + // exit 0 with empty stdout); the round-trip proves the whole app mounted. + expect(stdout).toContain('BUILT-BIN-OK ready.') + expect(stdout).toContain('[tool call] echo') + expect(stdout).toContain('[tool result] ECHO: HI') + expect(code).toBe(0) + }, 30_000) + + it('fails LOUD (non-zero exit + stderr) on a config whose directory does not exist', async () => { + // A consumer who typos the config path must get a clear failure, not silent + // success. This dir does not exist, so the include PLUGIN itself fails to + // import; the cordis Loader logs that and leaves the entry with no fiber (no + // rejection), which `boot()`'s entry-load check turns into a thrown error. + consumer = await makeConsumer('unused') + const { code, stderr } = await runBuiltBin(consumer, '/nonexistent/dir/cordis.yml', '') + expect(code).not.toBe(0) + expect(stderr).toContain('failed to load') + }, 30_000) + + it('fails LOUD (non-zero exit + stderr) on a missing config file in a real directory', async () => { + // The config DIRECTORY exists (the include plugin imports), but the file does + // not — the include's init throws "config file not found", which surfaces as + // an unhandled rejection the fail-loud guard turns into a non-zero exit. + consumer = await makeConsumer('unused') + const { code, stderr } = await runBuiltBin(consumer, './does-not-exist.yml', '') + expect(code).not.toBe(0) + expect(stderr).toContain('config file not found') + }, 30_000) +})