diff --git a/packages/ui/acp-agent/src/bin.ts b/packages/ui/acp-agent/src/bin.ts index 1352cde0ed..24e31f3251 100644 --- a/packages/ui/acp-agent/src/bin.ts +++ b/packages/ui/acp-agent/src/bin.ts @@ -85,9 +85,13 @@ export function installFailLoud(): void { * `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. + * + * A `disabled` entry is the one legitimate fiber-less state: `Entry.refresh()` + * deliberately skips `init()` for it, so it settles without a fiber by design — + * a valid "plugin turned off" config, not a failed import. Exclude it. */ function assertEntriesLoaded(ctx: Context): void { - const failed = [...ctx.loader.entries()].filter(entry => entry.fiber === undefined) + const failed = [...ctx.loader.entries()].filter(entry => entry.fiber === undefined && !entry.disabled) 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)`) diff --git a/packages/ui/stdio-agent/src/bin.ts b/packages/ui/stdio-agent/src/bin.ts index dae0299f06..92cd9f5e90 100644 --- a/packages/ui/stdio-agent/src/bin.ts +++ b/packages/ui/stdio-agent/src/bin.ts @@ -66,9 +66,14 @@ export function installFailLoud(): void { * 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. + * + * A `disabled` entry is the one legitimate fiber-less state: `Entry.refresh()` + * deliberately skips `init()` for it, so it settles without a fiber by design. + * That is a valid config (a consumer turning an optional plugin off), not a + * failed import — exclude it so the guard catches only real load failures. */ function assertEntriesLoaded(ctx: Context): void { - const failed = [...ctx.loader.entries()].filter(entry => entry.fiber === undefined) + const failed = [...ctx.loader.entries()].filter(entry => entry.fiber === undefined && !entry.disabled) 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)`) diff --git a/packages/ui/stdio-agent/tests/built-bin.e2e.ts b/packages/ui/stdio-agent/tests/built-bin.e2e.ts index cd6bfd6475..7605b35bb7 100644 --- a/packages/ui/stdio-agent/tests/built-bin.e2e.ts +++ b/packages/ui/stdio-agent/tests/built-bin.e2e.ts @@ -53,8 +53,13 @@ async function pkgName(absDir: string): Promise { * 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). + * + * `disabledBrokenEntry` appends an entry that points at a non-existent plugin but + * is marked `disabled: true`. The Loader leaves a disabled entry fiber-less by + * design, so it exercises that the fail-loud entry-load guard does NOT mistake a + * valid disabled entry for a failed import. */ -async function makeConsumer(welcome: string): Promise { +async function makeConsumer(welcome: string, disabledBrokenEntry = false): Promise { const dir = await mkdtemp(join(tmpdir(), 'stdio-built-bin-')) const nm = join(dir, 'node_modules') for (const rel of dshPackages) { @@ -88,6 +93,9 @@ async function makeConsumer(welcome: string): Promise { ' model: mock-echo', ' systemPrompt: \'demo\'', ` welcome: '${welcome}'`, + ...disabledBrokenEntry + ? ['- id: off', ' name: \'./src/does-not-exist.ts\'', ' disabled: true'] + : [], '', ].join('\n')) return dir @@ -143,6 +151,18 @@ describe.skipIf(!existsSync(stdioBin))('dsh-stdio-agent BUILT bin (node lib/bin. expect(code).toBe(0) }, 30_000) + it('boots cleanly when the config disables an (otherwise unresolvable) entry', async () => { + // A `disabled: true` entry settles without a fiber by design; the fail-loud + // entry-load guard must NOT mistake it for a failed import. Even though its + // plugin path does not exist, the app boots and the round-trip works. + consumer = await makeConsumer('DISABLED-OK ready.', true) + const { stdout, code, stderr } = await runBuiltBin(consumer, './cordis.yml', 'echo hi') + expect(stderr).not.toContain('failed to load') + expect(stdout).toContain('DISABLED-OK ready.') + 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 diff --git a/scripts/verify-package-paths.ts b/scripts/verify-package-paths.ts index 9f4a4aaa8e..7bec754dba 100644 --- a/scripts/verify-package-paths.ts +++ b/scripts/verify-package-paths.ts @@ -28,10 +28,13 @@ * Scope mirrors the other doc gates plus repo-authored TypeScript: Markdown * across README/docs/packages/AGENTS, and `.ts` under packages/** and * examples/** (excluding built `lib/`, `*.d.ts`, and vendored upstream source). - * A reference whose target path goes through a `lib/` segment is also skipped: - * that is a build OUTPUT (`packages/ui/acp-agent/lib/bin.js`), emitted only by - * `pnpm run build`, which CI runs AFTER this gate — flagging it would be a false - * positive on a path that is correct but not yet on disk. + * A reference to a package's build OUTPUT (`packages///lib/…`, + * e.g. `packages/ui/acp-agent/lib/bin.js` cited by a built-bin smoke) is also + * skipped — it is emitted only by `pnpm run build`, which CI runs AFTER this + * gate, so flagging it would be a false positive on a path that is correct but + * not yet on disk. That skip is scoped to a REAL package root: a stale + * group-less `packages/acp-agent/lib/bin.js` is still flagged (its root does not + * exist — exactly the moved-package drift this gate catches). * * Run: `tsx scripts/verify-package-paths.ts`. */ @@ -115,13 +118,17 @@ function findViolations(absPath: string): Violation[] { // class may have swallowed (`packages/core/tools.` / `…/tools/`). const ref = m[0].replace(/[./]+$/, '') if (existsSync(resolve(root, ref))) continue - // A reference INTO a package's built `lib/` is a build-output path, not an + // A reference INTO a package's built `lib/` is a build OUTPUT, not an // authored-source location: it does not exist until `pnpm run build` emits - // it, and CI runs this gate BEFORE the build step. This gate reports stale - // SOURCE paths (a moved package), so skip `lib/` targets the same way the - // file scan excludes `lib/` files — a `packages/ui/acp-agent/lib/bin.js` - // citation in a built-bin smoke is correct, just not yet on disk at lint. - if (ref.split('/').includes('lib')) continue + // it, and CI runs this gate BEFORE the build step. Skip it — but ONLY when + // the `packages//` ROOT it sits under is real and on disk, so + // `packages/ui/acp-agent/lib/bin.js` (correct, just not yet built) is + // exempt while a stale `packages/acp-agent/lib/bin.js` (group-less, the + // exact moved-package drift this gate exists to catch) still flags. A bare + // `lib` segment is not a blanket escape hatch. + const parts = ref.split('/') + const libAt = parts.indexOf('lib') + if (libAt === 3 && existsSync(resolve(root, parts.slice(0, 3).join('/')))) continue // Only a stale path to a REAL (moved) package is a violation; a segment // matching a live package name is the drift signal. const segments = ref.split('/').slice(1)