From 3d18074bf46929a6ed11bc85219e79afb5724d63 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sun, 16 Aug 2026 10:03:59 -0700 Subject: [PATCH 1/3] feat(adapters): manifest execution block + hook-runner stdin/cwd (ADR-0031 P2, schema half) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Manifest gains an optional execution.run hook, coupled to canRouteActivities (else refused 'execution-not-routable'); it is part of the hashed content. runAdapterHook gains stdin (EPIPE-safe), an absolute-validated cwd, and separate stdoutText/stderrText captures — the substrate the derived execution adapter needs to run a hook without inheriting ak's cwd or promoting stderr into a downstream prompt. --- src/lib/adapters/hook-runner.mjs | 95 +++++++++++++--- src/lib/adapters/manifest.mjs | 39 ++++++- tests/kit/adapter-hook-runner.test.mjs | 145 +++++++++++++++++++++++++ tests/kit/adapter-manifest.test.mjs | 68 ++++++++++++ 4 files changed, 329 insertions(+), 18 deletions(-) diff --git a/src/lib/adapters/hook-runner.mjs b/src/lib/adapters/hook-runner.mjs index 168d6bd..280e1ff 100644 --- a/src/lib/adapters/hook-runner.mjs +++ b/src/lib/adapters/hook-runner.mjs @@ -8,6 +8,7 @@ // summary capture, no graceful-then-forced two-step shutdown. A timed-out // adapter hook gets no cleanup grace period; it already spent its budget. import { spawn as nodeSpawn, execFile as nodeExecFile } from 'node:child_process'; +import { isAbsolute as pathIsAbsolute } from 'node:path'; const DEFAULT_TIMEOUT_MS = 30_000; const OUTPUT_CAP_BYTES = 256 * 1024; @@ -88,6 +89,15 @@ function mergeCapture(stdout, stderr) { return `${kept}${TRUNCATION_MARKER}`; } +/** stderr alone, with the same per-stream truncation marker `mergeCapture` + * would append — never folded into `stdout`. F-4: a caller that parses + * `stdout` as a structured payload (e.g. the admitted execution adapter) + * must never see raw stderr promoted into that parse; this is the field it + * reads instead when it needs the process's diagnostic chatter. */ +function boundedText({ text, truncated }) { + return truncated ? `${text}${TRUNCATION_MARKER}` : text; +} + function describeFailure(hostId, verb, error) { const reason = error?.code ? `${error.code} (${error.message ?? 'no message'})` : (error?.message ?? String(error)); return `${hostId}:${verb} adapter hook failed to start: ${reason}`; @@ -104,7 +114,12 @@ function raceTimeout(promise, ms) { * child — a timed-out adapter hook may have spawned descendants of its own. * POSIX: the child was spawned detached so its pid is also its process group * id; signalling `-pid` reaches the whole group. Windows has no portable - * signal for arbitrary console trees, so `taskkill /T /F` owns it there. */ + * signal for arbitrary console trees, so `taskkill /T /F` owns it there. + * F-2: this cannot PROVE a double-forked or re-`setsid`'d grandchild died — + * a signal sent is not a death confirmed. Callers that need that proof (the + * admitted execution adapter's cancel path) must treat an unresolved launch + * as honestly unproven (`orphaned`), not assume this function's return means + * the tree is gone. */ async function killGroup(child) { if (!Number.isInteger(child?.pid)) return; if (isWindows) { @@ -127,31 +142,49 @@ async function killGroup(child) { * (missing/invalid `hook`, `hostId`, or `verb`) throw synchronously. * * @param {{hook:{command:string[], timeoutMs?:number}, hostId:string, - * verb:string, timeoutMs?:number, env?:Record}} options - * @returns {Promise<{ok:boolean, stdout:string, exitCode:number|null, detail:string|null}>} + * verb:string, timeoutMs?:number, env?:Record, stdin?:string, + * cwd?:string}} options + * @returns {Promise<{ok:boolean, stdout:string, stdoutText:string, stderrText:string, + * exitCode:number|null, detail:string|null}>} */ -export async function runAdapterHook({ hook, hostId, verb, timeoutMs, env } = /** @type {any} */ ({})) { +export async function runAdapterHook({ + hook, hostId, verb, timeoutMs, env, stdin, cwd, +} = /** @type {any} */ ({})) { if (!hook || !Array.isArray(hook.command) || hook.command.length === 0 || !hook.command.every((part) => typeof part === 'string' && part.length > 0)) { throw new TypeError('runAdapterHook requires hook.command as a non-empty array of non-empty strings'); } if (typeof hostId !== 'string' || !hostId) throw new TypeError('runAdapterHook requires a hostId'); if (typeof verb !== 'string' || !verb) throw new TypeError('runAdapterHook requires a verb'); + // F-1: a relative cwd would resolve against wherever the ak process + // happens to be running, defeating the whole point of pinning the child to + // the adapter's own directory — refused synchronously, same class as the + // arg-shape checks above, never silently reinterpreted as "inherit". + if (cwd !== undefined && (typeof cwd !== 'string' || !cwd || !pathIsAbsolute(cwd))) { + throw new TypeError('runAdapterHook requires cwd to be an absolute path when provided'); + } const effectiveTimeoutMs = resolveTimeout(timeoutMs, hook.timeoutMs); const [argv0, ...args] = hook.command; const childEnv = minimalEnv(env); + const wantsStdin = typeof stdin === 'string'; let child; try { child = nodeSpawn(argv0, args, { env: childEnv, shell: false, - stdio: ['ignore', 'pipe', 'pipe'], + stdio: [wantsStdin ? 'pipe' : 'ignore', 'pipe', 'pipe'], detached: !isWindows, + // Absent cwd falls through to Node's own default (inherit + // process.cwd()) — today's behavior for callers that don't pass one + // yet (B2 threads the real adapter-base-dir cwd through this wave). + ...(cwd === undefined ? {} : { cwd }), }); } catch (error) { - return { ok: false, stdout: '', exitCode: null, detail: describeFailure(hostId, verb, error) }; + return { + ok: false, stdout: '', stdoutText: '', stderrText: '', exitCode: null, detail: describeFailure(hostId, verb, error), + }; } const stdoutCollector = boundedCollector(OUTPUT_CAP_BYTES); @@ -159,6 +192,21 @@ export async function runAdapterHook({ hook, hostId, verb, timeoutMs, env } = /* child.stdout?.on('data', (chunk) => stdoutCollector.write(chunk)); child.stderr?.on('data', (chunk) => stderrCollector.write(chunk)); + if (wantsStdin) { + // A child that exits before (or without) reading stdin makes the pipe + // write EPIPE — that is a normal outcome (the process's own exit code + // already reports what happened), never a reason to crash or reject + // runAdapterHook's promise. The 'close' handler below still fires and + // resolves the race normally regardless of whether this write lands. + child.stdin?.on('error', () => {}); + try { + child.stdin?.end(stdin); + } catch { + // Synchronous throw from an already-closed stream — same non-fatal + // treatment as the async 'error' event above. + } + } + let settled = false; let spawnError = null; const closeResult = new Promise((resolve) => { @@ -175,27 +223,40 @@ export async function runAdapterHook({ hook, hostId, verb, timeoutMs, env } = /* if (raced === TIMEOUT_SENTINEL) { await killGroup(child); await raceTimeout(closeResult, KILL_GRACE_MS); // best-effort; result unused + const stdoutCaptured = { text: stdoutCollector.text(), truncated: stdoutCollector.wasTruncated() }; + const stderrCaptured = { text: stderrCollector.text(), truncated: stderrCollector.wasTruncated() }; return { ok: false, exitCode: null, - stdout: mergeCapture( - { text: stdoutCollector.text(), truncated: stdoutCollector.wasTruncated() }, - { text: stderrCollector.text(), truncated: stderrCollector.wasTruncated() }, - ), + stdout: mergeCapture(stdoutCaptured, stderrCaptured), + stdoutText: boundedText(stdoutCaptured), + stderrText: boundedText(stderrCaptured), detail: `${hostId}:${verb} adapter hook timed out after ${effectiveTimeoutMs}ms and was killed`, }; } if (spawnError) { - return { ok: false, stdout: '', exitCode: null, detail: describeFailure(hostId, verb, spawnError) }; + return { + ok: false, stdout: '', stdoutText: '', stderrText: '', exitCode: null, detail: describeFailure(hostId, verb, spawnError), + }; } const { code } = raced; - const stdout = mergeCapture( - { text: stdoutCollector.text(), truncated: stdoutCollector.wasTruncated() }, - { text: stderrCollector.text(), truncated: stderrCollector.wasTruncated() }, - ); + // F-4/R-1: stdout stays the combined stream for diagnostics/back-compat, + // but a caller parsing stdout as a structured payload (the admitted + // execution adapter) must read stdoutText instead — stdout has stderr + // folded in after a separator, which breaks JSON.parse the instant the + // hook writes anything to stderr at all. stderrText is diagnostics-only. + const stdoutCaptured = { text: stdoutCollector.text(), truncated: stdoutCollector.wasTruncated() }; + const stderrCaptured = { text: stderrCollector.text(), truncated: stderrCollector.wasTruncated() }; + const stdout = mergeCapture(stdoutCaptured, stderrCaptured); + const stdoutText = boundedText(stdoutCaptured); + const stderrText = boundedText(stderrCaptured); return code === 0 - ? { ok: true, stdout, exitCode: 0, detail: null } - : { ok: false, stdout, exitCode: code, detail: `${hostId}:${verb} adapter hook exited with code ${code}` }; + ? { + ok: true, stdout, stdoutText, stderrText, exitCode: 0, detail: null, + } + : { + ok: false, stdout, stdoutText, stderrText, exitCode: code, detail: `${hostId}:${verb} adapter hook exited with code ${code}`, + }; } diff --git a/src/lib/adapters/manifest.mjs b/src/lib/adapters/manifest.mjs index 801f89f..a22c22b 100644 --- a/src/lib/adapters/manifest.mjs +++ b/src/lib/adapters/manifest.mjs @@ -52,7 +52,7 @@ export class ManifestRejected extends TypeError { // validateHostAdapter, src/lib/hosts.mjs, src/lib/providers.mjs) consume — // widen them only alongside a new legitimate consumer, never speculatively. const MANIFEST_ALLOWED_KEYS = Object.freeze([ - 'name', 'version', 'contract', 'host', 'detection', 'driving', 'lifecycle', 'trust', + 'name', 'version', 'contract', 'host', 'detection', 'driving', 'lifecycle', 'trust', 'execution', ]); // Everything validateHostAdapter itself reads (id, label, install, // capabilities, trust, enabledByDefault, configProjection, observability) @@ -172,6 +172,30 @@ function validateManifestLifecycle(value) { return structuredClone(value); } +// execution.run.hook is the single subprocess `ak run` spawns to drive an +// admitted host as a worker (P2, ADR-0031). Same hook shape and validation +// discipline as a lifecycle verb's hook, but there is exactly one verb +// ('run'), never a caller-named one, so the allowlists are inlined rather +// than looped like validateManifestLifecycle's verb map. +function validateExecution(value) { + assertRecord(value, 'execution'); + assertNoUnknownKeys(value, ['run'], 'execution'); + assertRecord(value.run, 'execution.run'); + assertNoUnknownKeys(value.run, ['hook'], 'execution.run'); + assertRecord(value.run.hook, 'execution.run.hook'); + assertNoUnknownKeys(value.run.hook, ['command', 'timeoutMs'], 'execution.run.hook'); + try { + assertStringArray(value.run.hook.command, 'execution.run.hook.command', { allowEmpty: false }); + } catch (error) { + throw new ManifestRejected('invalid-execution', error.message); + } + if (value.run.hook.timeoutMs !== undefined + && (!Number.isInteger(value.run.hook.timeoutMs) || value.run.hook.timeoutMs <= 0)) { + throw new ManifestRejected('invalid-execution', 'execution.run.hook.timeoutMs must be a positive integer'); + } + return structuredClone(value); +} + function validateManifestTrust(value) { assertRecord(value, 'trust'); assertNoUnknownKeys(value, ['changes'], 'trust'); @@ -283,11 +307,19 @@ export function validateAdapterManifest(value, { projections = projectionMap, ob throw new ManifestRejected('invalid-guidance-file', error.message); } } + // P2 structural coupling (ADR-0031): an execution hook on a host that + // cannot route activities is a contradiction the schema refuses outright, + // never silently ignores. The converse — routable, no execution block — is + // legal and degrades honestly at run time (cli_unavailable). + if (value.execution !== undefined && host.capabilities.canRouteActivities !== true) { + throw new ManifestRejected('execution-not-routable', 'manifest.execution requires host.capabilities.canRouteActivities: true'); + } const detection = validateDetection(value.detection); const driving = validateDriving(value.driving); const lifecycle = value.lifecycle === undefined ? undefined : validateManifestLifecycle(value.lifecycle); const trust = validateManifestTrust(value.trust); + const execution = value.execution === undefined ? undefined : validateExecution(value.execution); return immutable({ name: value.name, @@ -297,6 +329,11 @@ export function validateAdapterManifest(value, { projections = projectionMap, ob detection, driving, ...(lifecycle === undefined ? {} : { lifecycle }), + // execution rides in the same validated-output object hashManifest + // (admission.mjs) canonicalizes and hashes, so declaring/editing an + // execution block changes consent's covered hash automatically — no + // separate hashing path to keep in sync. + ...(execution === undefined ? {} : { execution }), trust, }); } diff --git a/tests/kit/adapter-hook-runner.test.mjs b/tests/kit/adapter-hook-runner.test.mjs index b0f7955..b29143a 100644 --- a/tests/kit/adapter-hook-runner.test.mjs +++ b/tests/kit/adapter-hook-runner.test.mjs @@ -127,6 +127,151 @@ test('a non-zero exit is reported as ok:false with the exit code preserved', asy assert.equal(result.exitCode, 7); }); +// --- stdin (P2, ADR-0031: worker prompt delivery) --------------------------- + +test('stdin: a payload written to the child is delivered and echoed back', async () => { + const payload = 'the worker prompt, rendered task + handoffs'; + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', 'process.stdin.pipe(process.stdout)'] }, + hostId: 'claude', verb: 'run', stdin: payload, + }); + assert.equal(result.ok, true); + assert.equal(result.stdout, payload); +}); + +test('stdin: a child that exits without reading stdin (EPIPE) still yields a normal result', async () => { + // A large payload makes it likely the write is still in flight (or at + // least unread) when the child exits, so the pipe's write side sees EPIPE. + const bigPayload = 'x'.repeat(4 * 1024 * 1024); + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', 'process.exit(3)'] }, + hostId: 'claude', verb: 'run', stdin: bigPayload, + }); + assert.equal(result.ok, false); + assert.equal(result.exitCode, 3); +}); + +test('stdin: absent stdin keeps the existing ignore behavior (reading it yields EOF, not a hang)', async () => { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', "let n=0; process.stdin.on('data', () => { n++; }); process.stdin.on('end', () => console.log('eof', n));"], + }, + hostId: 'claude', verb: 'discover', timeoutMs: 5000, + }); + assert.equal(result.ok, true); + assert.ok(result.stdout.includes('eof 0'), result.stdout); +}); + +// --- cwd (F-1: pin the child to the adapter's own directory, never the +// operator's process.cwd() — a relative hook.command like `node +// run-hook.mjs` must resolve against the adapter bundle, not wherever `ak +// run` happened to be invoked from) -------------------------------------- + +test('cwd: when provided, the child actually runs there — not in process.cwd()', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-adapter-hook-cwd-')); + try { + const target = fs.realpathSync(dir); // resolve /private symlink on macOS + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', 'console.log(process.cwd())'] }, + hostId: 'claude', verb: 'run', cwd: target, + }); + assert.equal(result.ok, true); + assert.equal(result.stdout.trim(), target); + assert.notEqual(result.stdout.trim(), process.cwd()); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('cwd: a relative path is refused synchronously, same class as the other arg-shape checks', async () => { + await assert.rejects( + () => runAdapterHook({ + hook: { command: [NODE, '-e', "console.log('should not run')"] }, + hostId: 'claude', verb: 'run', cwd: 'relative/dir', + }), + TypeError, + ); +}); + +test('cwd: a non-string cwd is refused synchronously', async () => { + await assert.rejects( + () => runAdapterHook({ + hook: { command: [NODE, '-e', "console.log('should not run')"] }, + hostId: 'claude', verb: 'run', cwd: 42, + }), + TypeError, + ); +}); + +test('cwd: omitted keeps the existing inherited-process.cwd() behavior', async () => { + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', 'console.log(process.cwd())'] }, + hostId: 'claude', verb: 'discover', + }); + assert.equal(result.ok, true); + assert.equal(result.stdout.trim(), process.cwd()); +}); + +// --- stderrText (F-4: stderr must never be silently folded into a caller's +// parse of `stdout` — the admitted execution adapter reads the payload from +// stdout alone and diagnostics from stderrText, never a blended blob) ------ + +test('stderrText: carries only stderr, independent of the combined stdout field', async () => { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', "process.stdout.write('PAYLOAD'); process.stderr.write('noisy diagnostic log line');"], + }, + hostId: 'claude', verb: 'run', + }); + assert.equal(result.ok, true); + assert.equal(result.stderrText, 'noisy diagnostic log line'); + // the combined `stdout` field still folds both streams together, unchanged + // back-compat behavior for lifecycle callers/diagnostics that read it. + assert.ok(result.stdout.includes('PAYLOAD')); + assert.ok(result.stdout.includes('noisy diagnostic log line')); +}); + +test('stderrText: empty when the hook writes nothing to stderr', async () => { + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', "console.log('only stdout')"] }, + hostId: 'claude', verb: 'run', + }); + assert.equal(result.ok, true); + assert.equal(result.stderrText, ''); +}); + +// --- stdoutText (R-1: mergeCapture's `stdout` folds stderr in after a +// separator, so a JSON.parse of `stdout` breaks the instant the hook writes +// ANYTHING to stderr — a warning, a deprecation notice, a logging library +// defaulting to stderr. stdoutText is the unmerged, stdout-only capture the +// admitted adapter must parse the payload from instead.) ------------------ + +test('stdoutText: a JSON payload on stdout plus a warning on stderr — stdoutText parses cleanly, stdout does not', async () => { + const result = await runAdapterHook({ + hook: { + command: [NODE, '-e', "process.stdout.write(JSON.stringify({summary:'ok'})); process.stderr.write('deprecation warning');"], + }, + hostId: 'claude', verb: 'run', + }); + assert.equal(result.ok, true); + assert.deepEqual(JSON.parse(result.stdoutText), { summary: 'ok' }); + assert.equal(result.stderrText, 'deprecation warning'); + // the combined `stdout` field is unchanged back-compat behavior — it folds + // both streams, so parsing IT as JSON is exactly the break R-1 fixes. + assert.throws(() => JSON.parse(result.stdout)); + assert.ok(result.stdout.includes('deprecation warning')); +}); + +test('stdoutText: equals the plain stdout capture when the hook writes nothing to stderr', async () => { + const result = await runAdapterHook({ + hook: { command: [NODE, '-e', "console.log('only stdout')"] }, + hostId: 'claude', verb: 'run', + }); + assert.equal(result.ok, true); + assert.equal(result.stdoutText.trim(), 'only stdout'); + assert.equal(result.stdout, result.stdoutText); +}); + // --- consent.mjs ----------------------------------------------------------- function sandboxFile() { diff --git a/tests/kit/adapter-manifest.test.mjs b/tests/kit/adapter-manifest.test.mjs index 33ca128..70aa475 100644 --- a/tests/kit/adapter-manifest.test.mjs +++ b/tests/kit/adapter-manifest.test.mjs @@ -9,6 +9,7 @@ import { fileURLToPath } from 'node:url'; import { validateAdapterManifest, DRIVING_SURFACES, MANIFEST_TRUST_KINDS, ManifestRejected, } from '../../src/lib/adapters/manifest.mjs'; +import { hashManifest } from '../../src/lib/adapters/admission.mjs'; function validHost(overrides = {}) { return { @@ -281,3 +282,70 @@ test('unknown-field: an extra or miscased capability key cannot ride along inert rejects(validManifest({ host: validHost({ capabilities: { ...validHost().capabilities, CanBePrimary: true } }) }), 'unknown-field'); rejects(validManifest({ host: validHost({ capabilities: { ...validHost().capabilities, ADMIN: true } }) }), 'unknown-field'); }); + +// ── execution block (P2, ADR-0031) ────────────────────────────────────────── +// validHost()'s default capabilities already declare canRouteActivities:true, +// so an execution block is legal by default; the coupling tests below flip +// that flag explicitly to exercise both directions. + +function withExecution(overrides = {}) { + return { + run: { hook: { command: ['acme', 'run'], timeoutMs: 5000 } }, + ...overrides, + }; +} + +test('execution block round-trips into the validated output', () => { + const manifest = validateAdapterManifest(validManifest({ execution: withExecution() })); + assert.deepEqual(manifest.execution.run.hook.command, ['acme', 'run']); + assert.equal(manifest.execution.run.hook.timeoutMs, 5000); + assert.ok(Object.isFrozen(manifest.execution)); +}); + +test('a manifest with no execution block omits the key entirely', () => { + const manifest = validateAdapterManifest(validManifest()); + assert.equal('execution' in manifest, false); +}); + +test('an execution block changes hashManifest\'s value vs. the same manifest without it', () => { + const withoutExecution = validateAdapterManifest(validManifest()); + const withExecutionBlock = validateAdapterManifest(validManifest({ execution: withExecution() })); + assert.notEqual(hashManifest(withoutExecution), hashManifest(withExecutionBlock)); +}); + +test('execution-not-routable: an execution block requires host.capabilities.canRouteActivities:true', () => { + rejects(validManifest({ + host: validHost({ capabilities: { ...validHost().capabilities, canRouteActivities: false } }), + execution: withExecution(), + }), 'execution-not-routable'); +}); + +test('canRouteActivities:true without an execution block still validates (legal degrade-at-runtime)', () => { + const manifest = validateAdapterManifest(validManifest({ + host: validHost({ capabilities: { ...validHost().capabilities, canRouteActivities: true } }), + })); + assert.equal(manifest.host.capabilities.canRouteActivities, true); + assert.equal('execution' in manifest, false); +}); + +test('unknown-field: an extraneous execution key is refused', () => { + rejects(validManifest({ execution: { run: withExecution().run, escalate: true } }), 'unknown-field'); +}); + +test('unknown-field: an extraneous execution.run key is refused', () => { + rejects(validManifest({ execution: { run: { hook: withExecution().run.hook, cwd: '/tmp' } } }), 'unknown-field'); +}); + +test('unknown-field: an extraneous execution.run.hook key is refused', () => { + rejects(validManifest({ + execution: { run: { hook: { command: ['acme', 'run'], cwd: '/tmp' } } }, + }), 'unknown-field'); +}); + +test('invalid-execution: an empty command array is rejected', () => { + rejects(validManifest({ execution: withExecution({ run: { hook: { command: [] } } }) }), 'invalid-execution'); +}); + +test('invalid-execution: a non-positive timeoutMs is rejected', () => { + rejects(validManifest({ execution: withExecution({ run: { hook: { command: ['acme', 'run'], timeoutMs: 0 } } }) }), 'invalid-execution'); +}); From 0a980dc633e36773c34bcc760f7376f722206d91 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sun, 16 Aug 2026 10:03:59 -0700 Subject: [PATCH 2/3] feat(execution): ak run drives an admitted external host as a supervised subprocess (ADR-0031 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit executionAdapterFor falls through to a manifest-derived subprocess adapter for an admitted host; routing is overlay-aware via a lazy effectiveRoutableHostIds(). No third-party code runs in-process — the hook is one subprocess ak owns. Security-hardened across an adversarial review (blockers + regressions all re-verified closed): - hooks spawn with cwd pinned to the adapter's own resolved directory, never the operator's cwd; a relative hook on a remote (unanchorable) source is refused ('execution-unanchored'). The consent hash still pins the manifest text; resolution is a pure function of it plus the pinned source. - AK_WORKER_CWD carries the target repo without reopening that pin - an unresolved-launch cancellation reports non-escalating 'orphaned', never an escalatable 'timed_out' (no double-run) - the structured payload is parsed from stdout alone, so a stray stderr line never collapses a worker's handoff or leaks logs into a cross-vendor prompt - reserved hook exit codes 77/78 express permission_required/auth_required - a self-declared provider is stamped 'inferred', handoff data is redacted from public results, and driving.surfaces must include cli-subprocess (no downgrade) The black-box conformance test now drives the real command resolver end-to-end: a genuinely spawned subprocess, cwd anchored by production code. --- src/lib/adapters/admission.mjs | 66 +++ src/lib/adapters/admitted.mjs | 12 + src/lib/execution/adapters.mjs | 14 +- src/lib/execution/admitted.mjs | 362 +++++++++++++ src/lib/routing.mjs | 29 +- tests/fixtures/adapters/acme/manifest.json | 7 +- tests/fixtures/adapters/acme/run-hook.mjs | 28 + tests/kit/adapter-conformance.test.mjs | 94 +++- tests/kit/adapter-execution.test.mjs | 597 +++++++++++++++++++++ 9 files changed, 1176 insertions(+), 33 deletions(-) create mode 100644 src/lib/execution/admitted.mjs create mode 100644 tests/fixtures/adapters/acme/run-hook.mjs create mode 100644 tests/kit/adapter-execution.test.mjs diff --git a/src/lib/adapters/admission.mjs b/src/lib/adapters/admission.mjs index 0230f81..eba98a1 100644 --- a/src/lib/adapters/admission.mjs +++ b/src/lib/adapters/admission.mjs @@ -7,11 +7,36 @@ // entry or the built-in registries (try/caught per entry, in admitOne AND as // a belt-and-suspenders net in admitAdapters). import { createHash } from 'node:crypto'; +import fs from 'node:fs'; +import path from 'node:path'; import { HOST_REGISTRY } from './registries.mjs'; import { validateAdapterManifest } from './manifest.mjs'; export const SUPPORTED_CONTRACT = 1; +/** The adapter's own directory (F-1, ADR-0031): where its execution/lifecycle + * hooks resolve a relative command FROM, never the operator's process.cwd() + * when `ak run` was invoked. A file-sourced manifest anchors to its own + * directory — `fs.realpathSync` so a symlinked manifest can't relocate that + * pin out from under consent. An npm/https source has no persistent local + * bundle (resolved, hashed, and discarded per admission pass — sources.mjs) + * so there is nothing to anchor to: `null`. buildAdmittedExecutionAdapter + * (execution/admitted.mjs) then refuses a relative hook command outright + * for a `null` baseDir rather than guessing a cwd. An unreadable/vanished + * file source also resolves to `null` — the same honest refusal, not a + * silent fallback to process.cwd(). */ +function baseDirForSource(source) { + if (typeof source !== 'string' || !source + || source.startsWith('https://') || source.startsWith('http://') || source.startsWith('npm:')) { + return null; + } + try { + return path.dirname(fs.realpathSync(source)); + } catch { + return null; + } +} + /** Deterministic, key-sorted JSON — same stable-stringify shape used * elsewhere in this codebase (e.g. opencode.mjs's deepEqual) so two manifests * that differ only in key order or incidental whitespace hash identically. @@ -193,6 +218,47 @@ export async function bootstrapHostAdapters({ if (admitted.length) { const { applyAdmitted } = await import('./admitted.mjs'); applyAdmitted(admitted); + + // P2 (ADR-0031): an admitted manifest declaring both an execution block + // and host.capabilities.canRouteActivities gets its execution adapter + // derived and registered here, so `ak run` can route to it. Same + // guarded, non-fatal posture as the rest of bootstrap: one adapter's + // registration failure never blocks the others or the admission result. + const executionCandidates = admitted.filter((result) => ( + result.manifest?.execution && result.entry?.capabilities?.canRouteActivities === true + )); + if (executionCandidates.length) { + // name -> the cfg entry's own declared source, for F-1's baseDir + // derivation below (admitted results carry the validated manifest, not + // the raw cfg entry that named where it came from). + const sourceByName = new Map(entries.map((entry) => [entry?.name, entry?.source])); + try { + const { registerAdmittedExecution } = await import('../execution/admitted.mjs'); + for (const result of executionCandidates) { + // F-5 (ADR-0029 §2): a manifest that never declared the + // cli-subprocess driving surface gets no cli-subprocess execution + // adapter — refused with its own reason, before even attempting + // registration (buildAdmittedExecutionAdapter re-checks this too, + // defence-in-depth for any caller that bypasses this filter). + if (!result.manifest?.driving?.surfaces?.includes('cli-subprocess')) { + warnings.push({ + name: result.name, reason: 'surface-unsupported', + detail: `'${result.name}' declares an execution block but not driving.surfaces including 'cli-subprocess'`, + }); + continue; + } + try { + registerAdmittedExecution(result.manifest, { baseDir: baseDirForSource(sourceByName.get(result.name)) }); + } catch (error) { + warnings.push({ name: result.name, reason: error?.reason ?? 'execution-registration-failed', detail: error?.message ?? String(error) }); + } + } + } catch (error) { + for (const result of executionCandidates) { + warnings.push({ name: result.name, reason: 'execution-registration-failed', detail: error?.message ?? String(error) }); + } + } + } } return { active: true, admitted, warnings }; diff --git a/src/lib/adapters/admitted.mjs b/src/lib/adapters/admitted.mjs index 21838fe..5f42e25 100644 --- a/src/lib/adapters/admitted.mjs +++ b/src/lib/adapters/admitted.mjs @@ -62,3 +62,15 @@ export function effectiveHostRegistry() { if (!applied || admittedEntries.length === 0) return HOST_REGISTRY; return Object.freeze([...HOST_REGISTRY, ...admittedEntries]); } + +/** Built-ins ∪ admitted hosts whose manifest declares + * capabilities.canRouteActivities — the LAZY set every routing VALIDATION + * path (routing.mjs's isRoutableHost, validateRoute, materializeRunPlan) + * must consult (P2, ADR-0031). routing.mjs's `HOSTS` constant stays frozen + * at import time and built-ins-only — it is display strings only now, never + * a validation source. Fresh on every call, like admittedHostIds() above. */ +export function effectiveRoutableHostIds() { + return effectiveHostRegistry() + .filter((host) => host.capabilities.canRouteActivities === true) + .map((host) => host.id); +} diff --git a/src/lib/execution/adapters.mjs b/src/lib/execution/adapters.mjs index 27e9bb9..3afde5e 100644 --- a/src/lib/execution/adapters.mjs +++ b/src/lib/execution/adapters.mjs @@ -4,6 +4,7 @@ import { OPENCODE_EXECUTION_ADAPTER } from './opencode.mjs'; import { CLAUDE_EXECUTION_ADAPTER } from './claude.mjs'; import { CODEX_EXECUTION_ADAPTER } from './codex.mjs'; import { routableHostIds } from '../adapters/index.mjs'; +import { admittedExecutionAdapterFor } from './admitted.mjs'; export const EXECUTION_ADAPTERS = Object.freeze(new Map([ ['claude', CLAUDE_EXECUTION_ADAPTER], @@ -43,11 +44,10 @@ assertBuiltinAdaptersRoutable(); * single worker instead of failing an entire run. */ export function executionAdapterFor(hostId) { if (EXECUTION_ADAPTERS.has(hostId)) return EXECUTION_ADAPTERS.get(hostId); - // Wave 4 (adapter door): an admitted external host whose manifest declares - // driving.surfaces including 'cli-subprocess' MAY, in a later wave, get a - // constructed execution adapter here. This wave builds none: admitted - // hosts have no execution adapter yet, full stop, so they fall through to - // the same null return (and the runner's existing cli_unavailable - // degradation) as any other routable-but-unadapted host. - return null; + // P2 (ADR-0031): an admitted external host whose manifest declared an + // execution block gets its adapter derived and registered at bootstrap + // (admission.mjs) into execution/admitted.mjs's overlay. A routable host + // with no execution block (or nothing admitted at all) still returns null + // here — the runner's existing cli_unavailable degradation, unchanged. + return admittedExecutionAdapterFor(hostId); } diff --git a/src/lib/execution/admitted.mjs b/src/lib/execution/admitted.mjs new file mode 100644 index 0000000..2a3ba8c --- /dev/null +++ b/src/lib/execution/admitted.mjs @@ -0,0 +1,362 @@ +// Derived execution adapter for an ADMITTED external host (P2, ADR-0031). +// This is the ONLY place an admitted manifest's execution.run.hook ever runs: +// a single-shot supervised subprocess through hook-runner.mjs — no in-process +// third-party code, ever. Deliberately simpler than opencode.mjs: one spawn, +// no server lifecycle, no streaming — launch() completes when the child exits. +import path from 'node:path'; +import { runAdapterHook } from '../adapters/hook-runner.mjs'; +import { resetAdmitted } from '../adapters/admitted.mjs'; +import { have } from '../exec.mjs'; +import { validateExecutionAdapter, validateWorkerResult } from './schema.mjs'; +import { redactHandoffData } from './handoff.mjs'; + +const DEFAULT_TIMEOUT_MS = 120_000; +// hook-runner's own inner timeout is what actually kills the child; the +// runner's outer phase deadline must never race it. Both timers would be +// registered for the same duration if we handed hook-runner the raw +// remaining budget, but hook-runner's timer starts a beat later (spawn +// overhead) — so it would lose that race. Shaving a small margin off what we +// hand the hook keeps the inner kill strictly first. +const LAUNCH_TIMEOUT_MARGIN_MS = 250; +const REASON_MAX_BYTES = 240; +const PROVIDER_MAX_CHARS = 64; +// Wave B security review (F-6): reserved hook exit codes give an admitted +// host an honest consent/auth boundary instead of a silent re-run — part of +// the adapter contract, alongside stdin/env/exit-0 in the module header. +// 77 -> the hook needs interactive/out-of-band consent it does not have +// (mirrors sysexits.h EX_NOPERM in spirit): status 'blocked', +// exitCategory 'permission_required' — already non-escalating +// (runner.mjs's BLOCKING_CATEGORIES). +// 78 -> the hook needs authentication it does not have: status 'failed', +// exitCategory 'auth_required'. +const EXIT_PERMISSION_REQUIRED = 77; +const EXIT_AUTH_REQUIRED = 78; + +const nowIso = () => new Date().toISOString(); + +function boundedReason(text, maxBytes = REASON_MAX_BYTES) { + const value = typeof text === 'string' ? text : String(text ?? ''); + return value.length > maxBytes ? `${value.slice(0, maxBytes - 1)}…` : value; +} + +function execError(reason, message) { + return Object.assign(new Error(message), { reason }); +} + +// F-1: a relative hook command resolves against whatever `cwd` the child +// spawns with. With no anchored adapter base directory (a remote npm/https +// source has no persistent local bundle), that would fall back to the +// OPERATOR's process.cwd() when `ak run` was invoked — arbitrary-code- +// execution by planting a same-named file, with the consent hash unchanged. +// A bare interpreter/binary name found through PATH (node, hermes) is +// unaffected by cwd and stays legal; only a path-separator-bearing or +// script-looking bare token is refused. +const SCRIPT_LIKE_RE = /\.(?:mjs|cjs|js|ts|py|rb|sh|pl)$/i; + +function looksRelative(token) { + if (typeof token !== 'string' || !token) return false; + if (path.isAbsolute(token)) return false; + if (token.includes('/') || token.includes('\\')) return true; + return SCRIPT_LIKE_RE.test(token); +} + +// R-3: EVERY arg is inspected, not just non-flag ones — a `--flag=value` +// token starts with '-' but its value half can still be a relative path +// (`--import=./evil.mjs`), and looksRelative's own `.includes('/')` check +// already catches that once the whole token is examined (no separate '=' +// split needed: the token as a whole still contains '/'). Skipping +// '-'-prefixed args entirely (the pre-R-3 shape) let exactly this slip past +// with a null baseDir. A bare flag with no path-like value (`--verbose`, +// `-v`) is unaffected — it matches neither check and stays legal. +function commandIsUnanchorable(command) { + const [argv0, ...args] = command; + if (looksRelative(argv0)) return true; + return args.some((arg) => looksRelative(arg)); +} + +/** stdout is EITHER a JSON object with optional {summary, observedModel, + * provider, usage} — read when the whole trimmed `stdoutText` (R-1: the + * UNMERGED stdout hook-runner reports; never `stdout`, which folds stderr + * in after a separator and would break JSON.parse the instant the hook + * writes anything to stderr at all) parses as a JSON object — OR plain + * text, treated as the summary capture (ADR-0029 §2). + * F-4: `stderrText` gates ONLY the plain-text path — a hook that wrote to + * stderr (warnings, a crash trace, interpreter noise) never has that text + * silently promoted into the cross-vendor dependency handoff. A genuine + * JSON payload parses and is trusted REGARDLESS of stderr (R-1: a stray + * deprecation warning must not blank out an otherwise-valid summary and + * cascade into "required worker handoff was missing" for every dependent). + * F-7: a self-declared `provider` is bounded (a payload cannot claim an + * unbounded-length vendor name). */ +function parseStdout(stdoutText, stderrText) { + const trimmed = typeof stdoutText === 'string' ? stdoutText.trim() : ''; + const hasStderr = typeof stderrText === 'string' && stderrText.trim() !== ''; + if (!trimmed) return { summary: null, observedModel: null, provider: null, usage: null }; + let parsed; + try { parsed = JSON.parse(trimmed); } catch { parsed = undefined; } + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + return { + summary: typeof parsed.summary === 'string' ? parsed.summary : null, + observedModel: typeof parsed.observedModel === 'string' ? parsed.observedModel : null, + provider: typeof parsed.provider === 'string' ? parsed.provider.slice(0, PROVIDER_MAX_CHARS) : null, + usage: parsed.usage && typeof parsed.usage === 'object' && !Array.isArray(parsed.usage) ? parsed.usage : null, + }; + } + if (hasStderr) return { summary: null, observedModel: null, provider: null, usage: null }; + return { summary: trimmed, observedModel: null, provider: null, usage: null }; +} + +/** + * Build a host-neutral execution adapter for one admitted manifest declaring + * `execution.run.hook`. `runHook`/`haveFn`/`clock` are injectable for tests; + * production defaults spawn the real subprocess. `baseDir` (F-1) is the + * adapter's own directory — derived by the caller (admission.mjs) from the + * manifest's `source` at registration time, `null` for a source with no + * persistent local bundle (npm/https) — never process.cwd(). + */ +export function buildAdmittedExecutionAdapter(manifest, { + runHook = runAdapterHook, haveFn = have, clock = nowIso, baseDir = null, +} = {}) { + if (!manifest || typeof manifest !== 'object') throw new TypeError('buildAdmittedExecutionAdapter requires a manifest'); + const hostId = manifest.host?.id; + if (typeof hostId !== 'string' || !hostId) throw new TypeError('buildAdmittedExecutionAdapter requires manifest.host.id'); + // F-8 (defence-in-depth, INEXPRESSIBLE-not-refused doctrine): the manifest + // schema already couples execution -> canRouteActivities (manifest.mjs's + // 'execution-not-routable'), but this is the construction site that + // actually wires a subprocess spawn — it re-asserts the invariant itself + // rather than trusting every caller to have validated upstream. + if (manifest.host?.capabilities?.canRouteActivities !== true) { + throw execError('not-routable', `'${hostId}' execution adapter requires host.capabilities.canRouteActivities:true`); + } + // F-5 (ADR-0029 §2): a manifest that never declared the cli-subprocess + // driving surface gets no cli-subprocess execution adapter — refused, not + // silently downgraded. Re-checked here even though the bootstrap filter + // (admission.mjs) already screens candidates, so no other caller can skip it. + if (!Array.isArray(manifest.driving?.surfaces) || !manifest.driving.surfaces.includes('cli-subprocess')) { + throw execError('surface-unsupported', `'${hostId}' execution adapter requires driving.surfaces to include 'cli-subprocess'`); + } + const hook = manifest.execution?.run?.hook; + if (!hook || !Array.isArray(hook.command) || hook.command.length === 0) { + throw new TypeError(`buildAdmittedExecutionAdapter requires manifest.execution.run.hook for '${hostId}'`); + } + const detectionBin = manifest.detection?.bin; + if (typeof detectionBin !== 'string' || !detectionBin) { + throw new TypeError(`buildAdmittedExecutionAdapter requires manifest.detection.bin for '${hostId}'`); + } + if (baseDir == null && commandIsUnanchorable(hook.command)) { + throw execError('execution-unanchored', + `'${hostId}' declares a relative execution.run.hook.command with no anchored adapter base directory ` + + '(a remote npm/https source has no persistent local bundle) — use an absolute path or a PATH binary'); + } + + function terminalResult(state, base) { + return validateWorkerResult({ + workerId: state.worker.id, activity: state.worker.activity, role: state.worker.role, host: hostId, + startedAt: state.startedAt, endedAt: clock(), + durationMs: Math.max(0, Date.parse(clock()) - Date.parse(state.startedAt)), + provider: null, providerProvenance: 'unknown', configuredModel: state.worker.configuredModel ?? null, + observedModel: null, sessionId: null, transcriptRefs: [], failure: null, usage: null, + ...base, + }); + } + + const adapter = { + id: `${hostId}-adapter`, + + async readiness({ signal, timeoutMs } = /** @type {{signal?:AbortSignal,timeoutMs?:number}} */ ({})) { + signal?.throwIfAborted?.(); + const ready = await haveFn(detectionBin, { signal, timeout: timeoutMs }); + signal?.throwIfAborted?.(); + return ready ? { ready: true } : { ready: false, exitCategory: 'cli_unavailable' }; + }, + + async prepare({ worker, cwd = process.cwd() } = /** @type {{worker?:any,cwd?:string}} */ ({})) { + if (!worker || worker.host !== hostId) throw new TypeError(`${hostId} adapter requires a ${hostId} worker`); + // R-2: state.cwd doubles as the fallback spawn cwd (below) AND rides + // in AK_WORKER_CWD — runAdapterHook itself throws on a non-absolute + // cwd, so this assertion (mirroring opencode.mjs's own worker-cwd + // check) is what makes that downstream guarantee hold, not an + // incidental duplicate of it. + if (!path.isAbsolute(cwd)) throw new TypeError(`${hostId} worker cwd must be absolute`); + return { worker, cwd, prompt: worker.prompt, startedAt: clock() }; + }, + + async launch(state, { timeoutMs, signal } = /** @type {{timeoutMs?:number,signal?:AbortSignal}} */ ({})) { + signal?.throwIfAborted?.(); + const budget = Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : DEFAULT_TIMEOUT_MS; + const innerTimeoutMs = Math.max(1, budget - LAUNCH_TIMEOUT_MARGIN_MS); + const env = { + AK_WORKER_ID: state.worker.id, + AK_WORKER_ACTIVITY: state.worker.activity, + AK_WORKER_ROLE: state.worker.role, + AK_WORKER_MODEL: state.worker.configuredModel ?? '', + // R-2: F-1's cwd pin (below) took away the hook's only implicit + // channel for learning which repo it's working on — the spawn cwd is + // now the adapter's own baseDir, not the caller's. Told explicitly + // instead, so a hook that needs the target repo can still find it + // without reopening F-1 by spawning there. + AK_WORKER_CWD: state.cwd, + }; + state.hookResult = await runHook({ + hook, hostId, verb: 'run', timeoutMs: innerTimeoutMs, env, stdin: state.prompt, + // R-2: the spawn cwd is uniform and explicit, never Node's own + // "inherit ak's process.cwd()" default (runAdapterHook's own + // fallback for an omitted cwd) — baseDir anchors a relative script + // to the adapter's own directory when one was declared (F-1); + // otherwise the construction-time check above already proved the + // command has no relative component that a cwd could redirect, so + // falling back to the repo cwd (state.cwd) here is safe (only bare + // PATH binaries reach this branch) and gives the hook a normal + // "run me from the target repo" default. + cwd: baseDir ?? state.cwd, + }); + return state; + }, + + async observe(state) { + const result = state.hookResult; + if (!result) throw new Error(`${hostId} adapter observed before launch completed`); + return { + type: 'exit', ok: result.ok, stdout: result.stdout, + // R-1: stdoutText is the UNMERGED stdout — see parseStdout's header + // comment for why the payload must never be parsed from `stdout`. + stdoutText: result.stdoutText ?? '', + exitCode: result.exitCode, detail: result.detail, + stderrText: result.stderrText ?? result.stderr ?? '', + }; + }, + + interpret(state, observation) { + // Runner-injected terminal events (outer phase deadline aborting + // launch/observe before our own {type:'exit'} observation lands, or an + // unexpected cancel/cleanup failure). See LAUNCH_TIMEOUT_MARGIN_MS above + // for why the 'exit' path is the expected one in practice. + if (observation?.type === 'timeout') { + return terminalResult(state, { status: 'timed_out', exitCategory: 'timeout', failure: { reason: boundedReason(observation.reason ?? 'timeout') } }); + } + if (observation?.type === 'orphaned') { + return terminalResult(state, { status: 'failed', exitCategory: 'orphaned', failure: { reason: 'admitted host subprocess did not terminate' } }); + } + if (observation?.type !== 'exit') { + return terminalResult(state, { status: 'failed', exitCategory: 'protocol_error', failure: { reason: 'unrecognized observation from admitted host adapter' } }); + } + + const { + ok, exitCode, stdout, stdoutText, detail, stderrText, + } = observation; + if (ok && exitCode === 0) { + const parsed = parseStdout(stdoutText, stderrText); + return terminalResult(state, { + status: 'succeeded', exitCategory: 'success', failure: null, + observedModel: parsed.observedModel, + provider: parsed.provider, + // F-7: a payload's self-declared provider is NEVER 'observed' — the + // hook asserted it, ak didn't verify it against anything. + providerProvenance: parsed.provider ? 'inferred' : 'unknown', + usage: parsed.usage, + }); + } + // F-6: reserved hook exit codes (see module header) — checked ahead of + // the generic non-zero-exit fallback below. + if (exitCode === EXIT_PERMISSION_REQUIRED) { + return terminalResult(state, { + status: 'blocked', exitCategory: 'permission_required', + failure: { reason: boundedReason(detail ?? 'adapter hook requires consent it does not have (exit 77)') }, + }); + } + if (exitCode === EXIT_AUTH_REQUIRED) { + return terminalResult(state, { + status: 'failed', exitCategory: 'auth_required', + failure: { reason: boundedReason(detail ?? 'adapter hook requires authentication it does not have (exit 78)') }, + }); + } + if (exitCode === null && typeof detail === 'string' && /timed out/i.test(detail)) { + return terminalResult(state, { status: 'timed_out', exitCategory: 'timeout', failure: { reason: boundedReason(detail) } }); + } + if (exitCode === null) { + return terminalResult(state, { status: 'failed', exitCategory: 'cli_unavailable', failure: { reason: boundedReason(detail ?? 'adapter hook failed to start') } }); + } + // F-3: the stdout tail folded into a worker_error reason is untrusted + // hook output — it may itself contain a `` block (or + // other private-protocol payload) that must never leak through a + // public WorkerResult's failure.reason. Redact before bounding, same + // as subprocess.mjs's failureFor. + const tail = typeof stdout === 'string' ? stdout.trim().slice(-200) : ''; + const raw = tail ? `${detail} — ${tail}` : (detail ?? `adapter hook exited with code ${exitCode}`); + return terminalResult(state, { status: 'failed', exitCategory: 'worker_error', failure: { reason: boundedReason(redactHandoffData(raw)) } }); + }, + + // The hook script's own protocol is simpler than the tagged-block one + // LLM-driven built-in hosts use: a JSON payload's `summary` field (or + // plain-text stdout, when stderr is empty) IS the outcome, verbatim — no + // `` parsing. Wrap it into normalizeHandoff's exact + // accepted shape rather than returning the bare `{summary}` it was read + // from. + summarize(_state, observation) { + if (observation?.type !== 'exit' || !observation.ok || observation.exitCode !== 0) return null; + const { summary } = parseStdout(observation.stdoutText, observation.stderrText); + if (!summary) return null; + return { outcome: summary, artifacts: [], decisions: [], risks: [] }; + }, + + // hook-runner owns the kill (group SIGKILL on its own inner timeout); + // launch() only resolves once the subprocess has already exited, so by + // the time cancel/cleanup can run there is normally no live resource + // left to terminate. The one exception (F-2): if the runner's OUTER + // phase deadline fires WHILE launch() is still pending — state.hookResult + // never got set — the subprocess may still be alive and its termination + // is unproven. Reporting plain 'cancelled' there would let the runner's + // escalation ladder treat this as an ordinary escalatable timed_out and + // fire a SECOND attempt against a worker that might still be running. + // 'orphaned' is honest (uncertain, non-escalating) and matches + // runner.mjs's own BLOCKING_CATEGORIES. + async cancel(state) { + if (!state?.hookResult) return { type: 'cancelled', orphaned: true }; + return { type: 'cancelled' }; + }, + async cleanup() { return { cleaned: true }; }, + }; + + return validateExecutionAdapter(adapter); +} + +// ── overlay registry (mirrors adapters/admitted.mjs's discipline) ────────── +// Module-level Map, not the built-in EXECUTION_ADAPTERS Map: an admitted +// external host's adapter is DERIVED from its manifest at bootstrap time, not +// hand-authored in-tree. A second register for the same host id replaces the +// first (re-admission, or a test re-registering); production populates this +// only from bootstrapHostAdapters (admission.mjs). +let admittedExecutionAdapters = new Map(); + +/** Build and register an execution adapter for an admitted manifest. Returns + * the built adapter. Throws (uncaught) on a manifest with no execution + * block or a malformed one — callers (bootstrap) are expected to guard this + * per-adapter and treat a throw as a non-fatal warning, not a crash. */ +export function registerAdmittedExecution(manifest, options) { + const adapter = buildAdmittedExecutionAdapter(manifest, options); + admittedExecutionAdapters.set(manifest.host.id, adapter); + return adapter; +} + +/** Test-only reset back to the unregistered, built-ins-only state. */ +export function resetAdmittedExecution() { + admittedExecutionAdapters = new Map(); +} + +/** One admitted host's execution adapter, or null when none is registered — + * never throws, mirroring executionAdapterFor's degrade-one-worker posture. */ +export function admittedExecutionAdapterFor(hostId) { + return admittedExecutionAdapters.get(hostId) ?? null; +} + +// F-9: the host overlay (adapters/admitted.mjs) and this execution overlay +// are two independently-mutable module singletons that a caller could reset +// out of step (e.g. a test resetting only one), leaving the other stale — an +// admitted-but-execution-orphaned or execution-registered-but-unadmitted +// state neither overlay's own reset guards against alone. Pairing them here +// (rather than reaching into adapters/admitted.mjs to add an upward +// dependency on this module) keeps that file untouched. +export function resetAllAdmitted() { + resetAdmittedExecution(); + resetAdmitted(); +} diff --git a/src/lib/routing.mjs b/src/lib/routing.mjs index 2f95f8b..86efc2e 100644 --- a/src/lib/routing.mjs +++ b/src/lib/routing.mjs @@ -6,7 +6,9 @@ // (no I/O) so the projectors and defaults are unit-testable in isolation; the // writers/UX that consume it live in providers.mjs / the commands. import { vendorOf } from './qeCourt.mjs'; -import { routableHostIds, primaryHostIds, validateActivityHost } from './adapters/index.mjs'; +import { + routableHostIds, primaryHostIds, validateActivityHost, effectiveHostRegistry, effectiveRoutableHostIds, +} from './adapters/index.mjs'; // ── Vocabulary ─────────────────────────────────────────────────────────────── // Canonical development activities ak routes (ADR-0002). Array order = display order. @@ -21,8 +23,14 @@ export const AK_ORIGINATED = new Set(['packaging', 'release']); // Host → aqe/router provider type. OpenCode deliberately has no entry: its // execution provider is observed per worker and must never be inferred from the -// host or silently projected into AQE's separate provider vocabulary. +// host or silently projected into AQE's separate provider vocabulary. An +// admitted external host (P2, ADR-0031) gets no entry either, same reasoning. export const HOST_PROVIDER = { claude: 'claude-code', codex: 'codex' }; +// Frozen at import time — built-ins only. Display strings and built-in +// listings ONLY (formatModelHelp, model catalogs below): every VALIDATION +// path (isRoutableHost, validateRoute, materializeRunPlan) consults the lazy +// effectiveRoutableHostIds()/effectiveHostRegistry() instead, so an admitted +// external host routes without this constant ever needing to change. export const HOSTS = routableHostIds(); // Providers aqe's ProviderManager can construct — grounded in agentic-qe 3.13.1 @@ -234,9 +242,12 @@ export const AGENT_ACTIVITY_MAP = { // ── Policy resolution + projections (pure) ────────────────────────────────── -/** True when both frontier hosts appear in a route (a route's host is valid). */ +/** True when `host` is routable: a built-in, or an admitted external host + * whose manifest declared capabilities.canRouteActivities (P2, ADR-0031). + * Lazy — re-reads the effective registry on every call, so it reflects an + * overlay applied after this module first loaded. */ export function isRoutableHost(host) { - return HOSTS.includes(host); + return effectiveRoutableHostIds().includes(host); } /** Substitute a retired model for its replacement, recording what was swapped. @@ -576,11 +587,15 @@ export function materializeRunPlan(policy = {}, { template = 'feature', task = ' const nodes = RUN_TEMPLATES[template]; if (!nodes) throw new Error(`unknown template "${template}" (expected: ${RUN_TEMPLATE_NAMES.join(', ')})`); const routes = resolveRoutes(policy); + // Snapshot once per materialization (not per validateActivityHost call): an + // admitted overlay applied mid-call must not be able to make one worker's + // eligibility check see a different registry than another's in the same plan. + const hosts = effectiveHostRegistry(); return { template, workers: nodes.map((n) => { const r = routes[n.activity]; - const eligibility = validateActivityHost(r.host); + const eligibility = validateActivityHost(r.host, hosts); if (!eligibility.ok) { throw new Error(`route for "${n.activity}" cannot materialize: host "${r.host}" requires canRouteActivities`); } @@ -591,7 +606,7 @@ export function materializeRunPlan(policy = {}, { template = 'feature', task = ' const ladder = (r.escalation ?? []) .filter((rung) => rung && (rung.host !== r.host || (rung.model ?? null) !== (r.model ?? null))) .map((rung) => { - const rungEligibility = validateActivityHost(rung.host); + const rungEligibility = validateActivityHost(rung.host, hosts); if (!rungEligibility.ok) { throw new Error(`escalation rung for "${n.activity}" cannot materialize: host "${rung.host}" requires canRouteActivities`); } @@ -647,7 +662,7 @@ export function routingSummary(policy = {}) { export function validateRoute(route = {}) { const { host, model } = route; const errs = []; - if (!isRoutableHost(host)) errs.push(`unknown host "${host}" (expected: ${HOSTS.join('|')})`); + if (!isRoutableHost(host)) errs.push(`unknown host "${host}" (expected: ${effectiveRoutableHostIds().join('|')})`); else if (HOST_PROVIDER[host] && !AQE_CONSTRUCTIBLE_PROVIDERS.includes(HOST_PROVIDER[host])) errs.push(`host "${host}" maps to a non-constructible provider`); if (model != null && (typeof model !== 'string' || model.trim() === '')) errs.push('model must be a non-empty string'); return errs; diff --git a/tests/fixtures/adapters/acme/manifest.json b/tests/fixtures/adapters/acme/manifest.json index d2919a6..98bd98b 100644 --- a/tests/fixtures/adapters/acme/manifest.json +++ b/tests/fixtures/adapters/acme/manifest.json @@ -9,7 +9,7 @@ "capabilities": { "canDriveSession": false, "canBePrimary": false, - "canRouteActivities": false, + "canRouteActivities": true, "commandStatusline": false, "transcripts": false, "usage": false, @@ -32,6 +32,11 @@ "hook": { "command": ["node", "detect-hook.mjs"], "timeoutMs": 5000 } } }, + "execution": { + "run": { + "hook": { "command": ["node", "run-hook.mjs"], "timeoutMs": 5000 } + } + }, "trust": { "changes": [ { diff --git a/tests/fixtures/adapters/acme/run-hook.mjs b/tests/fixtures/adapters/acme/run-hook.mjs new file mode 100644 index 0000000..31e0b83 --- /dev/null +++ b/tests/fixtures/adapters/acme/run-hook.mjs @@ -0,0 +1,28 @@ +// Fixture execution hook for the "acme" conformance adapter (P2, ADR-0031). +// A real, standalone subprocess — no dependencies, no network, no filesystem +// writes — invoked exactly as an admitted external adapter's declared +// execution.run.hook would be, through the real runAdapterHook. It reads the +// worker prompt from stdin (proving the stdin wiring) and echoes a JSON +// result whose `summary` names the AK_WORKER_* metadata env vars it received +// (proving the env wiring), matching the {summary, observedModel, provider} +// shape buildAdmittedExecutionAdapter (execution/admitted.mjs) parses. +// +// ACME_RUN_HOOK_FAIL=1 makes the hook fail deliberately, for a negative test +// of the worker_error mapping path. +let input = ''; +process.stdin.setEncoding('utf8'); +process.stdin.on('data', (chunk) => { input += chunk; }); +process.stdin.on('end', () => { + if (process.env.ACME_RUN_HOOK_FAIL === '1') { + process.stderr.write('acme fixture run hook: deliberate failure\n'); + process.exit(3); + return; + } + process.stdout.write(JSON.stringify({ + summary: `acme ran worker=${process.env.AK_WORKER_ID ?? ''} activity=${process.env.AK_WORKER_ACTIVITY ?? ''} ` + + `role=${process.env.AK_WORKER_ROLE ?? ''} model=${process.env.AK_WORKER_MODEL ?? ''} ` + + `promptBytes=${Buffer.byteLength(input, 'utf8')}`, + observedModel: process.env.AK_WORKER_MODEL || null, + provider: 'acme', + })); +}); diff --git a/tests/kit/adapter-conformance.test.mjs b/tests/kit/adapter-conformance.test.mjs index 9483396..c64b8b0 100644 --- a/tests/kit/adapter-conformance.test.mjs +++ b/tests/kit/adapter-conformance.test.mjs @@ -28,6 +28,8 @@ import { recordConsent, recordedHashFor, isTrusted, revokeConsent, } from '../../src/lib/adapters/consent.mjs'; import { registerAdmittedLifecycle } from '../../src/lib/adapters/lifecycle-registry.mjs'; +import { registerAdmittedExecution, resetAdmittedExecution } from '../../src/lib/execution/admitted.mjs'; +import { executeRunPlan } from '../../src/lib/execution/runner.mjs'; // Resolved relative to THIS file via fileURLToPath, never a hardcoded // repo-absolute or monorepo-sibling path (ruflo #2912 counter-example) — so @@ -42,28 +44,42 @@ const NEGATIVE_CORPUS = [ ]; /** - * The manifest contract has no path-resolution policy of its own yet - * (hook.command is just "a non-empty array of non-empty strings" — - * manifest.mjs never inspects the values). A real installed adapter would - * need SOME resolution step to turn a portable manifest's declared command + * The manifest contract has no path-resolution policy of its own for + * LIFECYCLE hooks (hook.command is just "a non-empty array of non-empty + * strings" — manifest.mjs never inspects the values, and lifecycle-registry.mjs + * spawns with no `cwd` anchoring). A real installed adapter would need SOME + * resolution step to turn a portable manifest's declared lifecycle command * into a locally-runnable one; that step doesn't exist in src/ today, so - * this is a minimal, test-owned stand-in: the literal token 'node' becomes - * process.execPath (never a bare PATH-searched 'node' — the same portability - * concern the hook-runner tests already document), and a relative *.mjs - * argument is resolved against the fixture's own directory (where the - * committed hook script actually lives), not the CWD. + * this remains a minimal, test-owned stand-in for lifecycle hooks ONLY: the + * literal token 'node' becomes process.execPath, and a relative *.mjs + * argument is resolved against the fixture's own directory. + * + * The EXECUTION hook (`execution.run.hook`) needs no such rewriting — Wave B + * security review (F-1) gave admission.mjs a real baseDir-derivation + + * cwd-anchoring path (registerAdmittedExecution -> buildAdmittedExecutionAdapter + * -> runAdapterHook's `cwd` option), so the fixture's literal, UNREWRITTEN + * `["node", "run-hook.mjs"]` resolves correctly through the real resolver: + * 'node' via PATH, 'run-hook.mjs' relative to the fixture's own directory + * (this manifest's `source` is a file path, so baseDir = FIXTURE_ROOT). A + * test-side rewrite here would prove a safer-than-production path, not the + * real one — see the negative-corpus/unanchored tests in adapter-execution.test.mjs + * for what happens when there is no baseDir to anchor to. */ +function resolveHookCommand(command, hookDir) { + return command.map((part) => { + if (part === 'node') return process.execPath; + if (part.endsWith('.mjs') && !path.isAbsolute(part)) return path.join(hookDir, part); + return part; + }); +} + function resolveManifestCommands(raw, hookDir) { if (!raw || typeof raw !== 'object' || !raw.lifecycle || typeof raw.lifecycle !== 'object') return raw; const lifecycle = {}; for (const [verb, entry] of Object.entries(raw.lifecycle)) { - if (!entry?.hook?.command) { lifecycle[verb] = entry; continue; } - const command = entry.hook.command.map((part) => { - if (part === 'node') return process.execPath; - if (part.endsWith('.mjs') && !path.isAbsolute(part)) return path.join(hookDir, part); - return part; - }); - lifecycle[verb] = { ...entry, hook: { ...entry.hook, command } }; + lifecycle[verb] = entry?.hook?.command + ? { ...entry, hook: { ...entry.hook, command: resolveHookCommand(entry.hook.command, hookDir) } } + : entry; } return { ...raw, lifecycle }; } @@ -151,6 +167,44 @@ export async function runConformanceReport({ fixtureRoot = FIXTURE_ROOT } = {}) assert.equal(typeof detected?.observed?.pid, 'number'); }); + await run("the fixture's declared execution.run hook drives a real one-worker plan end-to-end (P2, ADR-0031)", async () => { + if (!validated) throw new Error('prerequisite: manifest was not validated'); + // No runHook injection here either: registerAdmittedExecution's default + // dynamically imports the real hook-runner.mjs, and executeRunPlan's + // default adapter lookup (executionAdapterFor) falls through to the + // admitted overlay — proof the whole `ak run` path (materialized plan -> + // execution seam -> derived adapter -> hook runner -> spawned Node + // process -> stdout JSON -> WorkerResult) actually runs end-to-end. + // baseDir mirrors exactly what bootstrapHostAdapters derives in + // production (F-1) — this call bypasses that bootstrap wiring (it calls + // registerAdmittedExecution directly), so it must reproduce the same + // derivation rather than a test-only shortcut. + resetAdmittedExecution(); + try { + // haveFn only: the fixture's detection.bin ('acme') is a fictitious + // binary that will never actually be on a test machine's PATH — that's + // orthogonal to what this check proves (the execution.run hook itself + // running end-to-end), so readiness is stubbed the same way a real + // installed adapter's `acme` binary would report present. runHook + // stays the real default: a genuine spawned subprocess. + registerAdmittedExecution(validated, { + haveFn: async () => true, + baseDir: path.dirname(fs.realpathSync(validManifestPath)), + }); + const plan = { workers: [{ id: 'w1', activity: 'implementation', role: 'coder', host: 'acme', prompt: 'do the thing' }] }; + const [result] = await executeRunPlan(plan, { clock: () => new Date().toISOString() }); + assert.equal(result.status, 'succeeded', result.failure?.reason ?? 'expected a succeeded WorkerResult'); + assert.equal(result.host, 'acme'); + assert.equal(result.exitCategory, 'success'); + assert.equal(result.provider, 'acme'); + // F-7: a payload-declared provider is 'inferred', never 'observed' — + // ak did not verify the hook's claim against anything. + assert.equal(result.providerProvenance, 'inferred'); + } finally { + resetAdmittedExecution(); + } + }); + for (const [description, file, cfgName, reason] of NEGATIVE_CORPUS) { await run(`negative corpus: ${description} is refused with reason '${reason}'`, async () => { const results = await admitAdapters({ @@ -216,6 +270,10 @@ test("the fixture's declared detect hook runs as a real subprocess and its JSON assertCheck("the fixture's declared detect hook runs as a real subprocess and its JSON payload flows back"); }); +test("the fixture's declared execution.run hook drives a real one-worker plan end-to-end (P2, ADR-0031)", () => { + assertCheck("the fixture's declared execution.run hook drives a real one-worker plan end-to-end (P2, ADR-0031)"); +}); + for (const [description, , , reason] of NEGATIVE_CORPUS) { const name = `negative corpus: ${description} is refused with reason '${reason}'`; test(name, () => assertCheck(name)); @@ -227,8 +285,8 @@ test('edit-invalidation: mutating one byte of the manifest invalidates the prior test('runConformanceReport reports a clean pass with no failures', () => { assert.equal(report.failed, 0, JSON.stringify(report.checks.filter((c) => !c.ok), null, 2)); - assert.equal(report.total, 8); - assert.equal(report.passed, 8); + assert.equal(report.total, 9); + assert.equal(report.passed, 9); }); // ── GAP CLOSED (Wave 4 security remediation, P0-A): consent hashing used to diff --git a/tests/kit/adapter-execution.test.mjs b/tests/kit/adapter-execution.test.mjs new file mode 100644 index 0000000..92f804a --- /dev/null +++ b/tests/kit/adapter-execution.test.mjs @@ -0,0 +1,597 @@ +// P2 (ADR-0031) — the derived execution adapter for an admitted external +// host. Unit-level: every case here injects `runHook`/`haveFn`/`clock`, so +// none of it depends on a real subprocess or on B1's manifest/hook-runner +// landing timing. The real-subprocess, real-manifest black-box proof lives in +// adapter-conformance.test.mjs (the acme fixture's execution.run hook). +import { test, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { + buildAdmittedExecutionAdapter, registerAdmittedExecution, resetAdmittedExecution, admittedExecutionAdapterFor, + resetAllAdmitted, +} from '../../src/lib/execution/admitted.mjs'; +import { executionAdapterFor } from '../../src/lib/execution/adapters.mjs'; +import { validateExecutionAdapter } from '../../src/lib/execution/schema.mjs'; +import { normalizeHandoff, HANDOFF_START, HANDOFF_END } from '../../src/lib/execution/handoff.mjs'; +import { validateAdapterManifest } from '../../src/lib/adapters/manifest.mjs'; +import { applyAdmitted, effectiveHostRegistry } from '../../src/lib/adapters/admitted.mjs'; +import { bootstrapHostAdapters } from '../../src/lib/adapters/admission.mjs'; +import { isRoutableHost, validateRoute } from '../../src/lib/routing.mjs'; + +const clock = () => '2026-08-16T00:00:00.000Z'; + +function validHost(overrides = {}) { + return { + id: 'hermes', + label: 'Hermes', + install: { bin: 'hermes', externalInstallPolicy: 'detect-never-overwrite' }, + capabilities: { + canDriveSession: false, canBePrimary: false, canRouteActivities: true, + commandStatusline: false, transcripts: false, usage: false, + nativeMcpConfig: false, nativeGuidance: false, + }, + trust: { approvalPolicy: 'unchanged', changes: [] }, + enabledByDefault: false, + configProjection: 'ruflo', + observability: [], + ...overrides, + }; +} + +function hermesManifest(overrides = {}) { + return validateAdapterManifest({ + name: 'hermes', + version: '1.0.0', + contract: 1, + host: validHost(), + detection: { bin: 'hermes' }, + driving: { surfaces: ['cli-subprocess'] }, + execution: { run: { hook: { command: ['hermes-run'], timeoutMs: 30_000 } } }, + trust: { + changes: [{ + id: 'hermes-subprocess-hooks', kind: 'third-party-adapter', scope: 'project', + owner: 'hermes', value: 'subprocess hooks', effect: 'run consented hooks for hermes', + }], + }, + ...overrides, + }); +} + +const worker = (overrides = {}) => ({ + id: 'coder', activity: 'implementation', role: 'coder', host: 'hermes', + configuredModel: 'hermes-large', prompt: 'implement the thing', ...overrides, +}); + +// F-9: pairs both overlay resets so they can never desync between tests. +beforeEach(() => resetAllAdmitted()); + +// ── shape ──────────────────────────────────────────────────────────────── + +test('buildAdmittedExecutionAdapter returns a shape that passes validateExecutionAdapter', () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest()); + assert.equal(adapter.id, 'hermes-adapter'); + assert.doesNotThrow(() => validateExecutionAdapter(adapter)); +}); + +test('buildAdmittedExecutionAdapter requires an execution block and a detection.bin', () => { + const routableHost = { + id: 'x', capabilities: { canRouteActivities: true }, + }; + assert.throws(() => buildAdmittedExecutionAdapter({ + host: routableHost, driving: { surfaces: ['cli-subprocess'] }, + }), TypeError); + assert.throws(() => buildAdmittedExecutionAdapter({ + host: routableHost, driving: { surfaces: ['cli-subprocess'] }, execution: { run: { hook: { command: ['x'] } } }, + }), TypeError, 'missing detection.bin'); +}); + +// ── readiness ──────────────────────────────────────────────────────────── + +test('readiness reflects haveFn', async () => { + const ready = buildAdmittedExecutionAdapter(hermesManifest(), { haveFn: async () => true }); + assert.deepEqual(await ready.readiness({}), { ready: true }); + + const notReady = buildAdmittedExecutionAdapter(hermesManifest(), { haveFn: async () => false }); + assert.deepEqual(await notReady.readiness({}), { ready: false, exitCategory: 'cli_unavailable' }); +}); + +// ── prepare ────────────────────────────────────────────────────────────── + +test('prepare rejects a worker for a different host and a relative cwd', async () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { clock }); + await assert.rejects(() => adapter.prepare({ worker: worker({ host: 'codex' }), cwd: '/abs' }), TypeError); + await assert.rejects(() => adapter.prepare({ worker: worker(), cwd: 'relative/path' }), TypeError); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + assert.equal(state.prompt, 'implement the thing'); + assert.equal(state.startedAt, clock()); +}); + +// ── launch: stdin, env, and the min-wins timeout margin ───────────────── + +test('launch invokes runHook with stdin=prompt, AK_WORKER_* env (incl. AK_WORKER_CWD, R-2), and a margin below the phase budget', async () => { + const calls = []; + const runHook = async (options) => { calls.push(options); return { ok: true, stdout: 'done', stdoutText: 'done', exitCode: 0, detail: null }; }; + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { runHook, clock }); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + await adapter.launch(state, { timeoutMs: 3000 }); + + assert.equal(calls.length, 1); + const call = calls[0]; + assert.equal(call.hostId, 'hermes'); + assert.equal(call.verb, 'run'); + assert.equal(call.stdin, 'implement the thing'); + assert.deepEqual(call.env, { + AK_WORKER_ID: 'coder', AK_WORKER_ACTIVITY: 'implementation', AK_WORKER_ROLE: 'coder', AK_WORKER_MODEL: 'hermes-large', + AK_WORKER_CWD: '/abs', + }); + // Inner (hook-runner-facing) timeout must be strictly less than the phase + // budget, so hook-runner's own kill always fires first. + assert.ok(call.timeoutMs < 3000, 'inner timeout must undercut the phase budget'); + assert.ok(call.timeoutMs > 0); +}); + +test('launch sends an empty string AK_WORKER_MODEL when the worker has no configuredModel', async () => { + const calls = []; + const runHook = async (options) => { calls.push(options); return { ok: true, stdout: '', exitCode: 0, detail: null }; }; + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { runHook, clock }); + const state = await adapter.prepare({ worker: worker({ configuredModel: null }), cwd: '/abs' }); + await adapter.launch(state, { timeoutMs: 5000 }); + assert.equal(calls[0].env.AK_WORKER_MODEL, ''); +}); + +// ── observe / interpret: the full mapping matrix ───────────────────────── + +async function runToResult(hookResult, { worker: w = worker() } = {}) { + const runHook = async () => hookResult; + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { runHook, clock }); + const state = await adapter.prepare({ worker: w, cwd: '/abs' }); + await adapter.launch(state, { timeoutMs: 5000 }); + const observation = await adapter.observe(state); + return { adapter, state, observation, result: adapter.interpret(state, observation) }; +} + +test('exit 0 with a JSON payload maps summary/observedModel/provider/usage and providerProvenance=inferred (F-7)', async () => { + const stdout = JSON.stringify({ + summary: 'implemented the thing', observedModel: 'hermes-large-2', provider: 'hermes-vendor', usage: { tokens: 42 }, + }); + const { result, adapter, state, observation } = await runToResult({ + ok: true, stdout, stdoutText: stdout, stderrText: '', exitCode: 0, detail: null, + }); + assert.equal(result.status, 'succeeded'); + assert.equal(result.exitCategory, 'success'); + assert.equal(result.failure, null); + assert.equal(result.observedModel, 'hermes-large-2'); + assert.equal(result.provider, 'hermes-vendor'); + // F-7: a self-declared payload provider is 'inferred', NEVER 'observed' — + // ak did not verify the hook's claim against anything. + assert.equal(result.providerProvenance, 'inferred'); + assert.deepEqual(result.usage, { tokens: 42 }); + assert.equal(result.host, 'hermes'); + + const handoff = adapter.summarize(state, observation); + assert.deepEqual(handoff, { outcome: 'implemented the thing', artifacts: [], decisions: [], risks: [] }); + assert.doesNotThrow(() => normalizeHandoff(handoff), 'summarize output must be normalizeHandoff-compatible'); +}); + +test('F-7: a payload-declared provider is bounded to 64 chars', async () => { + const longProvider = 'x'.repeat(200); + const stdout = JSON.stringify({ summary: 'ok', provider: longProvider }); + const { result } = await runToResult({ ok: true, stdout, stdoutText: stdout, stderrText: '', exitCode: 0, detail: null }); + assert.equal(result.provider.length, 64); + assert.equal(result.provider, longProvider.slice(0, 64)); +}); + +test('exit 0 with plain-text stdout becomes the summary when stderr is empty', async () => { + const { result, adapter, state, observation } = await runToResult({ + ok: true, stdout: 'plain text result', stdoutText: 'plain text result', stderrText: '', exitCode: 0, detail: null, + }); + assert.equal(result.status, 'succeeded'); + assert.equal(result.provider, null); + assert.equal(result.providerProvenance, 'unknown'); + assert.equal(result.observedModel, null); + assert.equal(result.usage, null); + + const handoff = adapter.summarize(state, observation); + assert.deepEqual(handoff, { outcome: 'plain text result', artifacts: [], decisions: [], risks: [] }); +}); + +test('F-4: non-empty stderrText is never auto-promoted into a summary, even with plain-text stdout', async () => { + const { adapter, state, observation } = await runToResult({ + ok: true, stdout: 'looks like a normal result', stdoutText: 'looks like a normal result', + stderrText: 'a stray debug line', exitCode: 0, detail: null, + }); + assert.equal(adapter.summarize(state, observation), null, + 'stderr chatter must never become the cross-vendor dependency handoff'); +}); + +// R-1 (HIGH, blocker fix): the pre-R-1 shape parsed the JSON payload from +// hook-runner's MERGED `stdout` field — the instant a hook wrote anything to +// stderr (a Node/npm deprecation warning, nothing to do with the hook's own +// correctness), the merged text was no longer clean JSON, JSON.parse threw, +// and a perfectly valid summary/provider/usage was silently discarded. Worse +// downstream: any worker OTHERS depend on (`requireHandoff`) would then throw +// "required worker handoff was missing" and fail the whole pipeline over one +// stderr line. Fixed by parsing from `stdoutText` (hook-runner's UNMERGED +// stdout) instead — a valid JSON payload now parses regardless of stderr; +// only the PLAIN-TEXT promotion path stays gated on stderr being empty (F-4, +// unchanged, and still proven by the sibling test above). +test('R-1: a JSON payload alongside stderr chatter STILL yields its summary, provider, and usage', async () => { + const stdoutText = JSON.stringify({ summary: 'built the thing', provider: 'hermes-vendor', usage: { tokens: 7 } }); + const stdout = `${stdoutText}\n--- stderr ---\nnpm warn deprecated some-pkg@1.0.0`; + const { result, adapter, state, observation } = await runToResult({ + ok: true, stdout, stdoutText, stderrText: 'npm warn deprecated some-pkg@1.0.0', exitCode: 0, detail: null, + }); + assert.equal(result.status, 'succeeded'); + assert.equal(result.provider, 'hermes-vendor'); + assert.equal(result.providerProvenance, 'inferred'); + assert.deepEqual(result.usage, { tokens: 7 }); + + const handoff = adapter.summarize(state, observation); + assert.deepEqual(handoff, { outcome: 'built the thing', artifacts: [], decisions: [], risks: [] }); + assert.doesNotThrow(() => normalizeHandoff(handoff)); +}); + +test('exit 0 with empty stdout summarizes to null (no fabricated handoff)', async () => { + const { adapter, state, observation } = await runToResult({ + ok: true, stdout: '', stdoutText: '', stderrText: '', exitCode: 0, detail: null, + }); + assert.equal(adapter.summarize(state, observation), null); +}); + +// R-1 (pipeline-level): the scenario above must not just parse cleanly in +// isolation — it must not block a DEPENDENT worker either. Pre-R-1, the +// producer's missing handoff (mustSummarize, since a descendant depends on +// it) turned executeWorkerAttempt's requireHandoff check into a protocol +// error, blocking the producer AND its descendant over one stderr line. +test('R-1 (pipeline): a JSON summary alongside stderr flows to a dependent worker without blocking it', async () => { + const { executeRunPlan } = await import('../../src/lib/execution/runner.mjs'); + const stdoutText = JSON.stringify({ summary: 'built the thing', provider: 'hermes-vendor' }); + const stdout = `${stdoutText}\n--- stderr ---\nnpm warn deprecated some-pkg@1.0.0`; + const hookResult = { + ok: true, stdout, stdoutText, stderrText: 'npm warn deprecated some-pkg@1.0.0', exitCode: 0, detail: null, + }; + const runHook = async () => hookResult; + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { runHook, clock, haveFn: async () => true }); + const plan = { + workers: [ + { id: 'producer', activity: 'implementation', role: 'coder', host: 'hermes', prompt: 'produce' }, + { + id: 'consumer', activity: 'review', role: 'reviewer', host: 'hermes', prompt: 'consume', dependsOn: ['producer'], + }, + ], + }; + const results = await executeRunPlan(plan, { adapters: { hermes: adapter }, clock }); + const producer = results.find((r) => r.workerId === 'producer'); + const consumer = results.find((r) => r.workerId === 'consumer'); + assert.equal(producer.status, 'succeeded', producer.failure?.reason); + assert.equal(producer.provider, 'hermes-vendor'); + assert.equal(consumer.status, 'succeeded', consumer.failure?.reason); +}); + +test('non-zero exit maps to failed/worker_error with a bounded reason including the exit code', async () => { + const { result, adapter, state, observation } = await runToResult({ + ok: false, stdout: 'boom on line 1\n--- stderr ---\nstack trace tail', stderrText: 'stack trace tail', + exitCode: 1, detail: 'hermes:run adapter hook exited with code 1', + }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'worker_error'); + assert.match(result.failure.reason, /code 1/); + assert.ok(result.failure.reason.length <= 240); + assert.equal(adapter.summarize(state, observation), null, 'a failed worker never produces a handoff'); +}); + +test('F-3: a private handoff block embedded in the failing stdout tail is redacted from the worker_error reason', async () => { + const leaking = `some output ${HANDOFF_START}{"outcome":"secret","artifacts":[],"decisions":[],"risks":[]}${HANDOFF_END} more`; + const { result } = await runToResult({ + ok: false, stdout: leaking, stderrText: '', exitCode: 1, detail: 'hermes:run adapter hook exited with code 1', + }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'worker_error'); + assert.doesNotMatch(result.failure.reason, /secret/); + assert.doesNotMatch(result.failure.reason, /AK_HANDOFF_V1/); + assert.match(result.failure.reason, /private handoff withheld/); +}); + +// ── F-6: reserved hook exit codes (consent/auth boundary) ──────────────── + +test('F-6: exit code 77 maps to status blocked / exitCategory permission_required', async () => { + const { result } = await runToResult({ + ok: false, stdout: '', stderrText: '', exitCode: 77, detail: 'hermes:run adapter hook exited with code 77', + }); + assert.equal(result.status, 'blocked'); + assert.equal(result.exitCategory, 'permission_required'); +}); + +test('F-6: exit code 78 maps to status failed / exitCategory auth_required', async () => { + const { result } = await runToResult({ + ok: false, stdout: '', stderrText: '', exitCode: 78, detail: 'hermes:run adapter hook exited with code 78', + }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'auth_required'); +}); + +test('a spawn failure (ENOENT-style) maps to failed/cli_unavailable', async () => { + const { result } = await runToResult({ + ok: false, stdout: '', exitCode: null, detail: "hermes:run adapter hook failed to start: ENOENT (spawn hermes-run ENOENT)", + }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'cli_unavailable'); + assert.match(result.failure.reason, /ENOENT/); +}); + +test("hook-runner's own timeout detail maps to timed_out/timeout", async () => { + const { result } = await runToResult({ + ok: false, stdout: '', exitCode: null, detail: 'hermes:run adapter hook timed out after 5000ms and was killed', + }); + assert.equal(result.status, 'timed_out'); + assert.equal(result.exitCategory, 'timeout'); +}); + +// ── interpret: runner-injected terminal events (outer deadline abort) ──── + +test('interpret handles a runner-injected {type:"timeout"} terminal event', async () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { clock }); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + const result = adapter.interpret(state, { type: 'timeout', reason: 'launch exceeded the worker deadline' }); + assert.equal(result.status, 'timed_out'); + assert.equal(result.exitCategory, 'timeout'); +}); + +test('interpret handles a runner-injected {type:"orphaned"} terminal event', async () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { clock }); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + const result = adapter.interpret(state, { type: 'orphaned' }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'orphaned'); +}); + +test('interpret treats an unrecognized observation as a protocol error rather than fabricating success', async () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { clock }); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + const result = adapter.interpret(state, { type: 'idle' }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'protocol_error'); +}); + +// ── cancel / cleanup: honest post-launch no-ops, honest pre-launch orphan ─ + +test('cancel after launch has resolved is an honest no-op (hook-runner already owns the kill)', async () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { clock }); + const launchedState = { hookResult: { ok: true, stdout: '', stderrText: '', exitCode: 0, detail: null } }; + assert.deepEqual(await adapter.cancel(launchedState), { type: 'cancelled' }); + assert.deepEqual(await adapter.cleanup(launchedState), { cleaned: true }); +}); + +test('F-2: cancel BEFORE launch resolves reports orphaned (non-escalating), never a plain cancelled', async () => { + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { clock }); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + // state.hookResult was never set — launch() is (hypothetically) still + // pending when the runner's outer deadline fires and calls cancel(). + const cancelled = await adapter.cancel(state); + assert.deepEqual(cancelled, { type: 'cancelled', orphaned: true }); + + // The runner maps a cancel() reporting orphaned:true into + // interpret(state, {type:'orphaned'}) — prove OUR adapter's interpret + // honors that terminal shape (already covered generically for 'orphaned' + // above, but pinned here specifically as the F-2 consequence). + const result = adapter.interpret(state, { type: 'orphaned' }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'orphaned'); +}); + +// ── F-1: relative hook commands anchor to baseDir, or refuse outright ──── + +test('F-1: a relative hook command with no baseDir is refused at construction (execution-unanchored)', () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['run-hook.mjs'] } } } }); + assert.throws(() => buildAdmittedExecutionAdapter(manifest), (error) => { + assert.equal(error.reason, 'execution-unanchored'); + return true; + }); +}); + +test('F-1: a relative NON-flag argument with no baseDir is refused (argv0 itself may be a bare PATH binary)', () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['node', 'scripts/run.mjs'] } } } }); + assert.throws(() => buildAdmittedExecutionAdapter(manifest), (error) => { + assert.equal(error.reason, 'execution-unanchored'); + return true; + }); +}); + +test('F-1/R-3: a bare flag (and a non-path flag value) is never mistaken for a relative path', () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['hermes-run', '--config', 'x', '--verbose'] } } } }); + assert.doesNotThrow(() => buildAdmittedExecutionAdapter(manifest)); +}); + +// R-3 (residual of F-1): the pre-R-3 shape skipped every '-'-prefixed arg +// entirely before checking whether it looked like a path — so a single +// `--flag=value` token (still '-'-prefixed as a WHOLE token) slipped past +// unchecked even when its value half was a relative script. Now every arg is +// inspected, catching this with a null baseDir exactly like a bare relative +// argument would be. +test("R-3: a '--flag=value' token whose value is a relative path is refused with a null baseDir", () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['node', '--import=./evil.mjs', 'hermes-run'] } } } }); + assert.throws(() => buildAdmittedExecutionAdapter(manifest), (error) => { + assert.equal(error.reason, 'execution-unanchored'); + return true; + }); +}); + +test("R-3: the same '--flag=value' token is legal once a baseDir anchors the command", () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['node', '--import=./evil.mjs', 'hermes-run'] } } } }); + assert.doesNotThrow(() => buildAdmittedExecutionAdapter(manifest, { baseDir: '/adapters/hermes' })); +}); + +test('F-1: a bare PATH-resolved interpreter/binary command stays legal with no baseDir', () => { + const manifest = hermesManifest(); // command: ['hermes-run'] — no separator, no script extension + assert.doesNotThrow(() => buildAdmittedExecutionAdapter(manifest)); +}); + +test('F-1: an absolute command is always legal, baseDir or not', () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['/usr/bin/hermes-run', '/abs/arg.mjs'] } } } }); + assert.doesNotThrow(() => buildAdmittedExecutionAdapter(manifest)); +}); + +test('F-1: a relative command IS legal once a baseDir anchors it, and launch passes cwd=baseDir to runHook', async () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['node', 'run-hook.mjs'] } } } }); + const calls = []; + const runHook = async (options) => { calls.push(options); return { ok: true, stdout: '', stderrText: '', exitCode: 0, detail: null }; }; + const adapter = buildAdmittedExecutionAdapter(manifest, { runHook, clock, baseDir: '/adapters/hermes' }); + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + await adapter.launch(state, { timeoutMs: 5000 }); + assert.equal(calls[0].cwd, '/adapters/hermes'); +}); + +// R-2: the tempting fix for "the hook lost its cwd signal" would be spawning +// in the repo cwd unconditionally — but that reopens F-1 (a relative command +// would resolve against the operator's cwd again). Instead: baseDir anchors +// a relative command when one was declared; with NO baseDir, the +// construction-time check already proved the command has no relative +// component a cwd could redirect (bare PATH binaries only), so falling back +// to the repo cwd here is safe AND gives the hook a normal spawn location — +// never Node's own "inherit ak's own process.cwd()" default. +test('R-2: launch falls back to state.cwd (never omits cwd) when there is no baseDir', async () => { + const calls = []; + const runHook = async (options) => { calls.push(options); return { ok: true, stdout: '', stdoutText: '', stderrText: '', exitCode: 0, detail: null }; }; + const adapter = buildAdmittedExecutionAdapter(hermesManifest(), { runHook, clock }); // baseDir defaults to null + const state = await adapter.prepare({ worker: worker(), cwd: '/abs' }); + await adapter.launch(state, { timeoutMs: 5000 }); + assert.equal(calls[0].cwd, '/abs'); +}); + +// ── F-1 (bootstrap-level): baseDir derives from entry.source ───────────── + +test('F-1 (bootstrap): an npm-sourced execution adapter with a relative command is refused with a surfaced warning', async () => { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['run-hook.mjs'] } } } }); + const { hashManifest } = await import('../../src/lib/adapters/admission.mjs'); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: 'npm:hermes-adapter@1.0.0' }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: { recordedHashFor: () => hash, isTrusted: () => true }, + }); + assert.equal(result.admitted.length, 1, 'the host itself still admits — only execution registration fails'); + const warning = result.warnings.find((w) => w.reason === 'execution-unanchored'); + assert.ok(warning, `expected an 'execution-unanchored' warning; got ${JSON.stringify(result.warnings)}`); +}); + +test('F-1 (bootstrap): a file-sourced manifest derives baseDir from realpath(dirname(source)) and registers cleanly', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-execution-basedir-')); + try { + const manifest = hermesManifest({ execution: { run: { hook: { command: ['node', 'run-hook.mjs'] } } } }); + const { hashManifest } = await import('../../src/lib/adapters/admission.mjs'); + const hash = hashManifest(manifest); + const manifestPath = path.join(tmpDir, 'manifest.json'); + fs.writeFileSync(manifestPath, JSON.stringify(manifest)); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: manifestPath }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: { recordedHashFor: () => hash, isTrusted: () => true }, + }); + assert.equal(result.admitted.length, 1); + assert.deepEqual(result.warnings, [], `expected no warnings; got ${JSON.stringify(result.warnings)}`); + assert.notEqual(admittedExecutionAdapterFor('hermes'), null, 'registration must have succeeded with a real baseDir'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +}); + +// ── F-5: driving.surfaces must include 'cli-subprocess' ────────────────── + +test("F-5: a manifest whose driving.surfaces omits 'cli-subprocess' gets no execution adapter (surface-unsupported)", () => { + const manifest = hermesManifest({ driving: { surfaces: ['mcp'] } }); + assert.throws(() => buildAdmittedExecutionAdapter(manifest), (error) => { + assert.equal(error.reason, 'surface-unsupported'); + return true; + }); +}); + +test('F-5 (bootstrap): the execution-candidate filter refuses a non-cli-subprocess manifest with its own reason', async () => { + const manifest = hermesManifest({ driving: { surfaces: ['mcp'] } }); + const { hashManifest } = await import('../../src/lib/adapters/admission.mjs'); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name: 'hermes', source: '/does/not/matter/manifest.json' }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: { recordedHashFor: () => hash, isTrusted: () => true }, + }); + assert.equal(result.admitted.length, 1); + const warning = result.warnings.find((w) => w.reason === 'surface-unsupported'); + assert.ok(warning, `expected a 'surface-unsupported' warning; got ${JSON.stringify(result.warnings)}`); + assert.equal(admittedExecutionAdapterFor('hermes'), null); +}); + +// ── F-8: canRouteActivities is re-asserted at the construction site ────── + +test('F-8: a raw canRouteActivities:false host object is refused at construction (defence-in-depth)', () => { + const manifest = { + host: { id: 'hermes', capabilities: { canRouteActivities: false } }, + detection: { bin: 'hermes' }, + driving: { surfaces: ['cli-subprocess'] }, + execution: { run: { hook: { command: ['hermes-run'] } } }, + }; + assert.throws(() => buildAdmittedExecutionAdapter(manifest), (error) => { + assert.equal(error.reason, 'not-routable'); + return true; + }); +}); + +// ── F-9: paired overlay resets ──────────────────────────────────────────── + +test('F-9: resetAllAdmitted() clears both the host overlay and the execution overlay together', () => { + const manifest = hermesManifest({ name: 'acme', host: validHost({ id: 'acme' }) }); + applyAdmitted([{ entry: manifest.host }]); + registerAdmittedExecution(manifest); + assert.ok(effectiveHostRegistry().some((host) => host.id === 'acme')); + assert.notEqual(admittedExecutionAdapterFor('acme'), null); + + resetAllAdmitted(); + + assert.ok(!effectiveHostRegistry().some((host) => host.id === 'acme')); + assert.equal(admittedExecutionAdapterFor('acme'), null); +}); + +// ── registry: overlay discipline mirrors adapters/admitted.mjs ────────── + +test('registerAdmittedExecution/admittedExecutionAdapterFor/resetAdmittedExecution round-trip, and a second register replaces', () => { + assert.equal(admittedExecutionAdapterFor('hermes'), null); + const first = registerAdmittedExecution(hermesManifest()); + assert.equal(admittedExecutionAdapterFor('hermes'), first); + + const second = registerAdmittedExecution(hermesManifest({ version: '1.0.1' })); + assert.notEqual(second, first); + assert.equal(admittedExecutionAdapterFor('hermes'), second, 'a second register replaces, not accumulates'); + + resetAdmittedExecution(); + assert.equal(admittedExecutionAdapterFor('hermes'), null); +}); + +// ── seam: executionAdapterFor falls through to the admitted overlay ───── + +test('executionAdapterFor falls through to an admitted execution adapter for a non-built-in host', () => { + assert.equal(executionAdapterFor('hermes'), null); + const registered = registerAdmittedExecution(hermesManifest()); + assert.equal(executionAdapterFor('hermes'), registered); +}); + +// ── byte-zero pins (flag/overlay off vs on) ────────────────────────────── + +test('byte-zero: with nothing admitted, executionAdapterFor is null and validateRoute refuses the host', () => { + assert.equal(executionAdapterFor('acme'), null); + assert.equal(isRoutableHost('acme'), false); + assert.ok(validateRoute({ host: 'acme' }).length > 0); +}); + +test('with the host + execution overlay applied, executionAdapterFor resolves and validateRoute accepts', () => { + const manifest = hermesManifest({ name: 'acme', host: validHost({ id: 'acme' }) }); + applyAdmitted([{ entry: manifest.host }]); + registerAdmittedExecution(manifest); + + assert.equal(isRoutableHost('acme'), true); + assert.deepEqual(validateRoute({ host: 'acme' }), []); + assert.notEqual(executionAdapterFor('acme'), null); +}); From b85d662a1c12d3f551df27ee62aa3a1107dff873 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sun, 16 Aug 2026 10:03:59 -0700 Subject: [PATCH 3/3] docs(adr): ADR-0029 cli-subprocess execution contract; ADR-0031 external-execution row Working Records the command-resolution policy, reserved exit codes, and the no-trust-laundering rules settled while wiring external execution, plus the honest boundary of the remote-source anchorability screen. --- docs/adr/0029-host-adapter-extension-point.md | 40 ++++++++++++++++++- ...bility-graduation-and-upstream-requests.md | 4 +- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/docs/adr/0029-host-adapter-extension-point.md b/docs/adr/0029-host-adapter-extension-point.md index 1998718..36ac07a 100644 --- a/docs/adr/0029-host-adapter-extension-point.md +++ b/docs/adr/0029-host-adapter-extension-point.md @@ -141,7 +141,45 @@ names: An adapter manifest declaring `acp` or `mcp` today fails admission with an explicit "surface not yet supported" diagnostic. It is never silently downgraded to `cli-subprocess` — a silent downgrade would run a hook the adapter author never tested against that surface, exactly the kind of guessed -success ADR-0016 §5 and ADR-0023 already forbid elsewhere. +success ADR-0016 §5 and ADR-0023 already forbid elsewhere. An admitted host is given a +`cli-subprocess` **execution** adapter only when it declares that surface; a manifest missing it is +refused (`surface-unsupported`) rather than downgraded. + +**`cli-subprocess` execution details** (settled while wiring [ADR-0031](0031-capability-graduation-and-upstream-requests.md)'s +external-execution row, after an adversarial review of the surface): + +- **Command resolution is anchored, not ambient.** A hook subprocess runs with its working + directory pinned to the adapter's own resolved directory (a file-sourced manifest's `realpath` + directory), never `ak`'s current working directory — so a manifest declaring + `["node", "run-hook.mjs"]` runs *the adapter's* `run-hook.mjs`, and a file planted in the + operator's cwd is unreachable. A remote-sourced manifest (`npm:`/`https://`) has no persistent + local directory, so a *relative* hook command from such a source is refused + (`execution-unanchored`) rather than resolved against an ambient path; a bare PATH binary + (`node`, `hermes`) stays legal. The consent hash still pins the manifest text verbatim; the + resolution is a pure function of that text plus the (already-pinned) source, so it cannot drift + without the hash changing. + - *Boundary of the anchorability check for remote sources.* When a remote-sourced adapter has no + local directory to anchor to, its hook command spawns in the repository `ak run` was invoked in + (which the operator already runs at full trust, per ADR-0018), and the `execution-unanchored` + refusal is a **best-effort** screen for path-shaped tokens (separators, script extensions, flag + values), not a complete one: an *extensionless, separator-free* relative token + (`["node", "runhook"]`) is indistinguishable by inspection from an ordinary positional argument + (`["hermes-run", "build"]`), so it is not refused and would resolve against the repo. A complete + rule would have to reject every non-absolute, non-flag argument, which would also reject + legitimate positional arguments — a false-positive cost this contract does not pay by default. + The exposure is bounded on every axis that matters: it requires a remote (`npm:`/`https://`) + source, a consented manifest the operator hash-pinned with that exact relative token, and write + access to the operator's repo. A **file-sourced** adapter — the fixture, and every adapter that + ships a bundle — is fully anchored and unaffected. A remote-sourced adapter should declare + absolute paths or PATH binaries; a future contract revision may make that a hard requirement. +- **Reserved exit codes carry consent/auth boundaries.** Hook exit `77` maps to + `permission_required` (a blocked, never-escalated result — escalating around a consent boundary + is the safety violation ADR-0019 already forbids) and `78` to `auth_required`. This gives an + external host an honest way to say "I refused" or "I am not logged in" instead of a bare + non-zero exit that would be re-run on another host. +- **Results never launder trust.** Exit code is the sole authority for success; a self-declared + `provider` in hook stdout is stamped `inferred`, never `observed`; stderr is never promoted into + a downstream worker's prompt; and handoff data is redacted from public `WorkerResult`s. ### 3. Capability caps are schema-structural, not runtime-checked diff --git a/docs/adr/0031-capability-graduation-and-upstream-requests.md b/docs/adr/0031-capability-graduation-and-upstream-requests.md index 038d077..9472b77 100644 --- a/docs/adr/0031-capability-graduation-and-upstream-requests.md +++ b/docs/adr/0031-capability-graduation-and-upstream-requests.md @@ -149,11 +149,11 @@ Per the ADR discipline this repository adopted (a dated, self-graded table befor rests on delivery): the **governance decision** is accepted; the **machinery** is staged and mostly unbuilt. This table is the source of truth for what is real. -| Piece | Status (2026-08-16) | Note | +| Piece | Status | Note | | ----- | ------------------- | ---- | | Admission gate, consent store, hook runner, conformance kit (`admission` tier) | **Working** | ADR-0029, merged (PR #149) | | `ak host adapters trust` CLI (records consent/grants) | **Working** (2026-08-16, wave A) | `list`/`trust`/`revoke` + `--expect-hash` pinning; disclosure prints the full validated manifest (control-char-safe); mirrors every pre-hash admission refusal; `revoke` works with the flag off (fail-safe) | -| External execution (`ak run` drives an admitted host) | **Proposed — not built** | The seam is a comment-only lookup today | +| External execution (`ak run` drives an admitted host) | **Working** (2026-08-16, wave B) | Manifest `execution.run` hook (coupled to `canRouteActivities`, else refused `execution-not-routable`); derived subprocess adapter behind `executionAdapterFor`; routing is overlay-aware via a lazy `effectiveRoutableHostIds()`. Security-hardened (adversarial review): hooks spawn with `cwd` pinned to the adapter's own resolved directory (never the operator's cwd — a relative hook on a remote source is refused `execution-unanchored`); an unresolved-launch cancellation reports `orphaned` (non-escalating), never an escalatable `timed_out`; handoff data is redacted from public results; stderr is never promoted into a downstream prompt; reserved hook exit codes `77`/`78` express `permission_required`/`auth_required` boundaries; a self-declared `provider` is stamped `inferred`, never `observed` | | External lifecycle execution wired into setup/sync/uninstall | **Proposed — not built** | Loops are built-in-scoped by design until generalized | | Tiered conformance harness (`session-driving` … `statusline`) | **Proposed — not built** | Extends the single conformance kit | | Capability-grant store + promotion command | **Partial** (2026-08-16, wave A) | Data layer working (`grants.mjs`): hash-pinned, evidence-gated (grant-bearing tiers require non-empty evidence), edit-invalidated like consent; promotion command pending a later wave |