From 1b1ea80c07b44b4f3be0c818d311114981317b57 Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Sun, 16 Aug 2026 10:55:15 -0700 Subject: [PATCH 1/5] feat(lifecycle): external lifecycle execution in setup/sync/uninstall, shape-agnostic (ADR-0031 P3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lifecycle loops iterate hostsWithLifecycle() (built-ins + admitted) through a shared shape-dispatching renderer (opencode's rich per-surface shape and an admitted host's generic lifecycleResult both handled in one place), and the admission bootstrap now registers an admitted host's derived lifecycle adapter. An admitted host runs only when explicitly enabled in kit.json AND the experimental flag is set; built-in opencode is unchanged. Security-hardened (adversarial review): - admitted lifecycle hooks are cwd-anchored to the adapter's own resolved directory, per-verb 'lifecycle-unanchored' refusal for a relative hook on a remote source (the F-1 arbitrary-code-execution protection, now on the lifecycle path that P3 makes live) - rendered report lines strip C0/C1/DEL and clamp to one line, so a hostile hook cannot forge or erase a report line - the payload is parsed from stdout alone (stderr never collapses a result) - Windows executable extensions are treated as unanchorable bare tokens Also restores each opencode sub-surface's own failure level in the shared renderer (a failed plugin/agents/skill renders as a failure, not green) — fixing a sync-path regression the generalization would otherwise introduce. Known limitation (documented in ADR-0031): the sync path is wired but not yet reachable through a real `ak sync` — status.mjs's subsystem derivation is still opencode-scoped; setup and uninstall are fully live. --- src/commands/setup.mjs | 77 +++-- src/commands/sync.mjs | 45 ++- src/commands/uninstall.mjs | 55 +-- src/lib/adapters/admission.mjs | 53 ++- src/lib/adapters/lifecycle-registry.mjs | 236 +++++++++++-- src/lib/adapters/lifecycle-render.mjs | 200 +++++++++++ tests/kit/adapter-admission.test.mjs | 151 ++++++++- tests/kit/external-lifecycle.test.mjs | 427 ++++++++++++++++++++++++ tests/kit/lifecycle-registry.test.mjs | 285 +++++++++++++++- tests/kit/lifecycle-render.test.mjs | 156 +++++++++ tests/kit/uninstall-command.test.mjs | 17 + 11 files changed, 1585 insertions(+), 117 deletions(-) create mode 100644 src/lib/adapters/lifecycle-render.mjs create mode 100644 tests/kit/external-lifecycle.test.mjs create mode 100644 tests/kit/lifecycle-render.test.mjs diff --git a/src/commands/setup.mjs b/src/commands/setup.mjs index a285216..6f8a9e8 100644 --- a/src/commands/setup.mjs +++ b/src/commands/setup.mjs @@ -14,7 +14,8 @@ import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions } from '../lib/mcp.mjs'; import { reconcileOpencodeGuidance } from '../lib/opencode.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; -import { builtinHostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { hostsWithLifecycle, lifecycleAdapterFor, lifecycleExecutionEnabled, detectionBinFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { renderApplyReport } from '../lib/adapters/lifecycle-render.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; import { HOSTS, applyHosts, applyProviders, hostInstallState, installHost, applyAqeRouter, seedActivityRoutesIfMultiHost, printActivityRoutingTable, aqeSupportsAgentOverrides, ensureCodexMcp, ensureRufloMcpInCodex, applySetupHostFlags, bothHostsEnabled } from '../lib/providers.mjs'; import { installedVersion } from '../lib/versions.mjs'; @@ -29,6 +30,17 @@ import { import * as paths from '../lib/paths.mjs'; import { ok, warn, fail, info, heading, bold, dim, reportOutcome } from '../lib/output.mjs'; +/** Prints one lifecycle-render.mjs report line at its own level — 'fail' + * (F5, Wave C security review) reaches `fail()`, not a fallback `info()`, + * so a genuinely failed opencode sub-surface never reads as merely + * informational. */ +function printReportLine(line) { + if (line.level === 'ok') ok(line.text); + else if (line.level === 'warn') warn(line.text); + else if (line.level === 'fail') fail(line.text); + else info(line.text); +} + export const options = { 'dry-run': { type: 'boolean', default: false }, yes: { type: 'boolean', default: false }, @@ -215,21 +227,23 @@ export async function run_machine({ flags, pkgRoot, cfg }) { // 6b. host lifecycle wiring — config-file MCP + skills, lifecycle plugin, // converted agents, platform skill (each adapter owns its own surfaces - // — opencode.mjs for opencode). Registry-driven: loops - // builtinHostsWithLifecycle() rather than naming opencode, so a second - // BUILT-IN lifecycle host needs no new branch here. Only when the CLI - // is actually present: a declined/failed install must not leave a - // freshly-created config home behind (codex-review #4). The result - // SHAPE consumed below (stack.oc/plugin/agents/skill) is still - // opencode's own — the lifecycle contract doesn't mandate a common - // `apply()` result shape across hosts. builtinHostsWithLifecycle() - // (not hostsWithLifecycle()) deliberately excludes admitted external - // hosts: this loop body is opencode-shaped, and external lifecycle - // execution graduates in a later wave alongside a shape-agnostic body - // (see lifecycle-registry.mjs's registerAdmittedLifecycle comment). - for (const hostId of builtinHostsWithLifecycle()) { - if (!cfg.integrations?.hosts?.[hostId]) continue; - if (!(await have(hostId))) { + // — opencode.mjs for opencode; a subprocess hook for an admitted + // external — see lifecycle-registry.mjs's buildAdmittedLifecycleAdapter). + // Registry-driven: loops hostsWithLifecycle() (built-ins + admitted, + // ADR-0031 P3) rather than naming opencode, so a second lifecycle host + // — built-in or admitted — needs no new branch here. lifecycleExecutionEnabled + // gates each host: a built-in only needs cfg enablement (unchanged); an + // admitted external ALSO needs the experimental flag — an admitted host + // is opt-in exactly like opencode, and this never auto-enables anything. + // Only when the CLI is actually present: a declined/failed install must + // not leave a freshly-created config home behind (codex-review #4). + // lifecycle-render.mjs's renderApplyReport dispatches on the runLifecycle + // result's own shape (opencode's rich per-surface shape vs. an admitted + // host's generic lifecycleResult), so this loop body never destructures + // a host-specific result directly. + for (const hostId of hostsWithLifecycle()) { + if (!lifecycleExecutionEnabled(hostId, cfg)) continue; + if (!(await have(detectionBinFor(hostId)))) { const pkg = HOSTS.find((h) => h.id === hostId)?.pkg ?? hostId; warn(`${hostId}: enabled but CLI not installed — wiring skipped (re-run \`ak sync\` after installing ${pkg})`); continue; @@ -237,23 +251,22 @@ export async function run_machine({ flags, pkgRoot, cfg }) { const lifecycle = await runLifecycle({ adapter: lifecycleAdapterFor(hostId), action: 'apply', cfg, options: { pkgRoot }, }); - const stack = lifecycle.result; - (stack.oc.ok ? ok : warn)(`opencode: ${stack.oc.detail}`); - if (stack.oc.fatal) { - warn(`opencode plugin/agents/skill/guidance skipped — ${stack.oc.detail}`); - return false; + const report = renderApplyReport(hostId, lifecycle); + for (const line of report.lines) printReportLine(line); + if (report.fatal) return false; + // guidance blocks + the startup-reload note are opencode-specific surfaces + // (AGENTS.md blocks, opencode's own load-once-at-startup behavior) with no + // equivalent in the generic hook contract — stays gated on the rich shape. + if (report.shape === 'opencode') { + // guidance blocks for the opencode AGENTS.md land NOW (codex-review #18) + // — not on the next status-driven reconcile. Same shared reconcile pick + // and off use, so every command converges guidance identically. + const guidance = await reconcileOpencodeGuidance({ pkgRoot, cfg, cwd: process.cwd(), enabled: true }); + ok(`opencode guidance: ${guidance.detail.replace(/^guidance: /, '')}`); + // opencode loads config/plugins/MCP/agents once at startup — say so now, + // or the user files "hooks don't work" issues (observed live). + info('restart opencode to load the hooks + MCP servers (loaded once at startup)'); } - ok(`opencode plugin: ${stack.plugin.detail}`); - ok(`opencode agents: ${stack.agents.detail}`); - if (stack.skill.changed) ok(`opencode skill: ${stack.skill.detail}`); - // guidance blocks for the opencode AGENTS.md land NOW (codex-review #18) - // — not on the next status-driven reconcile. Same shared reconcile pick - // and off use, so every command converges guidance identically. - const guidance = await reconcileOpencodeGuidance({ pkgRoot, cfg, cwd: process.cwd(), enabled: true }); - ok(`opencode guidance: ${guidance.detail.replace(/^guidance: /, '')}`); - // opencode loads config/plugins/MCP/agents once at startup — say so now, - // or the user files "hooks don't work" issues (observed live). - info('restart opencode to load the hooks + MCP servers (loaded once at startup)'); } // 7. frontier host hint — codex detected but not enabled (opt-in via `ak host pick`) diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index 9f44e46..3160dfa 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -9,7 +9,8 @@ import { fixStatusline, helperStampStale } from '../lib/statusline.mjs'; import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions } from '../lib/mcp.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; -import { builtinHostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { hostsWithLifecycle, lifecycleAdapterFor, lifecycleExecutionEnabled, detectionBinFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { renderApplyReport } from '../lib/adapters/lifecycle-render.mjs'; import { listDaemons, staleDaemons, reap } from '../lib/daemons.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; import { commandHosts, applyHosts, applyProviders, hostInstallState, installHost, applyAqeRouter, seedActivityRoutesIfMultiHost, migrateRetiredRoutesInConfig, ensureCodexMcp, ensureRufloMcpInCodex, bothHostsEnabled } from '../lib/providers.mjs'; @@ -24,6 +25,17 @@ import * as paths from '../lib/paths.mjs'; import { ok, warn, fail, info, bold, dim, withProgress, reportOutcome } from '../lib/output.mjs'; import { applyCodexStatusline, projectionFor } from '../lib/codex-statusline.mjs'; +/** Prints one lifecycle-render.mjs report line at its own level — 'fail' + * (F5, Wave C security review) reaches `fail()`, not a fallback `info()`, + * so a genuinely failed opencode sub-surface never reads as merely + * informational. */ +function printReportLine(line) { + if (line.level === 'ok') ok(line.text); + else if (line.level === 'warn') warn(line.text); + else if (line.level === 'fail') fail(line.text); + else info(line.text); +} + export const options = { 'dry-run': { type: 'boolean', default: false }, 'no-upgrade': { type: 'boolean', default: false }, @@ -188,32 +200,29 @@ export async function run({ flags, pkgRoot, fetchLatest }) { // Runs BEFORE the blocks branch: the agents-opencode guidance target is gated // on the config home this branch creates — this order lets a fresh enable // converge guidance in the SAME sync (a second sync is then a true no-op). - // Registry-driven: loops builtinHostsWithLifecycle() rather than naming - // opencode, so a second BUILT-IN lifecycle host needs no new branch here. - // Only opencode is registered today, so this loop runs exactly once — - // byte-identical to the single-host branch it replaces. The result SHAPE - // consumed below (stack.oc/plugin/agents/skill) is still opencode's own — - // the lifecycle contract doesn't mandate a common `apply()` result shape - // across hosts. builtinHostsWithLifecycle() (not hostsWithLifecycle()) - // deliberately excludes admitted external hosts — see lifecycle-registry.mjs. - for (const hostId of builtinHostsWithLifecycle()) { - if (!subsystems.has(hostId) || !cfg.integrations?.hosts?.[hostId]) continue; - if (!(await have(hostId))) { + // Registry-driven: loops hostsWithLifecycle() (built-ins + admitted, + // ADR-0031 P3) rather than naming opencode, so a second lifecycle host — + // built-in or admitted — needs no new branch here. lifecycleExecutionEnabled + // gates each host exactly as setup.mjs does (built-in: cfg enablement only; + // admitted: cfg enablement AND the experimental flag — never auto-enabled). + // lifecycle-render.mjs's renderApplyReport dispatches on the runLifecycle + // result's own shape, so this loop body never destructures a host-specific + // result directly; opencode's per-surface lines render exactly as before. + for (const hostId of hostsWithLifecycle()) { + if (!subsystems.has(hostId) || !lifecycleExecutionEnabled(hostId, cfg)) continue; + if (!(await have(detectionBinFor(hostId)))) { info(`${hostId}: enabled but CLI not installed — wiring skipped (hosts step installs it)`); continue; } const lifecycle = await runLifecycle({ adapter: lifecycleAdapterFor(hostId), action: 'apply', cfg, options: { pkgRoot }, }); - const stack = lifecycle.result; + const applyReport = renderApplyReport(hostId, lifecycle); // persist the markers on ANY refresh (a converged file whose kit.json // markers are stale/missing still needs the save, or the next teardown // cannot prove ownership — codex-review r3), not only on file changes. - if (stack.oc.changed || stack.markersChanged) saveKitConfig(cfg); - if (stack.oc.changed || !stack.oc.ok) report('opencode', stack.oc); - report('opencode plugin', stack.plugin); - report('opencode agents', stack.agents); - if (stack.skill.changed || !stack.skill.ok) report('opencode skill', stack.skill); + if (applyReport.ocChanged || applyReport.markersChanged) saveKitConfig(cfg); + for (const line of applyReport.lines) printReportLine(line); } // The 'opencode' guard: the opencode branch above can CREATE the config home // that activates the agents-opencode guidance target — a machine whose other diff --git a/src/commands/uninstall.mjs b/src/commands/uninstall.mjs index 721e129..74ab88f 100644 --- a/src/commands/uninstall.mjs +++ b/src/commands/uninstall.mjs @@ -11,12 +11,25 @@ import { stripBlock, BEGIN, BUILTIN_BLOCKS } from '../lib/blocks.mjs'; import { unregister } from '../lib/mcp.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; -import { builtinHostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.mjs'; +import { hostsWithLifecycle, lifecycleAdapterFor, lifecycleExecutionEnabled, isBuiltinHost } from '../lib/adapters/lifecycle-registry.mjs'; +import { renderUndoReport } from '../lib/adapters/lifecycle-render.mjs'; import { present as rbPresent } from '../lib/ruvnet-brain.mjs'; import * as paths from '../lib/paths.mjs'; -import { ok, warn, info } from '../lib/output.mjs'; +import { ok, warn, fail, info } from '../lib/output.mjs'; import { removeCodexStatusline } from '../lib/codex-statusline.mjs'; +/** Prints one lifecycle-render.mjs report line at its own level — mirrors + * setup.mjs/sync.mjs's own printReportLine (N-2, Wave C security review + * follow-up): renderUndoReport only ever emits 'ok'/'warn' today, so this is + * latent, but the day an undo renderer adopts levelForResult (F5's mapping) + * a 'fail' line must reach fail(), not be silently downgraded to warn(). */ +export function printReportLine(line) { + if (line.level === 'ok') ok(line.text); + else if (line.level === 'warn') warn(line.text); + else if (line.level === 'fail') fail(line.text); + else info(line.text); +} + export const options = { 'dry-run': { type: 'boolean', default: false }, 'this-project': { type: 'boolean', default: false }, @@ -128,23 +141,31 @@ export async function run({ flags }) { // removes kit.json below, so persisting cfg here would recreate it. Each // adapter's own undo() already honors ownership/receipts (opencode's // undoOpencode no-ops when it never held mcp:'ak', and marker-gates - // artifact removal independent of that), so this call is unconditional per - // host — the only kit-side gate is "did anything actually happen", to - // avoid a no-op teardown line (and a needless kit.json rewrite) on a host - // that was never enabled. builtinHostsWithLifecycle() (not - // hostsWithLifecycle()) deliberately excludes admitted external hosts: - // this loop body destructures an opencode-shaped result (ret.undo, - // ret.artifacts) — see lifecycle-registry.mjs's registerAdmittedLifecycle - // comment for why external lifecycle execution isn't wired through here yet. - for (const hostId of builtinHostsWithLifecycle()) { + // artifact removal independent of that), so a BUILT-IN's call is + // unconditional per host, same as before ADR-0031 P3 — the only kit-side + // gate is "did anything actually happen", to avoid a no-op teardown line + // (and a needless kit.json rewrite) on a host that was never enabled. An + // ADMITTED external host is different: there is no "always safe, always + // idempotent" guarantee for an arbitrary third-party hook the way there is + // for opencode's own undo, so an admitted host's teardown is gated by + // lifecycleExecutionEnabled (cfg enablement AND the experimental flag) — + // an admitted host that was never enabled/consented for this run is never + // invoked. hostsWithLifecycle() (built-ins + admitted, ADR-0031 P3) is safe + // to loop unconditionally now: lifecycle-render.mjs's renderUndoReport + // dispatches on the runLifecycle result's own shape, so this loop body + // never destructures a host-specific result directly. + for (const hostId of hostsWithLifecycle()) { + if (!isBuiltinHost(hostId) && !lifecycleExecutionEnabled(hostId, cfg)) continue; const adapter = lifecycleAdapterFor(hostId); if (dry) { - info(`[dry-run] stripped ak-managed ${hostId} wiring + artifacts (opencode.json, plugin, agents, skill)`); + info(isBuiltinHost(hostId) + ? `[dry-run] stripped ak-managed ${hostId} wiring + artifacts (opencode.json, plugin, agents, skill)` + : `[dry-run] stripped ak-managed ${hostId} wiring + artifacts (hook-declared undo)`); continue; } const retired = await runLifecycle({ adapter, action: 'undo', cfg }); - const ret = retired.result; - ownershipTeardownOk = ownershipTeardownOk && ret.ok; + const undoReport = renderUndoReport(hostId, retired); + ownershipTeardownOk = ownershipTeardownOk && undoReport.ok; // Persist markers unconditionally, exactly like x/host.mjs's off()/pick(): // undo() mutates cfg's ownership markers in memory even when it rewrote // no file (`undo.changed` measures the FILE, not cfg), so gating the save @@ -152,11 +173,7 @@ export async function run({ flags }) { // quiet-success path. Only the human-facing line stays gated on "did // anything observable happen". if (!flags.purge) saveKitConfig(cfg); - if (ret.undo.changed || ret.artifacts.changed || !ret.ok) { - (ret.ok ? ok : warn)(ret.ok - ? `stripped ak-managed ${hostId} wiring + artifacts (opencode.json, plugin, agents, skill)` - : `${hostId} teardown incomplete — ${ret.undo.detail}`); - } + for (const line of undoReport.lines) printReportLine(line); } if (flags.purge && fs.existsSync(paths.kitConfigPath())) { if (ownershipTeardownOk) act('removed kit.json', () => fs.rmSync(paths.kitConfigPath())); diff --git a/src/lib/adapters/admission.mjs b/src/lib/adapters/admission.mjs index eba98a1..0702ab6 100644 --- a/src/lib/adapters/admission.mjs +++ b/src/lib/adapters/admission.mjs @@ -219,6 +219,14 @@ export async function bootstrapHostAdapters({ const { applyAdmitted } = await import('./admitted.mjs'); applyAdmitted(admitted); + // 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). Shared by both the + // execution- and lifecycle-registration blocks below — one map, not a + // second copy — so a caller correcting F-1 in one place can't drift from + // the other. + const sourceByName = new Map(entries.map((entry) => [entry?.name, entry?.source])); + // 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 @@ -228,10 +236,6 @@ export async function bootstrapHostAdapters({ 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) { @@ -259,6 +263,47 @@ export async function bootstrapHostAdapters({ } } } + + // P3 (ADR-0031): an admitted manifest declaring a lifecycle block gets its + // derived lifecycle adapter registered here, so it appears in + // hostsWithLifecycle() and setup/sync/uninstall's lifecycle loops can + // drive it (gated per-run by lifecycleExecutionEnabled — registration + // alone never runs a hook). Same guarded, non-fatal posture as the + // execution-registration block above: one adapter's registration failure + // never blocks the others or the admission result. F-1 (same as + // execution above): baseDir anchors a relative lifecycle hook command to + // the adapter's own directory — without it, a relative command would + // resolve against the OPERATOR's cwd, arbitrary-code-execution with the + // consent hash unchanged. Unlike execution (one hook, all-or-nothing), + // lifecycle has five independently-optional verbs, so an unanchorable + // one is refused per-verb (buildAdmittedLifecycleAdapter never wires it + // to spawn) rather than failing the whole registration — the other, + // anchored/PATH-binary verbs still register and work. + const lifecycleCandidates = admitted.filter((result) => !!result.manifest?.lifecycle); + if (lifecycleCandidates.length) { + try { + const { registerAdmittedLifecycle } = await import('./lifecycle-registry.mjs'); + for (const result of lifecycleCandidates) { + try { + const baseDir = baseDirForSource(sourceByName.get(result.name)); + const adapter = registerAdmittedLifecycle(result.manifest, { baseDir }); + if (adapter.unanchoredVerbs.length) { + warnings.push({ + name: result.name, reason: 'lifecycle-unanchored', + detail: `'${result.name}' lifecycle hook(s) refused (relative command, no anchored adapter ` + + `base directory): ${adapter.unanchoredVerbs.join(', ')}`, + }); + } + } catch (error) { + warnings.push({ name: result.name, reason: error?.reason ?? 'lifecycle-registration-failed', detail: error?.message ?? String(error) }); + } + } + } catch (error) { + for (const result of lifecycleCandidates) { + warnings.push({ name: result.name, reason: 'lifecycle-registration-failed', detail: error?.message ?? String(error) }); + } + } + } } return { active: true, admitted, warnings }; diff --git a/src/lib/adapters/lifecycle-registry.mjs b/src/lib/adapters/lifecycle-registry.mjs index 59b6188..ee0d6e0 100644 --- a/src/lib/adapters/lifecycle-registry.mjs +++ b/src/lib/adapters/lifecycle-registry.mjs @@ -12,12 +12,20 @@ // — so this is a new, one-way edge (adapters/* -> opencode.mjs) that never // cycles back (opencode.mjs has no reason to import this module: callers // reach it through lifecycleAdapterFor/hostsWithLifecycle instead). +import path from 'node:path'; import { validateLifecycleAdapter, LIFECYCLE_OPERATIONS, lifecycleResult } from './lifecycle.mjs'; import { HOST_REGISTRY } from './registries.mjs'; import { effectiveHostRegistry } from './admitted.mjs'; import { OPENCODE_LIFECYCLE_ADAPTER } from '../opencode.mjs'; const LIFECYCLE_ADAPTERS = new Map(); +// F7 (Wave C security review — prioritized for P4's paired-overlay-reset +// integration, mirroring execution/admitted.mjs's own resetAllAdmitted +// pairing): tracks which LIFECYCLE_ADAPTERS keys came from +// registerAdmittedLifecycle (never a built-in) so resetAdmittedLifecycle() +// below can clear exactly those without disturbing 'opencode' (or any other +// built-in registered via registerBuiltinLifecycle). +const ADMITTED_LIFECYCLE_IDS = new Set(); /** * Register a built-in host's lifecycle adapter. Internal — called by this @@ -68,24 +76,74 @@ export function hostsWithLifecycle() { /** * Host ids with a registered lifecycle adapter, restricted to BUILT-IN hosts * (LIFECYCLE_ADAPTERS keys intersected with HOST_REGISTRY — never an - * admitted external, even after applyAdmitted). setup.mjs/sync.mjs/ - * uninstall.mjs's lifecycle loops destructure an opencode-SHAPED result - * (stack.oc/.plugin/.agents/.skill, ret.undo/.artifacts) — those loop bodies - * are not generic across hosts yet. Today that's unreachable dead code, - * because registerAdmittedLifecycle is never called in production — but it - * is ARMED: the moment something wires an admitted external's lifecycle - * adapter in (registerAdmittedLifecycle exists for exactly that), a - * non-opencode-shaped host would enter hostsWithLifecycle() and crash one of - * those command loops on the first opencode-specific destructure. Command - * loops use this function instead until external lifecycle EXECUTION and a - * shape-agnostic loop body graduate together, in a later wave. - * hostsWithLifecycle() above stays for pure registry queries, unaffected. + * admitted external, even after applyAdmitted). Kept as a pure, built-ins- + * only registry query — e.g. for an install-hint lookup that only makes + * sense against HOSTS' own package metadata. + * + * setup.mjs/sync.mjs/uninstall.mjs no longer use this to pick their loop's + * iteration source (ADR-0031 P3): their lifecycle loops used to destructure + * an opencode-SHAPED result (stack.oc/.plugin/.agents/.skill, + * ret.undo/.artifacts) directly, which would have crashed the moment a + * non-opencode-shaped admitted host entered the loop. That crash risk is + * exactly what lifecycle-render.mjs's shape-dispatching renderer removes — + * the command loops now iterate hostsWithLifecycle() (built-ins + admitted) + * and render through renderApplyReport/renderUndoReport instead of raw + * destructuring, gated per-host by lifecycleExecutionEnabled() below. * @returns {string[]} */ export function builtinHostsWithLifecycle() { return HOST_REGISTRY.filter((host) => LIFECYCLE_ADAPTERS.has(host.id)).map((host) => host.id); } +/** + * True when hostId is one of HOST_REGISTRY's own entries (never an admitted + * external, even after applyAdmitted). Used by lifecycleExecutionEnabled and + * detectionBinFor to tell a built-in from an admitted host without a command + * reaching into registries.mjs directly. + * @param {string} hostId + * @returns {boolean} + */ +export function isBuiltinHost(hostId) { + return HOST_REGISTRY.some((host) => host.id === hostId); +} + +const EXPERIMENTAL_HOST_ADAPTERS_FLAG = 'AK_EXPERIMENTAL_HOST_ADAPTERS'; + +/** + * Whether setup/sync/uninstall should actually exercise hostId's lifecycle + * adapter this run (ADR-0031 P3). A BUILT-IN host is gated only by cfg's own + * enablement — unchanged from before this wave. An ADMITTED external host + * needs BOTH: explicit cfg enablement (opt-in exactly like opencode — there + * is no pick-UI for external hosts yet, so enablement is operator-set in + * kit.json) AND the experimental flag. Neither condition is ever inferred or + * set here — this function only reads, never auto-enables anything. + * @param {string} hostId + * @param {any} cfg + * @param {NodeJS.ProcessEnv} [env] + * @returns {boolean} + */ +export function lifecycleExecutionEnabled(hostId, cfg, env = process.env) { + if (!cfg?.integrations?.hosts?.[hostId]) return false; + if (isBuiltinHost(hostId)) return true; + return env?.[EXPERIMENTAL_HOST_ADAPTERS_FLAG] === '1'; +} + +/** + * The binary name a command loop should probe for hostId's CLI presence + * before exercising its lifecycle adapter (mirrors the built-in convention + * `have(hostId)` — a built-in's host id IS its binary name). An admitted + * host's binary name is whatever its manifest declared + * (manifest.detection.bin, always present on a validated manifest) — + * buildAdmittedLifecycleAdapter stashes it on the registered adapter as + * `.detectionBin` so this never needs a second store keyed by host id. + * @param {string} hostId + * @returns {string} + */ +export function detectionBinFor(hostId) { + if (isBuiltinHost(hostId)) return hostId; + return lifecycleAdapterFor(hostId)?.detectionBin ?? hostId; +} + // ── admitted (external) lifecycle adapters ───────────────────────────────── // A derived adapter never runs third-party code in-process: each declared // verb is a thin wrapper that shells out through the sibling's hook runner @@ -96,18 +154,93 @@ export function builtinHostsWithLifecycle() { // manifest never declared gets an honest no-op: it never ran, so nothing // changed. +// F4 (Wave C security review — the un-applied Wave B R-1 twin): `result.stdout` +// is the MERGED stdout+stderr (hook-runner.mjs's mergeCapture) — any stderr +// output (a deprecation warning, interpreter noise) breaks JSON.parse even +// when the hook fully succeeded, so a genuinely successful apply/undo would +// misreport as failed. `result.stdoutText` is the UNMERGED stdout only +// (hook-runner.mjs's boundedText(stdoutCaptured), the same field execution's +// own R-1 fix reads) — reading it here is the lifecycle-side twin of that +// fix, never applied to this file until now. function parseHookPayload(result) { - if (!result || typeof result.stdout !== 'string') return null; + if (!result || typeof result.stdoutText !== 'string') return null; try { - const parsed = JSON.parse(result.stdout); + const parsed = JSON.parse(result.stdoutText); return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null; } catch { return null; } } +// Minimal UTF-8-safe truncation (mirrors execution/handoff.mjs's own +// truncateUtf8 — that copy is canonical; not exported there, so this is a +// local twin) so a failure detail bounded from raw hook stdout can never +// promote an unbounded blob into lifecycle-render.mjs's print path (which +// feeds F3's own control-char stripping, but a huge string is still a +// separate flooding/DoS-adjacent concern worth bounding at the source). +function truncateUtf8(value, maxBytes) { + const bytes = (v) => Buffer.byteLength(v, 'utf8'); + if (bytes(value) <= maxBytes) return value; + if (maxBytes <= 3) return '.'.repeat(Math.max(0, maxBytes)); + let out = ''; + for (const char of value) { + if (bytes(`${out}${char}…`) > maxBytes) break; + out += char; + } + return `${out}…`; +} + function hookFailureResult(verb, result) { - const detail = result?.detail || (result?.stdout || '').trim() || `hook exited ${result?.exitCode ?? 'unknown'}`; + const detail = result?.detail + || truncateUtf8((result?.stdoutText || '').trim(), 240) + || `hook exited ${result?.exitCode ?? 'unknown'}`; + if (verb === 'detect' || verb === 'verify') return { observed: null, error: detail }; + if (verb === 'plan') return { changed: false, operations: [], error: detail }; + return lifecycleResult({ ok: false, changed: false, errors: [detail] }); +} + +// F-1 (mirrors execution/admitted.mjs's own F-1 — that copy is canonical; +// this is a minimal, lifecycle-scoped replica since the check isn't exported +// there): a relative lifecycle 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() — 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. Not shared as an import (yet) — a later refactor can hoist this +// into a common module once a second caller needs it; for now the small +// duplication keeps this file independent of execution/admitted.mjs. +// F9 (Windows parity): exe/bat/cmd/com/ps1 included alongside the script +// extensions — Windows' CreateProcess searches the CURRENT DIRECTORY before +// PATH for a bare relative executable name, so a null baseDir with e.g. +// `hook.bat` is exactly as exploitable there as a relative `.mjs` is on +// POSIX and must be refused the same way. +const SCRIPT_LIKE_RE = /\.(?:mjs|cjs|js|ts|py|rb|sh|pl|exe|bat|cmd|com|ps1)$/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); +} + +function commandIsUnanchorable(command) { + const [argv0, ...args] = command; + if (looksRelative(argv0)) return true; + return args.some((arg) => looksRelative(arg)); +} + +/** Honest refusal for a declared verb whose hook command is relative with no + * adapter base directory to anchor it — the hook subprocess is NEVER + * spawned for this verb (see commandIsUnanchorable above), so this can only + * ever be a refusal, never a fabricated success. Shaped exactly like + * hookFailureResult so callers (lifecycle-render.mjs's generic summary, + * runLifecycle) treat it identically to any other honest per-verb failure. */ +function unanchoredResult(verb, hostId, command) { + const detail = `'${hostId}' declares a relative lifecycle.${verb}.hook.command ` + + `(${JSON.stringify(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'; if (verb === 'detect' || verb === 'verify') return { observed: null, error: detail }; if (verb === 'plan') return { changed: false, operations: [], error: detail }; return lifecycleResult({ ok: false, changed: false, errors: [detail] }); @@ -121,24 +254,48 @@ function hookFailureResult(verb, result) { * defaults to a dynamic import of ./hook-runner.mjs (the sibling module), * fetched lazily so this factory (and this whole file) loads cleanly even * before that module exists, and so tests never pay for the import unless - * they omit the injection on purpose. + * they omit the injection on purpose. `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(). A verb whose hook.command + * is relative and has no baseDir to anchor it is NEVER wired to spawn — + * `adapter.unanchoredVerbs` names every verb refused this way, so a caller + * (registerAdmittedLifecycle's bootstrap caller) can surface it as a warning + * without needing to re-derive the check itself. * @param {any} manifest — validateAdapterManifest's return shape - * @param {{ runHook?: (args: any) => Promise<{ok:boolean, stdout:string, exitCode:number}> }} [opts] + * @param {{ runHook?: (args: any) => Promise<{ok:boolean, stdout:string, exitCode:number}>, + * baseDir?: string|null }} [opts] */ -export function buildAdmittedLifecycleAdapter(manifest, { runHook } = {}) { +export function buildAdmittedLifecycleAdapter(manifest, { runHook, baseDir = null } = {}) { const hostId = manifest.host.id; const declared = manifest.lifecycle ?? {}; - const adapter = { id: hostId }; + // Stashed alongside the verb functions (validateLifecycleAdapter only + // requires id + the five verbs — extra own-properties are untouched) so + // detectionBinFor(hostId) can find the manifest's own CLI binary name + // without a second store keyed by host id. + const adapter = { id: hostId, detectionBin: manifest.detection?.bin ?? hostId, unanchoredVerbs: [] }; for (const verb of LIFECYCLE_OPERATIONS) { const hookEntry = declared[verb]?.hook; if (!hookEntry) { adapter[verb] = async () => lifecycleResult({ ok: true, changed: false, facts: null }); continue; } + if (baseDir == null && commandIsUnanchorable(hookEntry.command)) { + adapter.unanchoredVerbs.push(verb); + adapter[verb] = async () => unanchoredResult(verb, hostId, hookEntry.command); + continue; + } adapter[verb] = async (context = {}) => { const run = runHook ?? (await import('./hook-runner.mjs')).runAdapterHook; const result = await run({ hook: hookEntry, hostId, verb, timeoutMs: hookEntry.timeoutMs, env: context.env, + // F-1: anchor a relative command to the adapter's own directory when + // one was declared; with no baseDir, the check above already proved + // this command has no relative component a cwd could redirect (bare + // PATH binaries only), so omitting cwd (Node's own default — inherit + // ak's process.cwd()) is safe and never reopens the arbitrary-code- + // execution vector this exists to close. + ...(baseDir == null ? {} : { cwd: baseDir }), }); if (!result?.ok) return hookFailureResult(verb, result); const payload = parseHookPayload(result); @@ -156,24 +313,43 @@ export function buildAdmittedLifecycleAdapter(manifest, { runHook } = {}) { * an admitted-but-not-yet-overlaid host id would otherwise never pass the * built-ins-only check. * - * Not called anywhere in production yet: registering an external host here - * makes it appear in hostsWithLifecycle() (registry-query, all hosts), but - * setup.mjs/sync.mjs/uninstall.mjs deliberately read builtinHostsWithLifecycle() - * instead, which stays built-ins-only. External lifecycle EXECUTION and a - * shape-agnostic command-loop body (today's loops assume the opencode result - * shape) graduate together, in a later wave — wiring a real caller for this - * function ahead of that loop-body rewrite would crash setup/sync/uninstall - * the moment a non-opencode-shaped host is admitted. + * Called from admission.mjs's bootstrapHostAdapters (ADR-0031 P3), as a + * sibling to the execution-registration block: any admitted manifest + * declaring a lifecycle block gets its adapter registered here so it appears + * in hostsWithLifecycle(). Registration alone never runs a hook — a + * registered admitted host is only actually EXERCISED by setup/sync/ + * uninstall's loops once lifecycleExecutionEnabled() also passes (the + * experimental flag AND explicit cfg enablement) for that run. `baseDir` + * (F-1) is threaded straight through to buildAdmittedLifecycleAdapter — the + * caller derives it from the manifest's own source the same way the + * execution-registration block does (baseDirForSource). * @param {any} manifest - * @param {{ runHook?: (args: any) => Promise }} [opts] + * @param {{ runHook?: (args: any) => Promise, baseDir?: string|null }} [opts] */ -export function registerAdmittedLifecycle(manifest, { runHook } = {}) { +export function registerAdmittedLifecycle(manifest, { runHook, baseDir = null } = {}) { const hostId = manifest.host.id; if (!effectiveHostRegistry().some((host) => host.id === hostId)) { throw new TypeError(`lifecycle registry: unknown host id '${hostId}' — not present in effectiveHostRegistry`); } - const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook, baseDir }); validateLifecycleAdapter(adapter); LIFECYCLE_ADAPTERS.set(hostId, adapter); + ADMITTED_LIFECYCLE_IDS.add(hostId); return adapter; } + +/** + * Clear every ADMITTED lifecycle registration (never a built-in — 'opencode' + * and any other registerBuiltinLifecycle entry survive untouched). Pairs + * with adapters/admitted.mjs's resetAdmitted() and execution/admitted.mjs's + * resetAdmittedExecution() the same way execution/admitted.mjs's own + * resetAllAdmitted() pairs those two: a test (or a future paired-reset + * helper) that resets the host overlay without also resetting this one would + * otherwise leave a stale, unreachable-but-still-registered lifecycle + * adapter behind for a host id that effectiveHostRegistry() no longer knows + * about. + */ +export function resetAdmittedLifecycle() { + for (const hostId of ADMITTED_LIFECYCLE_IDS) LIFECYCLE_ADAPTERS.delete(hostId); + ADMITTED_LIFECYCLE_IDS.clear(); +} diff --git a/src/lib/adapters/lifecycle-render.mjs b/src/lib/adapters/lifecycle-render.mjs new file mode 100644 index 0000000..e976f20 --- /dev/null +++ b/src/lib/adapters/lifecycle-render.mjs @@ -0,0 +1,200 @@ +// Shape-agnostic lifecycle report renderer (ADR-0031 P3). setup.mjs, sync.mjs +// and uninstall.mjs used to destructure a runLifecycle() result directly +// (stack.oc/.plugin/.agents/.skill for apply, ret.undo/.artifacts for undo) — +// a shape only OPENCODE_LIFECYCLE_ADAPTER produces. That worked only because +// the command loops iterated builtinHostsWithLifecycle() (opencode-only). +// Now that the loops iterate hostsWithLifecycle() (built-ins + admitted +// externals — see lifecycle-registry.mjs), an admitted host's adapter +// (buildAdmittedLifecycleAdapter) returns the GENERIC lifecycleResult shape +// instead ({ok, changed, facts, actions, ownership, warnings, errors} — see +// lifecycle.mjs) and the raw destructure would throw on `stack.oc`. This +// module is the single place that tells the two shapes apart and turns +// either one into print-ready lines, so no command needs to know which shape +// it got. +// +// opencode's rich shape renders the lines the three commands already printed +// before this wave (same text, same conditions) — pinned by the existing +// setup/sync/uninstall test suites — plus a level fix (Wave C security +// review F5): plugin/agents/skill now carry their OWN ok/status into the +// line's level instead of a hard-coded 'ok', so a failed sub-surface renders +// as failed rather than a fabricated green checkmark. A generic admitted +// host renders one honest summary line instead: ok/changed plus a short +// detail pulled from actions/errors/warnings, never a per-surface breakdown +// the manifest never promised. + +/** True when `lifecycle` is opencode's apply() shape: `{changed, result: + * {oc, plugin, agents, skill, markersChanged}}`. Any other shape (including + * a bare generic lifecycleResult, which has no `.result` at all) is generic. */ +function isOpencodeApplyShape(lifecycle) { + return !!(lifecycle && lifecycle.result && lifecycle.result.oc); +} + +/** True when `lifecycle` is opencode's undo() shape: `{changed, result: + * {undo, artifacts, ok}}`. */ +function isOpencodeUndoShape(lifecycle) { + return !!(lifecycle && lifecycle.result && lifecycle.result.undo); +} + +// F3 (Wave C security review, BLOCKER — ANSI/control-char smuggling): a +// hostile hook's stdout (a manifest's own detect/apply/undo hook, or a +// user-editable error string that eventually lands in one of these lines) +// can carry raw ANSI control sequences — e.g. ESC[2K (erase line) + ESC[1A +// (cursor up) followed by a forged "✓ opencode: … in sync" — which a +// terminal would happily execute, erasing the real (failing) line and +// forging a fake green one. Every line this module builds funnels through +// line() below, so stripping there is the one choke point that closes it +// for every caller, opencode-shaped or generic alike. Reused shape (not +// imported — command/lib boundary): src/commands/x/host-adapters.mjs's own +// ~8-line stripControl is the canonical copy; this is the lib-side twin. +function stripControl(value) { + const input = String(value ?? ''); + let out = ''; + for (const ch of input) { + const code = ch.codePointAt(0); + // Tab/LF/CR become a space rather than vanishing — this is also what + // clamps every line to a SINGLE line (no embedded newline can smuggle a + // second, attacker-controlled "line" into the terminal). + if (code === 0x09 || code === 0x0a || code === 0x0d) { out += ' '; continue; } + // C0 (0x00-0x1f, includes ESC 0x1b) and C1 (0x7f-0x9f, includes DEL + // 0x7f) are dropped outright — an ESC-led CSI sequence loses its ESC + // byte and the rest (e.g. "[2K") survives only as inert, visible text. + if (code <= 0x1f || (code >= 0x7f && code <= 0x9f)) continue; + out += ch; + } + return out; +} + +/** Typed constructor for one report line — keeps `level` a literal union + * instead of widening to `string` the moment it comes from a ternary, AND + * is the single choke point every line's text passes through (F3, above). + * @param {'ok'|'warn'|'info'|'fail'} level + * @param {string} text + * @returns {{level:'ok'|'warn'|'info'|'fail', text:string}} */ +function line(level, text) { + return { level, text: stripControl(text) }; +} + +// F5 (Wave C security review, BLOCKER — built-in sync regression): mirrors +// output.mjs's reportOutcome exactly (result.status ?? (ok?'ok':'failed'), +// then ok/degraded/skipped/anything-else -> ok/warn/info/fail) so a +// FAILED opencode sub-surface (plugin/agents/skill/oc itself) renders at its +// own real level instead of a level this renderer fabricates. Pre-wave sync +// read every opencode sub-result through reportOutcome directly; this is +// that same mapping, now shared by setup AND sync from the one renderer. +function levelForResult(result) { + const status = result?.status ?? (result?.ok ? 'ok' : 'failed'); + if (status === 'ok') return 'ok'; + if (status === 'degraded') return 'warn'; + if (status === 'skipped') return 'info'; + return 'fail'; +} + +/** A short, honest one-line detail for a generic lifecycleResult: the first + * error, else the first warning, else an action count, else "no changes" — + * never fabricated, never a per-surface guess. */ +function summarizeGeneric(result) { + if (Array.isArray(result?.errors) && result.errors.length) return result.errors[0]; + if (Array.isArray(result?.warnings) && result.warnings.length) return result.warnings[0]; + if (Array.isArray(result?.actions) && result.actions.length) return `${result.actions.length} action(s)`; + return 'no changes'; +} + +/** @returns {{shape:'opencode', fatal:boolean, ok:boolean, changed:boolean, + * ocChanged:boolean, markersChanged:boolean, + * lines:Array<{level:'ok'|'warn'|'info'|'fail', text:string}>}} */ +function renderOpencodeApply(hostId, lifecycle) { + const stack = lifecycle.result; + const lines = [line(levelForResult(stack.oc), `${hostId}: ${stack.oc.detail}`)]; + if (stack.oc.fatal) { + lines.push(line('warn', `${hostId} plugin/agents/skill/guidance skipped — ${stack.oc.detail}`)); + return { + shape: 'opencode', fatal: true, ok: !!stack.oc.ok, changed: !!lifecycle.changed, + ocChanged: !!stack.oc.changed, markersChanged: !!stack.markersChanged, lines, + }; + } + lines.push(line(levelForResult(stack.plugin), `${hostId} plugin: ${stack.plugin.detail}`)); + lines.push(line(levelForResult(stack.agents), `${hostId} agents: ${stack.agents.detail}`)); + // F5: restored the `|| !skill.ok` half of the gate — a failed skill write + // (ok:false, changed:false, e.g. opencode.mjs's adoptionBlocked path) must + // still be reported, not silently dropped because nothing "changed". + if (stack.skill.changed || !stack.skill.ok) { + lines.push(line(levelForResult(stack.skill), `${hostId} skill: ${stack.skill.detail}`)); + } + return { + shape: 'opencode', fatal: false, ok: !!stack.oc.ok, changed: !!lifecycle.changed, + ocChanged: !!stack.oc.changed, markersChanged: !!stack.markersChanged, lines, + }; +} + +/** @returns {{shape:'generic', fatal:boolean, ok:boolean, changed:boolean, + * ocChanged:boolean, markersChanged:boolean, + * lines:Array<{level:'ok'|'warn'|'info'|'fail', text:string}>}} */ +function renderGenericApply(hostId, result) { + const ok = !!result?.ok; + const changed = !!result?.changed; + const verdict = ok ? (changed ? 'applied' : 'in sync') : 'apply failed'; + const lines = [line(ok ? 'ok' : 'warn', `${hostId}: ${verdict} — ${summarizeGeneric(result)}`)]; + return { + shape: 'generic', fatal: false, ok, changed, ocChanged: false, markersChanged: false, lines, + }; +} + +/** + * Turn a runLifecycle({action:'apply', ...}) result into print-ready lines. + * Dispatches on shape (see module doc) — the caller never inspects the + * result's own fields, only this report's normalized ones. + * @param {string} hostId + * @param {any} lifecycle — runLifecycle's return value + * @returns {{shape:'opencode'|'generic', fatal:boolean, ok:boolean, + * changed:boolean, ocChanged:boolean, markersChanged:boolean, + * lines:Array<{level:'ok'|'warn'|'info'|'fail', text:string}>}} + */ +export function renderApplyReport(hostId, lifecycle) { + return isOpencodeApplyShape(lifecycle) + ? renderOpencodeApply(hostId, lifecycle) + : renderGenericApply(hostId, lifecycle); +} + +/** @returns {{shape:'opencode', ok:boolean, changed:boolean, + * lines:Array<{level:'ok'|'warn'|'info'|'fail', text:string}>}} */ +function renderOpencodeUndo(hostId, lifecycle) { + const ret = lifecycle.result; + const ok = !!ret.ok; + const changed = !!(ret.undo.changed || ret.artifacts.changed); + const lines = (changed || !ok) ? [line( + ok ? 'ok' : 'warn', + ok + ? `stripped ak-managed ${hostId} wiring + artifacts (opencode.json, plugin, agents, skill)` + : `${hostId} teardown incomplete — ${ret.undo.detail}`, + )] : []; + return { shape: 'opencode', ok, changed, lines }; +} + +/** @returns {{shape:'generic', ok:boolean, changed:boolean, + * lines:Array<{level:'ok'|'warn'|'info'|'fail', text:string}>}} */ +function renderGenericUndo(hostId, result) { + const ok = !!result?.ok; + const changed = !!result?.changed; + const lines = (changed || !ok) ? [line( + ok ? 'ok' : 'warn', + ok + ? `${hostId}: undo complete — ${summarizeGeneric(result)}` + : `${hostId} teardown incomplete — ${summarizeGeneric(result)}`, + )] : []; + return { shape: 'generic', ok, changed, lines }; +} + +/** + * Turn a runLifecycle({action:'undo', ...}) result into print-ready lines. + * Same dispatch as renderApplyReport. `ok` on the return is the caller's + * ownership-teardown signal (uninstall.mjs ANDs it across every host). + * @param {string} hostId + * @param {any} lifecycle — runLifecycle's return value + * @returns {{shape:'opencode'|'generic', ok:boolean, changed:boolean, + * lines:Array<{level:'ok'|'warn'|'info'|'fail', text:string}>}} + */ +export function renderUndoReport(hostId, lifecycle) { + return isOpencodeUndoShape(lifecycle) + ? renderOpencodeUndo(hostId, lifecycle) + : renderGenericUndo(hostId, lifecycle); +} diff --git a/tests/kit/adapter-admission.test.mjs b/tests/kit/adapter-admission.test.mjs index f318ee7..0984aa3 100644 --- a/tests/kit/adapter-admission.test.mjs +++ b/tests/kit/adapter-admission.test.mjs @@ -7,6 +7,9 @@ // undeclared one. 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 { admitAdapters, bootstrapHostAdapters, hashManifest, canonicalizeManifest, SUPPORTED_CONTRACT, } from '../../src/lib/adapters/admission.mjs'; @@ -285,7 +288,11 @@ test('buildAdmittedLifecycleAdapter satisfies validateLifecycleAdapter and route const calls = []; const runHook = async (args) => { calls.push(args); - return { ok: true, stdout: JSON.stringify({ observed: { version: '1.0.0' } }), exitCode: 0 }; + // F4: parseHookPayload reads stdoutText (the UNMERGED stdout hook-runner + // reports), not stdout (stdout+stderr merged) — a real runAdapterHook + // call always populates both; this mock does too, to match that contract. + const stdout = JSON.stringify({ observed: { version: '1.0.0' } }); + return { ok: true, stdout, stdoutText: stdout, exitCode: 0 }; }; const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); assert.doesNotThrow(() => validateLifecycleAdapter(adapter)); @@ -313,13 +320,47 @@ test('buildAdmittedLifecycleAdapter reports a hook failure honestly instead of f const manifest = validateAdapterManifest(validManifest({ lifecycle: { verify: { hook: { command: ['hermes', 'verify'] } } }, })); - const runHook = async () => ({ ok: false, stdout: 'boom', exitCode: 1 }); + // No `.detail` (a real failed runAdapterHook call always sets one — see + // hook-runner.mjs — so this specifically exercises hookFailureResult's + // OWN fallback: F4's fix reads stdoutText for that fallback, not stdout). + const runHook = async () => ({ ok: false, stdout: 'boom', stdoutText: 'boom', exitCode: 1 }); const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); const result = await adapter.verify({}); assert.equal(result.observed, null); assert.equal(result.error, 'boom'); }); +test('F4: hookFailureResult\'s detail fallback reads stdoutText (unmerged), never the merged stdout+stderr blob', async () => { + const manifest = validateAdapterManifest(validManifest({ + lifecycle: { verify: { hook: { command: ['hermes', 'verify'] } } }, + })); + // stdout is the MERGED blob (what a real hook-runner would produce when + // stderr chatter follows); stdoutText is the real, unmerged signal. + const runHook = async () => ({ + ok: false, stdout: 'clean-stdout\n--- stderr ---\nnoisy stderr chatter', stdoutText: 'clean-stdout', exitCode: 1, + }); + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); + const result = await adapter.verify({}); + assert.equal(result.error, 'clean-stdout', 'the fallback must read stdoutText, not the merged stdout blob'); +}); + +test('F4: a successful hook exit with valid JSON on stdout and unrelated stderr chatter still reports ok:true (Wave B R-1 twin)', async () => { + const manifest = validateAdapterManifest(validManifest({ + lifecycle: { apply: { hook: { command: ['hermes', 'apply'] } } }, + })); + const payload = JSON.stringify({ ok: true, changed: true, actions: ['wired'], ownership: [], warnings: [], errors: [] }); + // A stray stderr warning would break JSON.parse(stdout) (the merged blob) + // pre-fix — the hook still exited 0 with a fully valid JSON payload on its + // OWN stdout, so this must report success. + const runHook = async () => ({ + ok: true, stdout: `${payload}\n--- stderr ---\nsome deprecation warning`, stdoutText: payload, exitCode: 0, + }); + const adapter = buildAdmittedLifecycleAdapter(manifest, { runHook }); + const result = await adapter.apply({}); + assert.equal(result.ok, true, 'stderr chatter alongside valid stdout JSON must not fail the apply'); + assert.equal(result.changed, true); +}); + test('registerAdmittedLifecycle checks effectiveHostRegistry, not HOST_REGISTRY, and is retrievable via lifecycleAdapterFor', () => { applyAdmitted([{ entry: validHost({ id: 'hermes' }) }]); const manifest = validateAdapterManifest(validManifest()); @@ -332,3 +373,109 @@ test('registerAdmittedLifecycle throws for a host id absent from effectiveHostRe const manifest = validateAdapterManifest(validManifest({ name: 'ghost', host: validHost({ id: 'ghost' }) })); assert.throws(() => registerAdmittedLifecycle(manifest), /ghost/); }); + +// ── P3 (ADR-0031): bootstrapHostAdapters registers an admitted lifecycle ──── +// The sibling block to execution registration (§222-261 above): an admitted +// manifest declaring a lifecycle block gets its derived adapter registered +// during bootstrap, guarded and non-fatal, the same posture as execution. +// Every test here uses its own host id — LIFECYCLE_ADAPTERS is a +// process-shared Map with no unregister, so reusing 'hermes' would collide +// with the registerAdmittedLifecycle tests above. + +test('bootstrapHostAdapters registers the lifecycle adapter for an admitted manifest that declares one', async () => { + const name = 'hermes-boot-lifecycle'; + const manifest = validateAdapterManifest(validManifest({ + name, host: validHost({ id: name }), + lifecycle: { apply: { hook: { command: [name, 'apply'] } } }, + })); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name, source: 'mem://hermes-boot-lifecycle' }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: trustingConsent({ [name]: hash }), + }); + assert.equal(result.admitted.length, 1); + assert.deepEqual(result.warnings, [], `expected no warnings; got ${JSON.stringify(result.warnings)}`); + assert.notEqual(lifecycleAdapterFor(name), null, 'the lifecycle adapter must be registered by bootstrap'); +}); + +test('bootstrapHostAdapters never registers a lifecycle adapter for an admitted manifest with no lifecycle block', async () => { + const name = 'hermes-boot-no-lifecycle'; + const manifest = validateAdapterManifest(validManifest({ name, host: validHost({ id: name }) })); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name, source: 'mem://hermes-boot-no-lifecycle' }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: trustingConsent({ [name]: hash }), + }); + assert.equal(result.admitted.length, 1); + assert.equal(lifecycleAdapterFor(name), null, 'no lifecycle block declared — nothing to register'); +}); + +// ── F-1 (ADR-0031 P3, critical fix): bootstrap-level baseDir derivation ──── +// Mirrors adapter-execution.test.mjs's own F-1 (bootstrap) tests exactly: +// a file-sourced manifest's relative lifecycle hook resolves against the +// manifest's own directory (never the operator's cwd — this is a REAL +// subprocess spawn, not an injected runHook); a remote (npm/https) source +// has no persistent local bundle to anchor to, so its relative hook is +// refused with a surfaced 'lifecycle-unanchored' warning instead. + +test('F-1 (bootstrap): a file-sourced manifest derives baseDir from realpath(dirname(source)) and a real relative lifecycle hook runs anchored to it', async () => { + const name = 'hermes-f1-file'; + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-lifecycle-f1-basedir-')); + try { + // A REAL script, on disk, referenced by a RELATIVE path — proves the + // hook actually resolves against the manifest's own directory rather + // than wherever this test process happens to be running from. + fs.writeFileSync(path.join(tmpDir, 'apply-hook.mjs'), + "process.stdout.write(JSON.stringify({ok:true,changed:true,actions:['wired'],ownership:[],warnings:[],errors:[]}));\n"); + const manifest = validateAdapterManifest(validManifest({ + name, host: validHost({ id: name }), + lifecycle: { apply: { hook: { command: ['node', 'apply-hook.mjs'] } } }, + })); + const manifestPath = path.join(tmpDir, 'manifest.json'); + fs.writeFileSync(manifestPath, JSON.stringify(manifest)); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name, source: manifestPath }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: trustingConsent({ [name]: hash }), + }); + assert.equal(result.admitted.length, 1); + assert.deepEqual(result.warnings, [], `expected no warnings; got ${JSON.stringify(result.warnings)}`); + const adapter = lifecycleAdapterFor(name); + assert.notEqual(adapter, null); + assert.deepEqual(adapter.unanchoredVerbs, []); + const applied = await adapter.apply({}); + assert.equal(applied.ok, true, 'the real relative script must have actually run, anchored to the manifest\'s own directory'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } +}); + +test('F-1 (bootstrap): an npm-sourced admitted manifest with a relative lifecycle hook surfaces a lifecycle-unanchored warning, and the verb is refused (never spawned)', async () => { + const name = 'hermes-f1-npm'; + const manifest = validateAdapterManifest(validManifest({ + name, host: validHost({ id: name }), + lifecycle: { apply: { hook: { command: ['node', 'apply-hook.mjs'] } } }, + })); + const hash = hashManifest(manifest); + const result = await bootstrapHostAdapters({ + cfg: { hostAdapters: [{ name, source: 'npm:hermes-f1-npm-adapter@1.0.0' }] }, + env: { AK_EXPERIMENTAL_HOST_ADAPTERS: '1' }, + readManifest: async () => manifest, + consent: trustingConsent({ [name]: hash }), + }); + assert.equal(result.admitted.length, 1, 'the host itself still admits — only the unanchored verb is refused'); + const warning = result.warnings.find((w) => w.reason === 'lifecycle-unanchored'); + assert.ok(warning, `expected a 'lifecycle-unanchored' warning; got ${JSON.stringify(result.warnings)}`); + const adapter = lifecycleAdapterFor(name); + assert.notEqual(adapter, null, 'the adapter still registers — an unanchorable verb refuses itself, not the whole adapter'); + assert.deepEqual(adapter.unanchoredVerbs, ['apply']); + const applied = await adapter.apply({}); + assert.equal(applied.ok, false, 'the hook must NEVER have been spawned for an unanchored verb'); + assert.match(applied.errors[0], /no anchored adapter base directory/); +}); diff --git a/tests/kit/external-lifecycle.test.mjs b/tests/kit/external-lifecycle.test.mjs new file mode 100644 index 0000000..4c071f6 --- /dev/null +++ b/tests/kit/external-lifecycle.test.mjs @@ -0,0 +1,427 @@ +// ADR-0031 P3 — external lifecycle execution wired into setup/sync/uninstall. +// A synthetic ADMITTED host ('globex') with real, standalone node-script +// lifecycle hooks (apply/undo — no injected runHook, spawned through the +// REAL hook-runner, same black-box posture as adapter-conformance.test.mjs's +// acme fixture) proves the three command loops now iterate hostsWithLifecycle() +// safely: the hook actually runs and lifecycle-render.mjs's generic one-line +// summary prints, gated by lifecycleExecutionEnabled (cfg enablement AND the +// experimental flag — an admitted host is never exercised without both). +// +// setup.mjs and uninstall.mjs's admitted-host branch is reachable through a +// real command call (setup.run_machine / uninstall.run). sync.mjs's branch +// is additionally gated by `subsystems.has(hostId)`, sourced from +// status.mjs's collect() — which has no admitted-host awareness yet +// (HOST_DETAIL_RENDERERS is hardcoded to opencode; status.mjs is out of this +// wave's scope). That pre-existing gate means an admitted host's row never +// enters sync's plan today, so its lifecycle loop body is honestly +// unreachable through a real `ak sync` — pinned below as documented, current +// behavior (not a P3 regression: the gate and its rationale predate this +// wave, and generalizing status.mjs is a separate follow-up). +import { test } 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 { + sandboxHome, assertSandboxed, captureLog, rmrf, writeKitConfig, offlineKitConfig, fakeGlobalRoot, +} from './helpers/home-sandbox.mjs'; + +const HOME = sandboxHome('ak-external-lifecycle'); +const paths = await import('../../src/lib/paths.mjs'); +const setup = await import('../../src/commands/setup.mjs'); +const sync = await import('../../src/commands/sync.mjs'); +const uninstall = await import('../../src/commands/uninstall.mjs'); +const { loadKitConfig } = await import('../../src/lib/config.mjs'); +const { validateAdapterManifest } = await import('../../src/lib/adapters/manifest.mjs'); +const { applyAdmitted, resetAdmitted } = await import('../../src/lib/adapters/admitted.mjs'); +const { registerAdmittedLifecycle, lifecycleAdapterFor } = await import('../../src/lib/adapters/lifecycle-registry.mjs'); +assertSandboxed(paths, HOME); + +const PKG_ROOT = path.resolve(path.dirname(new URL(import.meta.url).pathname), '../..'); +const FLAG = 'AK_EXPERIMENTAL_HOST_ADAPTERS'; + +function globexHost(overrides = {}) { + return { + id: 'globex', + label: 'Globex', + install: { bin: 'globex-cli', externalInstallPolicy: 'detect-never-overwrite' }, + capabilities: { + canDriveSession: false, canBePrimary: false, canRouteActivities: false, + commandStatusline: false, transcripts: false, usage: false, + nativeMcpConfig: false, nativeGuidance: false, + }, + trust: { approvalPolicy: 'unchanged', changes: [] }, + enabledByDefault: false, + configProjection: 'ruflo', + observability: [], + ...overrides, + }; +} + +/** A real, standalone `node -e` subprocess — no injected runHook — that + * writes a marker file (proving it actually ran) then echoes a valid + * lifecycleResult payload on stdout. */ +function markerHookCommand(markerFile, payload) { + const script = `require('fs').writeFileSync(${JSON.stringify(markerFile)}, 'ran');` + + `process.stdout.write(${JSON.stringify(JSON.stringify(payload))});`; + return [process.execPath, '-e', script]; +} + +function globexManifest({ applyMarker, undoMarker }) { + return validateAdapterManifest({ + name: 'globex', + version: '1.0.0', + contract: 1, + host: globexHost(), + detection: { bin: 'globex-cli' }, + driving: { surfaces: ['acp'] }, + lifecycle: { + apply: { + hook: { + command: markerHookCommand(applyMarker, { + ok: true, changed: true, facts: null, actions: ['wired'], ownership: [], warnings: [], errors: [], + }), + timeoutMs: 5000, + }, + }, + undo: { + hook: { + command: markerHookCommand(undoMarker, { + ok: true, changed: true, facts: null, actions: ['unwired'], ownership: [], warnings: [], errors: [], + }), + timeoutMs: 5000, + }, + }, + }, + trust: { + changes: [{ + id: 'globex-subprocess-hooks', kind: 'third-party-adapter', scope: 'project', + owner: 'globex', value: 'subprocess hooks', effect: 'run consented lifecycle hooks for globex', + }], + }, + }); +} + +/** Registers the real (non-injected) lifecycle adapter for 'globex' and + * returns the marker paths it will write when its hooks actually run. + * These tests are about the command-loop WIRING/gating (does the loop reach + * the hook at all), not F-1 anchoring itself — F-1 has its own dedicated + * tests below and in lifecycle-registry.test.mjs / adapter-admission.test.mjs + * — so `baseDir` is passed explicitly (tmpDir plays the adapter's own + * directory) rather than left null: the hook commands here are `node -e + *