diff --git a/docs/PROVIDERS.md b/docs/PROVIDERS.md index b2defa63..fc288300 100644 --- a/docs/PROVIDERS.md +++ b/docs/PROVIDERS.md @@ -47,6 +47,33 @@ checks every binding declared in `kit.json` and prints a warning naming any entr unknown host, unknown provider, or unsupported transport — warnings only; nothing is changed or removed on your behalf. +## Local OpenAI-compatible servers + +Running a local model behind an OpenAI-compatible endpoint — MLX, LM Studio, `llama.cpp`, vLLM — +rather than Ollama? Declare it as a `local-openai` binding in `kit.json`: + +```json +{ + "integrations": { + "bindings": [ + { + "id": "mlx-via-codex", + "host": "codex", + "provider": "local-openai", + "transport": "openai-compatible", + "endpoint": "http://127.0.0.1:8080/v1" + } + ] + } +} +``` + +This gets you a named local inference target with `$0` billing and configured-grade provenance, +however the endpoint is served. `local-openai` is not an AQE provider type — `ollama` is. Loopback +`http://` is allowed; a remote endpoint requires `https://`; and the endpoint may never embed +credentials, fragments, or secret-bearing query parameters. See +[ADR-0028](adr/0028-local-openai-compatible-providers.md). + --- ## Level 0 — do nothing (the point) @@ -340,5 +367,6 @@ just makes the good default automatic and the customization reversible. [ADR-0006](adr/0006-primary-host-and-ambidextrous-mirroring.md). - Capability-driven integration axes, bindings, and provenance: [ADR-0016](adr/0016-capability-driven-integration-adapters.md). +- The generic local OpenAI-compatible provider: [ADR-0028](adr/0028-local-openai-compatible-providers.md). - Host env flags (`ENABLE_CLAUDE_CODE` / `ENABLE_CODEX`): upstream ruflo ADR-034, "Optional MCP Backends". diff --git a/docs/adr/0028-local-openai-compatible-providers.md b/docs/adr/0028-local-openai-compatible-providers.md new file mode 100644 index 00000000..1b6d6354 --- /dev/null +++ b/docs/adr/0028-local-openai-compatible-providers.md @@ -0,0 +1,178 @@ +# ADR-0028 — One generic local OpenAI-compatible provider, not a vendor enumeration + +- **Status:** Accepted +- **Date:** 2026-08-11 +- **Updated:** 2026-08-14 +- **Update note:** Accepted with corrections after review of PR #131: the quoted Hermes + `api_mode: openai` value is annotated as invalid rather than reproduced as valid (F-30), and the + AQE-projection asymmetry between `ollama` and `local-openai` is now stated explicitly as + intentional (F-29). Implemented with in-tree projections `['ruflo', 'codex', 'opencode']`. +- **Deciders:** agentic-kit maintainers +- **Related:** [ADR-0011](0011-local-model-provenance-zero-cost-and-transcript-fidelity.md), + [ADR-0016](0016-capability-driven-integration-adapters.md), + [ADR-0021](0021-inference-provider-provenance.md) + +Proposed by [@adrianco](https://github.com/adrianco) in +[PR #131](https://github.com/pacphi/agentic-kit/pull/131); accepted with the corrections recorded +below. + +## Context + +[ADR-0016](0016-capability-driven-integration-adapters.md) separates inference **providers** from +execution **hosts**, and [ADR-0011](0011-local-model-provenance-zero-cost-and-transcript-fidelity.md) +governs what a local provider may claim. The provider registry declared exactly **one** local +provider — `ollama` — and `BUILTIN_BINDINGS` carried exactly two local bindings, +`ollama-via-claude` and `ollama-via-codex` (`src/lib/adapters/registries.mjs`, +`src/lib/adapters/bindings.mjs`). + +Ollama is not the only way a local model is served, and on real machines it is frequently not the +one in use. A local inference server is normally reached as an **OpenAI-compatible HTTP endpoint on +loopback**: MLX/`mlx_lm.server`, LM Studio, `llama.cpp`'s server, and vLLM all present that shape. +Nothing in ak could name such an endpoint as a provider, so a machine running one had its local +inference either invisible or misfiled. + +This was observed on the proposer's reference machine. `~/.hermes/config.yaml` declared: + +```yaml +provider: mlxlocal +providers: + mlxlocal: + api: http://127.0.0.1:8080/v1 + api_mode: openai + default_model: mlx-community--Qwen3-Coder-Next-4bit +``` + +`api_mode: openai` is not a valid Hermes value — verified against `NousResearch/hermes-agent` +v0.20.0's `_parse_api_mode`, which silently **drops** an unrecognized value rather than raising. +The valid set is `{chat_completions, codex_responses, anthropic_messages, bedrock_converse, +codex_app_server}` (the newer key spelling is `transport`); `chat_completions` is the correct value +here, and it is also the default the parser falls back to, which is why the endpoint worked despite +the invalid setting. The quote above is reproduced verbatim because it is what the proposer's +machine observed — but `api_mode: openai` is not read as valid Hermes configuration by this ADR. + +Two facts survive that correction. First, the endpoint is a plain OpenAI-compatible loopback URL — +the generic shape, not a vendor-specific protocol. Second, **the provider name is user-chosen** +(`mlxlocal`). No enumeration of vendor ids can cover that case; a registry that lists `mlx`, +`lmstudio`, `llamacpp`, and `vllm` still has no row for `mlxlocal`. + +The binding machinery already accommodates this. `validateEndpoint` accepts loopback `http://` +while rejecting remote `http://`, embedded credentials, fragments, and secret-bearing query +parameters (`src/lib/adapters/config.mjs`). `http://127.0.0.1:8080/v1` was already a legal binding +endpoint; only the provider row was missing. + +## Decision + +### 1. Add one generic provider row: `local-openai` + +A single provider represents "an OpenAI-compatible model server the user runs locally", regardless +of which program serves it: + +- `billing: 'local'`, `credentials: { kind: 'none' }`, `capabilities.pricing: 'zero'` — required by + the registry's own construction invariants for a local provider (`validateRegistries`), and + correct: a loopback server bills nothing. A server that wants a placeholder token does not make + the credential *required*, so `kind: 'none'` remains accurate. +- `transports: ['openai-compatible']` — the only transport the row may claim. Anthropic-compatible + and native shells stay Ollama's, established separately. +- `capabilities.modelDiscovery: false`, `runtimeDiscovery: false`, `quota: false`, + `cacheAccounting: 'unknown'`. A generic endpoint exposes no catalogue ak may rely on. Claiming + `/v1/models` discovery would assert a uniformity across MLX, LM Studio, llama.cpp, and vLLM this + ADR has not measured. +- `observability: []`. Ollama keeps `ollama-catalog` / `ollama-runtime`; the generic row gets + neither, because it has no daemon API ak has verified. + +`ollama` is unchanged. It keeps its richer transports and its two observability sources precisely +because those rest on a specific, known daemon. + +### 2. The endpoint carries the identity; the provider row does not + +Which program serves a `local-openai` binding is recorded as the **binding's** endpoint and model, +not as provider identity. A user running MLX on `:8080` and LM Studio on `:1234` has two bindings +against one provider — the same relation ADR-0011 already names for `ollama-via-claude` / +`ollama-via-codex`, one level more general. + +Consistent with [ADR-0021](0021-inference-provider-provenance.md), such a binding establishes +**configured** provenance and nothing stronger. The endpoint is user-declared, so it may not be +displayed as observed, and it does not upgrade model, token, cache, or digest claims. The `$0` +claim is the one exception and is a property of the billing type, not of evidence about the run. + +### 3. No built-in bindings for the generic provider + +`BUILTIN_BINDINGS` gains nothing here. Ollama's two rows are justified by a fixed, well-known +default port; a generic local endpoint has no default ak may presume. Bindings are declared by the +user in `kit.json` and validated by the existing `assertValidBinding` path. "Local" is a billing +claim (user-run, `$0` — ADR-0011), not a topology constraint: a binding may name a user-run server +on another machine over `https`, while plain `http` remains loopback-only per `validateEndpoint`. + +### 4. Replace the derived capability block with per-entry data + +`providerEntries` previously derived capabilities from identity comparisons inside a `.map` +(`modelDiscovery: id === 'ollama'`, `pricing: id === 'ollama' ? 'zero' : …`). That construction does +not survive a second local provider: `local-openai` needs `pricing: 'zero'` without +`modelDiscovery`, which the `id === 'ollama'` coupling cannot express. Provider entries become +explicit records carrying their own capability block, matching how `hostEntries` is already +written. + +### 5. `local-openai` projects to `['ruflo', 'codex', 'opencode']`, and is not an AQE provider type + +The in-tree row declares projections `['ruflo', 'codex', 'opencode']`. Two omissions, both +deliberate: + +- **No `'claude'` projection.** The row claims only the OpenAI-compatible transport; Claude's + projection expects an anthropic-compatible surface, which is Ollama's arrangement, not this + provider's. +- **No `'aqe'` projection — `local-openai` is not an AQE provider type; `ollama` is.** AQE's + provider set is upstream's own enumeration (`ollama`, `onnx` are its local types), not something + ak may extend by adding a row to its own registry. Projecting `local-openai` into AQE would + fabricate a provider identity AQE has never declared it understands. This is an intentional + asymmetry, not a bug: `ollama` gets AQE projection because AQE names it; `local-openai` does not, + because AQE does not. `ak status`'s provider surface reflects this distinction; surfacing it + clearly is a sibling work package's scope, not this ADR's. + +## Consequences + +- A machine serving models from MLX, LM Studio, llama.cpp, vLLM, or anything else speaking + OpenAI-compatible HTTP on loopback can be described to ak without a new ADR per vendor, and + without inventing a provider id the user did not choose. +- Every host that can be pointed at an OpenAI-compatible base URL gains a nameable local provider. +- The generic row deliberately supports **less** than `ollama`: no catalogue, no runtime probe, no + digest. Surfaces that show local-model detail for Ollama will show less for `local-openai`, and + that gap is the honest reading of the evidence, not a defect to paper over. +- `local-openai` is not an AQE provider type and is not projected as one. AQE's own local routing + (`ollama`, `onnx`) is a separate axis, defined upstream, and untouched by this ADR. + +## Alternatives considered + +- **Named rows per runtime (`mlx`, `lmstudio`, `llamacpp`, `vllm`).** Rejected for this revision on + two grounds. It cannot cover a user-named provider such as the observed `mlxlocal`, so the + generic row is required regardless and the named rows would be additive decoration. And each row + would assert transport and discovery facts for a server this repository has not measured — + precisely the derivation-without-measurement that + [docs/LOCAL-MODEL-VALIDATION.md](../LOCAL-MODEL-VALIDATION.md) exists to correct. Named rows + remain available later, gated on an evidence pass of the same kind, and would then be able to + claim real `/v1/models` discovery instead of guessing at it. +- **Extend `ollama` to mean "any local server".** Rejected: it would make an established provider + id lie about which daemon is answering, and `ollama-catalog` / `ollama-runtime` would be attached + to endpoints that serve neither. +- **Infer the runtime by probing the endpoint.** Rejected as a default: ak would be spawning + network probes during status collection to manufacture an identity claim that ADR-0021 would then + have to grade as inferred anyway. The user naming their own binding is cheaper and more honest. + +## References + +- `src/lib/adapters/registries.mjs` (`providerEntries`, `validateProviderAdapter`, + `validateRegistries` local-billing invariants), `src/lib/adapters/bindings.mjs` + (`BUILTIN_BINDINGS`, `assertValidBinding`), `src/lib/adapters/config.mjs` (`validateEndpoint` + loopback rule). +- ADR-0011 (local-model provenance, `$0`, transcript fidelity), ADR-0016 (provider/binding + separation), ADR-0021 (provenance is carried, never upgraded). +- Observed local configuration: `~/.hermes/config.yaml` on the proposer's reference machine + (`api: http://127.0.0.1:8080/v1`); the file's `api_mode: openai` is not a valid Hermes value (see + Context) and is not cited as correct usage. +- Hermes source verified for the `api_mode` correction: `NousResearch/hermes-agent` v0.20.0, + `_parse_api_mode`. +- [PR #131](https://github.com/pacphi/agentic-kit/pull/131) — original proposal, including + companion proposals for a host-adapter extension point (that PR's ADR-0029) and a Hermes reference + adapter (that PR's ADR-0030), neither adopted into this repository by this ADR. +- Tests: `tests/kit/adapter-registries.test.mjs` (deep-equality pin for the five pre-existing + providers, registry invariants for a second local provider, binding validation against a + loopback OpenAI-compatible endpoint). diff --git a/docs/adr/README.md b/docs/adr/README.md index 4e39037f..db9cff44 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -36,6 +36,7 @@ Consequences**, and cites the grounded source it rests on where relevant. | [0025](0025-machine-footprint-metrics.md) | Machine footprint: infrastructure metrics for install, runtime, storage, and catalog | Implemented | | [0026](0026-about-component-directory.md) | About: a component directory that explains everything ak installs | Implemented | | [0027](0027-shared-project-census.md) | One project census, four scopes, every count explains itself | Implemented | +| [0028](0028-local-openai-compatible-providers.md) | One generic local OpenAI-compatible provider, not a vendor enumeration | Accepted | Theme: ADRs **0001–0006** define **dual-host LLM routing and leadership** — how `ak` lets ruflo route each development activity (architecture, implementation, testing, review, …) to the right host (Claude @@ -187,3 +188,14 @@ it, while Intelligence folds a repo's sub-directories and throwaway agent worktr identity because that is what a user picks — a distinction that was also a live bug, since keying the picker off identity while listing directories made 7 of 24 rows unreachable. Counts that remain different stay different, and say why. + +**0028** adds a second local provider, `local-openai`, because a local model is normally served as +an OpenAI-compatible endpoint on loopback — MLX, LM Studio, `llama.cpp`, vLLM — and frequently under +a name the *user* chose, which no vendor enumeration can cover; the registry previously knew only +`ollama`. It deliberately claims less than `ollama` (no catalogue, no runtime probe, no digest), +puts the runtime's identity in the binding's endpoint rather than in the provider id, and projects +to `['ruflo', 'codex', 'opencode']` only — no `claude` (the row claims only the OpenAI-compatible +transport) and no `aqe` (AQE's provider set is upstream's own enumeration; `ollama` is in it, +`local-openai` deliberately is not). Proposed by community contributor adrianco in PR #131, accepted +with a correction to the PR's quoted Hermes reference config, whose `api_mode: openai` is not a +valid Hermes value. diff --git a/src/commands/setup.mjs b/src/commands/setup.mjs index 75296047..829de26a 100644 --- a/src/commands/setup.mjs +++ b/src/commands/setup.mjs @@ -12,8 +12,9 @@ import * as heal from '../lib/heal.mjs'; import { fixStatusline } from '../lib/statusline.mjs'; import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions } from '../lib/mcp.mjs'; -import { OPENCODE_LIFECYCLE_ADAPTER, reconcileOpencodeGuidance } from '../lib/opencode.mjs'; +import { reconcileOpencodeGuidance } from '../lib/opencode.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; +import { hostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.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'; @@ -23,7 +24,7 @@ import { readJson, writeJsonWithBackup } from '../lib/settings.mjs'; import { withDb } from '../lib/sqlite.mjs'; import { findMemoryEntry } from '../lib/project-memory.mjs'; import { - setupTrustManifest, trustChangesForHost, trustManifestLines, + setupTrustManifest, trustManifestLines, } from '../lib/trust-manifest.mjs'; import * as paths from '../lib/paths.mjs'; import { ok, warn, fail, info, heading, bold, dim, reportOutcome } from '../lib/output.mjs'; @@ -91,19 +92,27 @@ const ask = async (q, dflt, yes) => { return a === '' ? dflt : a.startsWith('y'); }; -export const PROJECT_PERMISSION_MANIFEST = Object.freeze( - trustChangesForHost('claude', { kind: 'auto-approve' }).map((entry) => ({ - owner: entry.owner, rule: entry.value, effect: entry.effect, - })), -); - -export function projectPermissionManifest(cfg) { - const claude = setupTrustManifest(cfg, { project: true }) - .find((group) => group.hostId === 'claude'); - return (claude?.changes ?? []).filter((entry) => entry.kind === 'auto-approve') +// The authorized/disclosed auto-approve set is the UNION across every +// enabled host's trust manifest, not claude's alone (F-04) — a host whose +// auto-approve rules don't gate on enablement (requiresHostEnabled: false, +// claude's posture today) always contributes; an opt-in host (opencode, +// codex, or a future one) only contributes once cfg actually enables it. +// `hosts` is injectable so tests can prove a second host's rule survives +// removeUndisclosedPermissions through the same seam trust-manifest.test.mjs +// uses for host-registry-construction tests. +export function projectPermissionManifest(cfg, /** @type {{hosts?: any[]}} */ { hosts } = {}) { + const manifest = setupTrustManifest(cfg, { project: true, ...(hosts ? { hosts } : {}) }); + return manifest.flatMap((group) => group.changes) + .filter((entry) => entry.kind === 'auto-approve') .map((entry) => ({ owner: entry.owner, rule: entry.value, effect: entry.effect })); } +// The baseline (no non-default host enabled) authorized set — identical to +// "claude's rules" today because every claude auto-approve change sets +// requiresHostEnabled: false, but now derived through the same registry- +// driven path as projectPermissionManifest rather than hardcoded to claude. +export const PROJECT_PERMISSION_MANIFEST = Object.freeze(projectPermissionManifest({})); + export function discloseSetupTrust(cfg, { project = false } = {}) { const manifest = setupTrustManifest(cfg, { project }); if (!manifest.length) return manifest; @@ -204,35 +213,43 @@ export async function run_machine({ flags, pkgRoot, cfg }) { } } - // 6b. opencode host wiring — config-file MCP + skills, lifecycle plugin, - // converted agents, platform skill (opencode.mjs owns all of it). Only - // when the CLI is actually present: a declined/failed install must not - // leave a freshly-created config home behind (codex-review #4). - if (cfg.integrations?.hosts?.opencode) { - if (!(await have('opencode'))) { - warn('opencode: enabled but CLI not installed — wiring skipped (re-run `ak sync` after installing opencode-ai)'); - } else { - const lifecycle = await runLifecycle({ - adapter: OPENCODE_LIFECYCLE_ADAPTER, 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; - } - 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)'); + // 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 + // hostsWithLifecycle() rather than naming opencode, so a second + // 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. + for (const hostId of hostsWithLifecycle()) { + if (!cfg.integrations?.hosts?.[hostId]) continue; + if (!(await have(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; + } + 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; } + 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/status.mjs b/src/commands/status.mjs index d757b593..2fe5c8b9 100644 --- a/src/commands/status.mjs +++ b/src/commands/status.mjs @@ -24,6 +24,7 @@ import { coherence as adbCoherence } from '../lib/agentdb.mjs'; import { readJson } from '../lib/settings.mjs'; import { have } from '../lib/exec.mjs'; import { HOSTS, settingsTarget, isDefault, managedEnv, MANAGED_ENV_KEYS, hostInstallState, hostAuthState, bothHostsEnabled, aqeRouterFile, aqeSupportsAgentOverrides, credentialGaps, collectIntegrationFacts } from '../lib/providers.mjs'; +import { PROVIDER_REGISTRY } from '../lib/adapters/index.mjs'; import { configuredPolicyToAgentOverrides, agentOverridesDrift, routingSummary, divergedRoutes } from '../lib/routing.mjs'; import { qeCourtShipped, readQeCourtConfig, validateCourtConfig } from '../lib/qeCourt.mjs'; import { drift as ruvectorDrift } from '../lib/ruvector.mjs'; @@ -580,6 +581,19 @@ export async function collect({ pkgRoot, cwd = process.cwd() }) { } } } + // ADR-0028 F-29: local-openai is a local ($0) provider deliberately NOT + // projected to 'aqe' (unlike ollama, which is) — surface that asymmetry + // plainly so it reads as a fact, not a bug. Registry-driven (billing + + // projections), not an id check, so any future provider of the same + // shape gets the same treatment for free. + const providerById = Object.fromEntries(PROVIDER_REGISTRY.map((p) => [p.id, p])); + for (const binding of cfg.integrations?.bindings ?? []) { + const provider = providerById[binding.provider]; + if (!provider || provider.billing !== 'local' || provider.projections.includes('aqe')) continue; + const endpoint = binding.endpoint ? ` @ ${binding.endpoint}` : ''; + rows.push(row('providers', 'info', + `local binding: ${binding.provider} via ${binding.host}${endpoint} (${provider.billing} $0; not an AQE provider type)`)); + } } catch (e) { rows.push(row('providers', 'warn', `provider check unavailable: ${e.message}`)); } diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index 3af737aa..e0616b1b 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -8,8 +8,8 @@ import { have } from '../lib/exec.mjs'; import { fixStatusline, helperStampStale } from '../lib/statusline.mjs'; import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions } from '../lib/mcp.mjs'; -import { OPENCODE_LIFECYCLE_ADAPTER } from '../lib/opencode.mjs'; import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; +import { hostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.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'; @@ -188,23 +188,30 @@ 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). - if (subsystems.has('opencode') && cfg.integrations?.hosts?.opencode) { - if (!(await have('opencode'))) { - info('opencode: enabled but CLI not installed — wiring skipped (hosts step installs it)'); - } else { - const lifecycle = await runLifecycle({ - adapter: OPENCODE_LIFECYCLE_ADAPTER, action: 'apply', cfg, options: { pkgRoot }, - }); - const stack = lifecycle.result; - // 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); + // Registry-driven: loops hostsWithLifecycle() rather than naming opencode, + // so a second 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. + for (const hostId of hostsWithLifecycle()) { + if (!subsystems.has(hostId) || !cfg.integrations?.hosts?.[hostId]) continue; + if (!(await have(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; + // 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); } // 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 bed193a8..251019ee 100644 --- a/src/commands/uninstall.mjs +++ b/src/commands/uninstall.mjs @@ -9,8 +9,9 @@ import readline from 'node:readline/promises'; import { run as runCmd } from '../lib/exec.mjs'; import { stripBlock, BEGIN, BUILTIN_BLOCKS } from '../lib/blocks.mjs'; import { unregister } from '../lib/mcp.mjs'; -import { retireOpencode } from '../lib/opencode.mjs'; import { loadKitConfig, saveKitConfig } from '../lib/config.mjs'; +import { runLifecycle } from '../lib/adapters/lifecycle.mjs'; +import { hostsWithLifecycle, lifecycleAdapterFor } from '../lib/adapters/lifecycle-registry.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'; @@ -121,23 +122,36 @@ export async function run({ flags }) { }); } } - { - // cfg comes from the top of run() (read before any purge of kit.json). - // --purge removes kit.json above; persisting cfg here would recreate it. - if (cfg.integrations?.ownership?.opencode?.mcp === 'ak') { - if (dry) info('[dry-run] stripped ak-managed opencode wiring + artifacts (opencode.json, plugin, agents, skill)'); - else { - const ret = retireOpencode(cfg); - ownershipTeardownOk = ret.ok; - if (!flags.purge) saveKitConfig(cfg); - (ret.ok ? ok : warn)(ret.ok - ? 'stripped ak-managed opencode wiring + artifacts (opencode.json, plugin, agents, skill)' - : `opencode teardown incomplete — ${ret.undo.detail}`); - } - } else if (fs.existsSync(paths.opencodeDir())) { - // Not ak-managed (or never enabled): artifacts are still marker-gated, so - // only ak-deployed files leave — user-owned agents/skills/plugins stay. - act('removed ak-deployed opencode artifacts (plugin/agents/skill)', () => { retireOpencode(cfg); }); + // Registry-driven host lifecycle teardown — reached by id, never by name + // (mirrors x/host.mjs's off(), which does its undo the same way). cfg comes + // from the top of run() (read before any purge of kit.json); --purge + // 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. + for (const hostId of hostsWithLifecycle()) { + const adapter = lifecycleAdapterFor(hostId); + if (dry) { + info(`[dry-run] stripped ak-managed ${hostId} wiring + artifacts (opencode.json, plugin, agents, skill)`); + continue; + } + const retired = await runLifecycle({ adapter, action: 'undo', cfg }); + const ret = retired.result; + ownershipTeardownOk = ownershipTeardownOk && ret.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 + // on `changed` would strand a stale mcp:'ak' receipt forever on the + // 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}`); } } if (flags.purge && fs.existsSync(paths.kitConfigPath())) { diff --git a/src/commands/x/host.mjs b/src/commands/x/host.mjs index 5d7f445a..65cdead4 100644 --- a/src/commands/x/host.mjs +++ b/src/commands/x/host.mjs @@ -16,8 +16,9 @@ import { } from '../../lib/providers.mjs'; import { parseRouteSpecs, formatModelHelp, PRIMARY_HOSTS, DEFAULT_PRIMARY_HOST, divergedRoutes, refreshSeededRoutes, pruneRoutesForHosts, modelNote, ACTIVITIES } from '../../lib/routing.mjs'; import { loadKitConfig, saveKitConfig } from '../../lib/config.mjs'; -import { OPENCODE_LIFECYCLE_ADAPTER, reconcileOpencodeGuidance } from '../../lib/opencode.mjs'; +import { reconcileOpencodeGuidance } from '../../lib/opencode.mjs'; import { runLifecycle } from '../../lib/adapters/lifecycle.mjs'; +import { lifecycleAdapterFor } from '../../lib/adapters/lifecycle-registry.mjs'; import { routableHostIds, defaultHostMap, validateBinding, HOST_REGISTRY, PROVIDER_REGISTRY, } from '../../lib/adapters/index.mjs'; @@ -337,7 +338,7 @@ async function off({ cwd, pkgRoot }) { const rufloCodexManaged = cfg.integrations?.ownership?.codex?.reverseMcp === 'ak'; // OpenCode teardown reads its ownership receipt before the host/routing reset. // A failed teardown retains that receipt (including catalogDir) for a retry. - const retired = await runLifecycle({ adapter: OPENCODE_LIFECYCLE_ADAPTER, action: 'undo', cfg }); + const retired = await runLifecycle({ adapter: lifecycleAdapterFor('opencode'), action: 'undo', cfg }); const ret = retired.result; cfg.providers = { aqeProvider: null, @@ -629,7 +630,7 @@ async function pick({ flags, cwd, pkgRoot }) { warn('opencode: enabled but CLI not installed — wiring skipped (re-run `ak sync` after installing opencode-ai)'); } else { const lifecycle = await runLifecycle({ - adapter: OPENCODE_LIFECYCLE_ADAPTER, action: 'apply', cfg, options: { pkgRoot }, + adapter: lifecycleAdapterFor('opencode'), action: 'apply', cfg, options: { pkgRoot }, }); const stack = lifecycle.result; // persist the markers on ANY refresh (converged file + stale markers is @@ -653,7 +654,7 @@ async function pick({ flags, cwd, pkgRoot }) { // never the user's own opencode config. A teardown that cannot complete // (e.g. a JSONC config) is reported honestly — markers stay for the retry // and "disabled" is never claimed over still-active wiring. - const retired = await runLifecycle({ adapter: OPENCODE_LIFECYCLE_ADAPTER, action: 'undo', cfg }); + const retired = await runLifecycle({ adapter: lifecycleAdapterFor('opencode'), action: 'undo', cfg }); const ret = retired.result; saveKitConfig(cfg); // persist markers (nulled on success, retained on failure) if (ret.ok) ok(`opencode disabled: ${ret.undo.detail}; ${ret.artifacts.detail}`); diff --git a/src/lib/adapters/lifecycle-registry.mjs b/src/lib/adapters/lifecycle-registry.mjs new file mode 100644 index 00000000..705e1d43 --- /dev/null +++ b/src/lib/adapters/lifecycle-registry.mjs @@ -0,0 +1,63 @@ +// Lifecycle adapter registry — host lifecycle (detect/plan/apply/verify/undo) +// reached by id lookup, never by a named import of a concrete host module. +// +// Lifecycle adapters carry FUNCTIONS (detect/plan/apply/verify/undo), so they +// cannot live in the structuredClone'd data registry (registries.mjs) — this +// module is the function-carrying sibling, keyed the same way (host id). +// +// Direction: this module imports the concrete OPENCODE_LIFECYCLE_ADAPTER from +// opencode.mjs and registers it here, rather than opencode.mjs importing this +// module and self-registering. opencode.mjs already imports from +// adapters/config.mjs today, but nothing under adapters/ imports opencode.mjs +// — 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 { validateLifecycleAdapter } from './lifecycle.mjs'; +import { HOST_REGISTRY } from './registries.mjs'; +import { OPENCODE_LIFECYCLE_ADAPTER } from '../opencode.mjs'; + +const LIFECYCLE_ADAPTERS = new Map(); + +/** + * Register a built-in host's lifecycle adapter. Internal — called by this + * module itself, once per built-in, at import time. There is no dynamic or + * third-party host concept yet, so this is not a general-purpose plugin API. + * + * Throws when the host id isn't in HOST_REGISTRY or the adapter doesn't + * satisfy validateLifecycleAdapter: a wiring bug in a built-in is a load-time + * fault, not a runtime one. `hostRegistry` is overridable so this invariant + * is unit-testable directly (a synthetic registry) without needing a + * fresh-module import trick. + * @param {string} hostId + * @param {any} adapter — shape enforced by validateLifecycleAdapter, not the type system + * @param {{ hostRegistry?: ReadonlyArray<{id: string}> }} [opts] + * @returns {any} + */ +export function registerBuiltinLifecycle(hostId, adapter, { hostRegistry = HOST_REGISTRY } = {}) { + if (!hostRegistry.some((host) => host.id === hostId)) { + throw new TypeError(`lifecycle registry: unknown host id '${hostId}' — not present in HOST_REGISTRY`); + } + validateLifecycleAdapter(adapter); + LIFECYCLE_ADAPTERS.set(hostId, adapter); + return adapter; +} + +registerBuiltinLifecycle('opencode', OPENCODE_LIFECYCLE_ADAPTER); + +/** + * @param {string} hostId + * @returns {any|null}|null} + */ +export function lifecycleAdapterFor(hostId) { + return LIFECYCLE_ADAPTERS.get(hostId) ?? null; +} + +/** + * Host ids with a registered lifecycle adapter, in HOST_REGISTRY order (not + * Map-insertion order) so callers get a deterministic, registry-driven + * iteration order as more hosts gain lifecycle adapters. + * @returns {string[]} + */ +export function hostsWithLifecycle() { + return HOST_REGISTRY.filter((host) => LIFECYCLE_ADAPTERS.has(host.id)).map((host) => host.id); +} diff --git a/src/lib/adapters/registries.mjs b/src/lib/adapters/registries.mjs index d9cbcd51..29fb5357 100644 --- a/src/lib/adapters/registries.mjs +++ b/src/lib/adapters/registries.mjs @@ -225,22 +225,97 @@ const hostEntries = [ }, ]; +// F-28: was a tuple-array `.map` that derived capabilities by identity +// comparison (`modelDiscovery: id === 'ollama'`, `pricing: id === 'ollama' ? +// 'zero' : …`) — a construction that cannot express a second local provider +// needing pricing 'zero' WITHOUT modelDiscovery (ADR-0028's local-openai). +// Explicit per-entry records instead, matching the style hostEntries already +// uses; the five pre-existing rows are pinned deep-equal in +// tests/kit/adapter-registries.test.mjs so this rewrite cannot silently +// change what ships. const providerEntries = [ - ['anthropic', 'Anthropic', 'subscription', { kind: 'host-login' }, ['native'], ['claude'], []], - ['openai', 'OpenAI', 'subscription', { kind: 'host-login' }, ['native', 'openai-compatible'], ['codex'], []], - ['google', 'Google Gemini', 'metered', { kind: 'environment', env: ['GOOGLE_API_KEY', 'GEMINI_API_KEY'] }, ['native'], ['ruflo', 'aqe'], []], - ['openrouter', 'OpenRouter', 'metered', { kind: 'environment', env: ['OPENROUTER_API_KEY'] }, ['openai-compatible'], ['ruflo', 'aqe', 'claude', 'codex', 'opencode'], ['openrouter-metadata']], - ['ollama', 'Ollama', 'local', { kind: 'none' }, ['native', 'openai-compatible', 'anthropic-compatible'], ['ruflo', 'aqe', 'claude', 'codex', 'opencode'], ['ollama-catalog', 'ollama-runtime']], -].map(([id, label, billing, credentials, transports, projections, observability]) => ({ - id, label, billing, credentials, transports, projections, observability, - legacy: { apiProvider: id !== 'openrouter' }, - capabilities: { - modelDiscovery: id === 'ollama', runtimeDiscovery: id === 'ollama', - pricing: id === 'ollama' ? 'zero' : (id === 'openrouter' ? 'dated-offline' : 'provider-specific'), - quota: id === 'anthropic' || id === 'openai', - cacheAccounting: id === 'ollama' ? 'unknown' : 'provider-dependent', + { + id: 'anthropic', label: 'Anthropic', billing: 'subscription', + credentials: { kind: 'host-login' }, transports: ['native'], projections: ['claude'], observability: [], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'provider-specific', + quota: true, cacheAccounting: 'provider-dependent', + }, + }, + { + id: 'openai', label: 'OpenAI', billing: 'subscription', + credentials: { kind: 'host-login' }, transports: ['native', 'openai-compatible'], projections: ['codex'], observability: [], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'provider-specific', + quota: true, cacheAccounting: 'provider-dependent', + }, + }, + { + id: 'google', label: 'Google Gemini', billing: 'metered', + credentials: { kind: 'environment', env: ['GOOGLE_API_KEY', 'GEMINI_API_KEY'] }, + transports: ['native'], projections: ['ruflo', 'aqe'], observability: [], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'provider-specific', + quota: false, cacheAccounting: 'provider-dependent', + }, + }, + { + id: 'openrouter', label: 'OpenRouter', billing: 'metered', + credentials: { kind: 'environment', env: ['OPENROUTER_API_KEY'] }, + transports: ['openai-compatible'], projections: ['ruflo', 'aqe', 'claude', 'codex', 'opencode'], + observability: ['openrouter-metadata'], + // Unlike the other four, openrouter fronts many vendors' models behind one + // aggregator surface rather than being itself a single named vendor's API + // (F-28 dead-field trace: apiProviderIds() — the sole consumer — was + // removed in #100; the field is kept as honest metadata for any future + // reader, not for a live filter). + legacy: { apiProvider: false }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'dated-offline', + quota: false, cacheAccounting: 'provider-dependent', + }, + }, + { + id: 'ollama', label: 'Ollama', billing: 'local', + credentials: { kind: 'none' }, + transports: ['native', 'openai-compatible', 'anthropic-compatible'], + projections: ['ruflo', 'aqe', 'claude', 'codex', 'opencode'], + observability: ['ollama-catalog', 'ollama-runtime'], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: true, runtimeDiscovery: true, pricing: 'zero', + quota: false, cacheAccounting: 'unknown', + }, }, -})); + // ADR-0028: one generic local provider for any OpenAI-compatible model + // server on loopback (MLX, LM Studio, llama.cpp, vLLM, user-named + // endpoints), instead of enumerating vendors. + { + id: 'local-openai', label: 'Local OpenAI-compatible', billing: 'local', + credentials: { kind: 'none' }, transports: ['openai-compatible'], + // No 'aqe' — ollama is an AQE provider type, local-openai deliberately is + // not (ADR-0028's stated asymmetry). No 'claude' — assertValidBinding + // gates a binding's projection through provider.projections, and + // claude's own configProjection ('claude') expects an + // anthropic-compatible surface; this row claims only openai-compatible, + // so 'claude' is absent by design, not by oversight. + projections: ['ruflo', 'codex', 'opencode'], + // No daemon API ak has verified for a generic endpoint (unlike ollama's + // catalog/runtime sources), so no observability sources. + observability: [], + // Same reasoning as openrouter above: a generic OpenAI-compatible proxy + // for an arbitrary user-run server is not itself a distinct named + // vendor's API. + legacy: { apiProvider: false }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'zero', + quota: false, cacheAccounting: 'unknown', + }, + }, +]; const HOST_MAP = registryFrom(hostEntries, (entry) => validateHostAdapter(entry, { projections: PROJECTION_MAP, observability: OBSERVABILITY_MAP }), 'host'); diff --git a/src/lib/blocks.mjs b/src/lib/blocks.mjs index 3b993be1..af6fa92d 100644 --- a/src/lib/blocks.mjs +++ b/src/lib/blocks.mjs @@ -19,6 +19,7 @@ import path from 'node:path'; import { claudeDir, claudeMdPath, codexDir, opencodeDir, home } from './paths.mjs'; import { have } from './exec.mjs'; import { writeFileWithBackup } from './file-write.mjs'; +import { HOST_REGISTRY } from './adapters/index.mjs'; export const BEGIN = (slug) => ``; export const END = (slug) => ``; @@ -255,38 +256,104 @@ export function blocksForTarget(rows, targetName) { * carries a detector that never fires (`{type:'retired'}` → detect() falls to * its default false), so a present block is stripped and an absent one is a * no-op — the original detector (e.g. a live `flag`) can never re-upsert it into - * a file it must stay out of. Absent from the file → nothing happens (no write). */ -export function retiredForTarget(rows, targetName) { + * a file it must stay out of. Absent from the file → nothing happens (no write). + * + * `knownTargets` (optional) is the full universe of REAL target names — e.g. + * `guidanceTargets().map(t => t.name)`. When supplied, a row is only force- + * stripped when it names at least one target from that universe: it was + * legitimately re-scoped AWAY from `targetName` to some other real target + * (F-17's original migration case — see the dual-mode-reference tests). A row + * whose `guidanceFiles` name NOTHING in `knownTargets` (a typo, or a target no + * currently-registered host produces) is left alone instead of being force- + * stripped from every real file — this module has no basis for treating an + * unrecognized target as "retired from here." Omitting `knownTargets` + * preserves the original unconditional behavior for every existing 2-arg call + * site (status.mjs, nudge.mjs, opencode.mjs — this refactor's edit boundary + * doesn't cover them; only `reconcileGuidance` below opts into the 3-arg + * form). For the registry as it ships today every row's `guidanceFiles` are + * already within the known-target universe, so this is a no-observable-change + * refinement, not a behavior change. */ +export function retiredForTarget(rows, targetName, knownTargets) { return rows - .filter((r) => !(r.guidanceFiles ?? ['claude']).includes(targetName)) + .filter((r) => { + const files = r.guidanceFiles ?? ['claude']; + if (files.includes(targetName)) return false; + if (!knownTargets) return true; + return files.some((f) => knownTargets.includes(f)); + }) .map((r) => ({ ...r, detector: { type: 'retired' } })); } -/** The logical guidance targets `sync` (apply) and `status` (dry-run) both loop. - * ONE source of truth so the two commands can never drift. Always: machine-wide - * `~/.claude/CLAUDE.md` (claude) + the project's own `/AGENTS.md` (agents). - * The machine-scoped `~/.codex/AGENTS.md` (agents-user) is included ONLY when - * `~/.codex` already exists — codex's presence signal — and is NEVER created by - * this discovery (dir-exists gate, no mkdir). That single gate covers both cases: - * a codex machine that is momentarily single-host still gets the target (so a - * stale block can be stripped), and a codex-less machine never grows a ~/.codex. - * `~/.config/opencode/AGENTS.md` (agents-opencode) follows the identical rule - * (opencode's config home is its presence signal; opencode prefers this file - * over ~/.claude/CLAUDE.md, so it needs its own managed copy rather than - * inheriting claude's). `cfg` is accepted for call-site symmetry/forward-compat; - * the target set is cfg-independent today. `codexRoot`/`opencodeRoot` are test - * seams (default to the real dirs). - * @param {{ cwd?: string, cfg?: object, codexRoot?: string, opencodeRoot?: string }} opts */ -export function guidanceTargets({ cwd = process.cwd(), codexRoot = codexDir(), opencodeRoot = opencodeDir() } = {}) { - const targets = [ - { name: 'claude', label: 'CLAUDE.md', file: claudeMdPath() }, - { name: 'agents', label: 'AGENTS.md', file: path.join(cwd, 'AGENTS.md') }, - ]; - if (fs.existsSync(codexRoot)) { - targets.push({ name: 'agents-user', label: '~/.codex/AGENTS.md', file: path.join(codexRoot, 'AGENTS.md') }); - } - if (fs.existsSync(opencodeRoot)) { - targets.push({ name: 'agents-opencode', label: 'opencode AGENTS.md', file: path.join(opencodeRoot, 'AGENTS.md') }); +/** Per-host guidance-target construction (F-17). Every host in `hosts` that + * declares `capabilities.nativeGuidance` contributes a target named after its + * `legacy.guidanceFile` — the logical name HOST_REGISTRY already carries but + * which nothing consumed before this. Paths/labels/presence-gating stay a + * caller concern (see the module header comment: "Logical names only; paths + * stay a caller concern"), so this module still owns the bespoke mapping for + * the three hosts it knows about: + * - claude → 'claude': machine-wide `~/.claude/CLAUDE.md`, always included. + * - codex → 'agents': the project's own `/AGENTS.md`, always included + * (the AGENTS.md convention is host-neutral, not gated on codex being + * installed). codex ALSO contributes a companion machine-scoped target, + * 'agents-user' (`~/.codex/AGENTS.md`) — included only when `~/.codex` + * already exists (codex's presence signal) and NEVER created by this + * discovery (dir-exists gate, no mkdir: a momentarily single-host codex + * machine still gets the target so a stale block can be stripped; a + * codex-less machine never grows a ~/.codex). This companion has no + * `guidanceFile` entry of its own in HOST_REGISTRY — the registry models + * one logical guidance file per host today, and adding a second field is + * outside blocks.mjs's edit boundary — so it stays keyed off the codex + * host id rather than being independently derived. + * - opencode → 'agents-opencode': `~/.config/opencode/AGENTS.md`, included + * only when opencode's config home already exists (identical presence + * rule to agents-user; opencode prefers this file over + * ~/.claude/CLAUDE.md, so it needs its own managed copy). + * A host id this module has no bespoke mapping for still joins the loop (it is + * not silently dropped the way the old hardcoded array would drop it): it gets + * a generic, always-on target named after its own `guidanceFile`. This is what + * makes the list "derived" rather than closed — see the synthetic-host test in + * tests/kit/guidance-targets.test.mjs. + * `cfg` is accepted for call-site symmetry/forward-compat; the target set is + * cfg-independent today. `codexRoot`/`opencodeRoot`/`hosts` are test seams + * (default to the real dirs / the real registry). + * @param {{ cwd?: string, cfg?: object, codexRoot?: string, opencodeRoot?: string, hosts?: Array }} opts */ +export function guidanceTargets({ + cwd = process.cwd(), codexRoot = codexDir(), opencodeRoot = opencodeDir(), hosts = HOST_REGISTRY, +} = {}) { + const targets = []; + for (const host of hosts) { + if (!host?.capabilities?.nativeGuidance) continue; + const name = host.legacy?.guidanceFile; + if (!name) continue; + switch (host.id) { + case 'claude': + targets.push({ name, label: 'CLAUDE.md', file: claudeMdPath() }); + break; + case 'codex': + targets.push({ name, label: 'AGENTS.md', file: path.join(cwd, 'AGENTS.md') }); + if (fs.existsSync(codexRoot)) { + targets.push({ name: 'agents-user', label: '~/.codex/AGENTS.md', file: path.join(codexRoot, 'AGENTS.md') }); + } + break; + case 'opencode': + if (fs.existsSync(opencodeRoot)) { + targets.push({ name, label: 'opencode AGENTS.md', file: path.join(opencodeRoot, 'AGENTS.md') }); + } + break; + default: + // Generic fallback so an unrecognized-but-nativeGuidance host still + // joins the reconciliation loop instead of being silently unlooped. + // Id-shaped names only: path.join normalizes '..' so a hostile + // guidanceFile could escape cwd into a real write primitive + // (reconcileGuidance → syncBlocks). A non-conforming name is skipped + // entirely — excluded-and-safe beats sanitized-and-surprising. + // Schema-level validation of legacy.guidanceFile is the wave-4 + // admission gate's job, deliberately not duplicated here. + if (/^[a-z][a-z0-9-]{0,63}$/.test(name)) { + targets.push({ name, label: name, file: path.join(cwd, `${name}.md`) }); + } + break; + } } return targets; } @@ -311,8 +378,10 @@ export async function reconcileGuidance({ cwd, cfg, pkgRoot, context = {}, dryRu const rows = registry(cfg.customBlocks); const resolve = templateResolver(pkgRoot); const out = []; - for (const t of guidanceTargets({ cwd, cfg })) { - const treg = [...blocksForTarget(rows, t.name), ...retiredForTarget(rows, t.name)]; + const targets = guidanceTargets({ cwd, cfg }); + const knownTargets = targets.map((t) => t.name); + for (const t of targets) { + const treg = [...blocksForTarget(rows, t.name), ...retiredForTarget(rows, t.name, knownTargets)]; const res = await syncBlocks(t.file, treg, resolve, { context, dryRun }); const changed = res.filter((r) => r.action !== 'unchanged' && r.action !== 'skipped') .map((r) => `${r.slug} ${r.action}`).join(', '); diff --git a/src/lib/config.mjs b/src/lib/config.mjs index 20bb0b27..c3ec4ff3 100644 --- a/src/lib/config.mjs +++ b/src/lib/config.mjs @@ -50,6 +50,31 @@ const DEFAULTS = { const plain = (value) => value !== null && typeof value === 'object' && !Array.isArray(value); +// F-14: kit.json top-level keys this ak version understands, derived from +// DEFAULTS (the envelope already lists every recognized key, versioned +// sub-objects included) rather than a second literal that could drift. +const KNOWN_TOP_LEVEL_KEYS = new Set(Object.keys(DEFAULTS)); + +// Warn once per process per distinct unknown-key set, not once per load — +// loadKitConfig runs on nearly every command invocation. +const warnedUnknownKeySignatures = new Set(); + +function warnUnknownTopLevelKeys(parsed) { + if (!plain(parsed)) return; + const unknown = Object.keys(parsed).filter((key) => !KNOWN_TOP_LEVEL_KEYS.has(key)); + if (unknown.length === 0) return; + const signature = [...unknown].sort().join(','); + if (warnedUnknownKeySignatures.has(signature)) return; + warnedUnknownKeySignatures.add(signature); + // Deliberately console.error, not lib/output.mjs's warn() — that helper + // writes to stdout (console.log), which would corrupt `--json` consumers + // of loadKitConfig. console.error here mirrors the existing stderr-only + // warning convention in commands/run.mjs. + console.error( + `kit.json keys not recognized by this ak version: ${unknown.join(', ')} — preserved, ignored`, + ); +} + function assertLoadableEnvelopes(config) { if (!plain(config.integrations)) { throw new TypeError( @@ -145,6 +170,7 @@ export function loadKitConfig(file = kitConfigPath()) { } catch (error) { throw new KitConfigError(cand, error.message, { cause: error }); } + warnUnknownTopLevelKeys(parsed); // Migrate raw presence before defaults can masquerade as legacy user intent. try { return structuredClone(withDefaults(migrateKitConfig(parsed))); diff --git a/src/lib/execution/adapters.mjs b/src/lib/execution/adapters.mjs index 88f1c593..6f43f216 100644 --- a/src/lib/execution/adapters.mjs +++ b/src/lib/execution/adapters.mjs @@ -11,20 +11,36 @@ export const EXECUTION_ADAPTERS = Object.freeze(new Map([ ['opencode', OPENCODE_EXECUTION_ADAPTER], ])); -// Construction invariant (#88, architecture leg): every routable host needs an -// execution adapter, and every adapter needs a routable host — enforced at -// import, exactly like the capability registries' own construction check. A -// host flipped to canRouteActivities without an adapter (or an adapter added -// for an unroutable host) would otherwise load cleanly and fail only at -// runtime with cli_unavailable on every worker — issue #71's trap. -{ - const routable = new Set(routableHostIds()); - const adapted = new Set(EXECUTION_ADAPTERS.keys()); - const missing = [...routable].filter((id) => !adapted.has(id)); - const stray = [...adapted].filter((id) => !routable.has(id)); - if (missing.length || stray.length) { +// Construction invariant (#88, amended W1-B per ADR-0019's cli_unavailable +// degradation precedent): only the built-in -> registry direction throws now. +// A built-in execution adapter wired for a host the registry no longer marks +// routable is an in-tree wiring mistake — that still throws loudly at import, +// exactly like before. The reverse used to throw too (a registry host marked +// routable with no built-in adapter), but that meant any future host flipped +// to canRouteActivities:true ahead of its adapter landing would brick this +// module's import for every consumer of `ak run` — the merge-seam gap this +// wave exists to open. That direction is no longer a construction error: +// executionAdapterFor() below returns null for it, and the runner degrades +// just that one worker with cli_unavailable instead of crashing the run +// (src/lib/execution/runner.mjs's adapterFor already had this fallback for +// wholly-unknown hosts — a routable-but-unadapted host now takes the same +// path, never a new one). +export function assertBuiltinAdaptersRoutable(routableIds = routableHostIds()) { + const routable = routableIds instanceof Set ? routableIds : new Set(routableIds); + const stray = [...EXECUTION_ADAPTERS.keys()].filter((id) => !routable.has(id)); + if (stray.length) { throw new Error(`execution adapters out of sync with routable hosts: ` - + `${missing.length ? `no adapter for routable host(s): ${missing.join(', ')}. ` : ''}` - + `${stray.length ? `adapter(s) for non-routable host(s): ${stray.join(', ')}` : ''}`.trim()); + + `adapter(s) for non-routable host(s): ${stray.join(', ')}`); } } + +assertBuiltinAdaptersRoutable(); + +/** Merge seam (W1-B): resolve one host's execution adapter without exposing + * the underlying Map. Built-ins resolve here today; a later wave admits + * externally-registered adapters into this same lookup. Returns null for a + * host with no adapter wired yet — never throws, so callers can degrade a + * single worker instead of failing an entire run. */ +export function executionAdapterFor(hostId) { + return EXECUTION_ADAPTERS.get(hostId) ?? null; +} diff --git a/src/lib/execution/runner.mjs b/src/lib/execution/runner.mjs index dad892f2..9108b4b0 100644 --- a/src/lib/execution/runner.mjs +++ b/src/lib/execution/runner.mjs @@ -1,7 +1,7 @@ // Host-neutral execution coordinator. It owns scheduling, deadlines, and // lifecycle cleanup; adapters own each host's transport and protocol details. import { validateExecutionAdapter, validateWorkerResult } from './schema.mjs'; -import { EXECUTION_ADAPTERS } from './adapters.mjs'; +import { executionAdapterFor } from './adapters.mjs'; import { HANDOFF_REQUEST, normalizeHandoff, @@ -191,8 +191,18 @@ function validatePlan(plan) { } } +// W1-B: an omitted `adapters` option resolves through the built-in merge +// seam (executionAdapterFor) instead of a raw Map reference, so a future +// externally-admitted adapter joins this same lookup without callers here +// changing. Explicit injection (tests, callers with their own registry) +// still takes a Map or plain object, unchanged. Only `undefined` selects the +// built-in seam: an explicit `null` disables ALL adapters (every worker +// degrades cli_unavailable) rather than meaning "use defaults" — fail-safe, +// but don't pass null expecting the built-ins. function adapterFor(adapters, host) { - const adapter = adapters instanceof Map ? adapters.get(host) : adapters?.[host]; + const adapter = adapters === undefined + ? executionAdapterFor(host) + : adapters instanceof Map ? adapters.get(host) : adapters?.[host]; return adapter ? validateExecutionAdapter(adapter) : null; } @@ -265,7 +275,7 @@ async function executeWorkerWithEscalation(worker, adapters, { * A failed dependency blocks descendants; independent branches keep running. * `escalate: true` enables bounded per-worker ladder retries (ADR-0019). */ export async function executeRunPlan(plan, { - adapters = EXECUTION_ADAPTERS, cwd = process.cwd(), maxConcurrent = 4, timeoutMs, clock = nowIso, escalate = false, + adapters, cwd = process.cwd(), maxConcurrent = 4, timeoutMs, clock = nowIso, escalate = false, } = /** @type {{adapters?:Record|Map, cwd?:string, maxConcurrent?:number, timeoutMs?:number, clock?:()=>string, escalate?:boolean}} */ ({})) { validatePlan(plan); if (!Number.isInteger(maxConcurrent) || maxConcurrent < 1) throw new TypeError('maxConcurrent must be a positive integer'); diff --git a/tests/kit/adapter-registries.test.mjs b/tests/kit/adapter-registries.test.mjs index 9009b967..8ead813d 100644 --- a/tests/kit/adapter-registries.test.mjs +++ b/tests/kit/adapter-registries.test.mjs @@ -8,6 +8,7 @@ import { validateRegistries, validateHostAdapter, defaultHostMap, + assertValidBinding, } from '../../src/lib/adapters/index.mjs'; import { validHost, validProvider, validRegistries, @@ -151,3 +152,149 @@ test('validateHostAdapter accepts a host with an explicit boolean enabledByDefau assert.doesNotThrow(() => validateHostAdapter(validHost({ enabledByDefault: true }))); assert.doesNotThrow(() => validateHostAdapter(validHost({ enabledByDefault: false }))); }); + +// ── F-28: providerEntries moved from a tuple-array `.map` (capabilities derived +// by identity comparison, e.g. `modelDiscovery: id === 'ollama'`) to explicit +// per-entry object records — the same style hostEntries already uses. That +// construction could not express a second local provider needing pricing +// 'zero' WITHOUT modelDiscovery (ADR-0028's local-openai). This test hardcodes +// the five pre-existing rows' expected shape (not re-derived from the source) +// so the refactor cannot silently change what ships. See also ADR-0028. +test('F-28: the five pre-existing providers are unchanged by the per-entry rewrite', () => { + const byId = Object.fromEntries(PROVIDER_REGISTRY.map((entry) => [entry.id, entry])); + assert.deepEqual(byId.anthropic, { + id: 'anthropic', label: 'Anthropic', billing: 'subscription', + credentials: { kind: 'host-login' }, transports: ['native'], projections: ['claude'], observability: [], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'provider-specific', + quota: true, cacheAccounting: 'provider-dependent', + }, + }); + assert.deepEqual(byId.openai, { + id: 'openai', label: 'OpenAI', billing: 'subscription', + credentials: { kind: 'host-login' }, transports: ['native', 'openai-compatible'], projections: ['codex'], observability: [], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'provider-specific', + quota: true, cacheAccounting: 'provider-dependent', + }, + }); + assert.deepEqual(byId.google, { + id: 'google', label: 'Google Gemini', billing: 'metered', + credentials: { kind: 'environment', env: ['GOOGLE_API_KEY', 'GEMINI_API_KEY'] }, + transports: ['native'], projections: ['ruflo', 'aqe'], observability: [], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'provider-specific', + quota: false, cacheAccounting: 'provider-dependent', + }, + }); + assert.deepEqual(byId.openrouter, { + id: 'openrouter', label: 'OpenRouter', billing: 'metered', + credentials: { kind: 'environment', env: ['OPENROUTER_API_KEY'] }, + transports: ['openai-compatible'], projections: ['ruflo', 'aqe', 'claude', 'codex', 'opencode'], + observability: ['openrouter-metadata'], + legacy: { apiProvider: false }, + capabilities: { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'dated-offline', + quota: false, cacheAccounting: 'provider-dependent', + }, + }); + assert.deepEqual(byId.ollama, { + id: 'ollama', label: 'Ollama', billing: 'local', + credentials: { kind: 'none' }, + transports: ['native', 'openai-compatible', 'anthropic-compatible'], + projections: ['ruflo', 'aqe', 'claude', 'codex', 'opencode'], + observability: ['ollama-catalog', 'ollama-runtime'], + legacy: { apiProvider: true }, + capabilities: { + modelDiscovery: true, runtimeDiscovery: true, pricing: 'zero', + quota: false, cacheAccounting: 'unknown', + }, + }); +}); + +// ADR-0028: one generic local provider for any OpenAI-compatible model server +// on loopback (MLX, LM Studio, llama.cpp, vLLM, user-named endpoints), instead +// of enumerating vendors. +test('ADR-0028: local-openai is registered with the accepted shape', () => { + const localOpenai = PROVIDER_REGISTRY.find((entry) => entry.id === 'local-openai'); + assert.ok(localOpenai, 'local-openai must be registered'); + assert.equal(localOpenai.billing, 'local'); + assert.deepEqual(localOpenai.credentials, { kind: 'none' }); + assert.deepEqual(localOpenai.transports, ['openai-compatible']); + // Deliberate asymmetry vs ollama: no 'aqe' (ollama is an AQE provider type, + // local-openai is not) and no 'claude' (claude's projection expects an + // anthropic-compatible surface; local-openai claims only openai-compatible). + assert.deepEqual(localOpenai.projections, ['ruflo', 'codex', 'opencode']); + assert.deepEqual(localOpenai.observability, []); + assert.deepEqual(localOpenai.capabilities, { + modelDiscovery: false, runtimeDiscovery: false, pricing: 'zero', + quota: false, cacheAccounting: 'unknown', + }); +}); + +test('PROVIDER_REGISTRY has exactly six entries after the local-openai addition', () => { + assert.deepEqual(PROVIDER_REGISTRY.map((entry) => entry.id).sort(), [ + 'anthropic', 'google', 'local-openai', 'ollama', 'openai', 'openrouter', + ]); +}); + +// assertValidBinding gating: local-openai's projections/transports are +// exercised end-to-end through a user-declared binding, both the accepted +// shape and the rejections the accepted ADR design implies. +test('assertValidBinding accepts a user-declared local-openai binding on codex', () => { + const binding = assertValidBinding({ + id: 'local-openai-via-codex', host: 'codex', provider: 'local-openai', + transport: 'openai-compatible', endpoint: 'http://127.0.0.1:8080/v1', + provenance: 'configured', + }); + assert.equal(binding.provider, 'local-openai'); + assert.equal(binding.projection, 'codex'); +}); + +test('assertValidBinding rejects a local-openai binding on the native transport', () => { + assert.throws(() => assertValidBinding({ + id: 'local-openai-native', host: 'codex', provider: 'local-openai', + transport: 'native', endpoint: 'http://127.0.0.1:8080/v1', provenance: 'configured', + }), /unsupported transport/); +}); + +test('assertValidBinding rejects local-openai for the aqe projection', () => { + assert.throws(() => assertValidBinding({ + id: 'local-openai-aqe', host: 'codex', provider: 'local-openai', + transport: 'openai-compatible', endpoint: 'http://127.0.0.1:8080/v1', + projection: 'aqe', provenance: 'configured', + }), /does not support projection aqe/); +}); + +test('assertValidBinding rejects local-openai on claude host default projection', () => { + assert.throws(() => assertValidBinding({ + id: 'local-openai-claude', host: 'claude', provider: 'local-openai', + transport: 'openai-compatible', endpoint: 'http://127.0.0.1:8080/v1', + provenance: 'configured', + }), /does not support projection claude/); +}); + +// Endpoint topology pin (review nit): "local" is a BILLING claim (user-run, +// $0 — ADR-0011), not a loopback constraint. A user-run server on another +// machine is legal over https; plain http stays loopback-only +// (validateEndpoint's remote-http rule). Pinned so a future "tighten local +// to loopback" change is a deliberate decision, not drift. +test('assertValidBinding accepts a remote https endpoint for local-openai (billing, not topology)', () => { + const binding = assertValidBinding({ + id: 'local-openai-lan', host: 'codex', provider: 'local-openai', + transport: 'openai-compatible', endpoint: 'https://models.lan.example/v1', + provenance: 'configured', + }); + assert.equal(binding.endpoint, 'https://models.lan.example/v1'); +}); + +test('assertValidBinding rejects a remote plain-http endpoint for local-openai', () => { + assert.throws(() => assertValidBinding({ + id: 'local-openai-remote-http', host: 'codex', provider: 'local-openai', + transport: 'openai-compatible', endpoint: 'http://models.lan.example/v1', + provenance: 'configured', + }), /remote-http/); +}); diff --git a/tests/kit/execution-runner.test.mjs b/tests/kit/execution-runner.test.mjs index 1298452e..ef35de9b 100644 --- a/tests/kit/execution-runner.test.mjs +++ b/tests/kit/execution-runner.test.mjs @@ -1,6 +1,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { EXECUTION_ADAPTERS } from '../../src/lib/execution/adapters.mjs'; +import { EXECUTION_ADAPTERS, assertBuiltinAdaptersRoutable, executionAdapterFor } from '../../src/lib/execution/adapters.mjs'; import { executeRunPlan } from '../../src/lib/execution/runner.mjs'; const worker = (id, host = 'opencode', dependsOn) => ({ id, activity: 'implementation', role: 'coder', host, prompt: id, ...(dependsOn ? { dependsOn } : {}) }); @@ -452,11 +452,12 @@ test('runner reports an unknown host without attempting a lifecycle', async () = assert.match(result.failure.reason, /no execution adapter/); }); -// #88: the construction invariant — every routable host has an execution -// adapter and vice versa, enforced at import. A fresh process import must -// never throw; a violating edit fails here (and at import) instead of at -// runtime with cli_unavailable on every worker. -test('routable hosts and execution adapters cannot drift apart (construction invariant)', async () => { +// #88 (amended W1-B): the construction invariant now only enforces the +// built-in -> registry direction. A fresh process import must never throw +// against the real registry; a violating in-tree edit (a built-in adapter +// wired for a host the registry doesn't mark routable) still fails at +// import, never silently at runtime. +test('a fresh process imports the real execution adapters registry cleanly (construction invariant)', async () => { const { spawnSync } = await import('node:child_process'); const { fileURLToPath } = await import('node:url'); const path = await import('node:path'); @@ -468,6 +469,43 @@ test('routable hosts and execution adapters cannot drift apart (construction inv assert.match(r.stdout, /in-sync/); }); +// #88 (amended W1-B): a built-in adapter for a host the registry no longer +// marks routable is still an in-tree wiring mistake — this direction keeps +// throwing loudly, naming the offending host(s). +test('a built-in adapter for a non-routable host still throws (in-tree wiring mistake)', () => { + assert.throws(() => assertBuiltinAdaptersRoutable(['codex', 'opencode']), + /adapter\(s\) for non-routable host\(s\): claude/); +}); + +// #88 (amended W1-B): the reverse direction — a registry host marked +// routable with NO built-in adapter — is now a legitimate merge-seam gap, +// not a construction error. This is exactly the scenario a future registry +// entry (canRouteActivities:true, adapter landing in a later wave) creates. +test('a routable registry host with no built-in adapter does NOT throw at import (merge seam)', () => { + assert.doesNotThrow(() => assertBuiltinAdaptersRoutable(['claude', 'codex', 'opencode', 'future-host'])); +}); + +test('executionAdapterFor resolves built-ins identically to the old map, and null for an unwired host', () => { + assert.equal(executionAdapterFor('claude'), EXECUTION_ADAPTERS.get('claude')); + assert.equal(executionAdapterFor('codex'), EXECUTION_ADAPTERS.get('codex')); + assert.equal(executionAdapterFor('opencode'), EXECUTION_ADAPTERS.get('opencode')); + assert.equal(executionAdapterFor('future-host'), null); +}); + +// #88 (amended W1-B): a routed plan naming a host with no built-in adapter +// degrades that one worker instead of crashing the run. This is the SAME +// code path as "runner reports an unknown host without attempting a +// lifecycle" above — adapterFor() only cares whether an adapter is wired, +// never whether the registry calls the host routable — so a +// routable-but-unadapted host (this wave's actual failure mode) degrades +// identically to a wholly unknown one. +test('a routed plan naming a routable-but-unadapted host degrades that worker (cli_unavailable), never crashes', async () => { + const [result] = await executeRunPlan({ workers: [worker('a', 'future-host')] }, { clock }); + assert.equal(result.status, 'failed'); + assert.equal(result.exitCategory, 'cli_unavailable'); + assert.match(result.failure.reason, /no execution adapter/); +}); + // #88 test-gap: plan-validation guards — removing any of these converts a // throw into a silent hang or a late crash. test('plan validation rejects duplicate ids, unknown deps, self-deps, and bad concurrency', async () => { diff --git a/tests/kit/guidance-targets.test.mjs b/tests/kit/guidance-targets.test.mjs index 8e944660..d010a5ec 100644 --- a/tests/kit/guidance-targets.test.mjs +++ b/tests/kit/guidance-targets.test.mjs @@ -7,6 +7,7 @@ import { guidanceTargets, retiredForTarget, blocksForTarget, syncBlocks, registry, BUILTIN_BLOCKS, } from '../../src/lib/blocks.mjs'; import { codexDir, codexAgentsMdPath, home, claudeMdPath } from '../../src/lib/paths.mjs'; +import { HOST_REGISTRY } from '../../src/lib/adapters/index.mjs'; const block = (slug, body) => `\n${body}\n\n`; @@ -52,6 +53,121 @@ test('guidanceTargets defaults codexRoot to the real ~/.codex path', () => { assert.equal(targets.some((t) => t.name === 'agents-user'), hasCodex); }); +// ── F-17: guidanceTargets is derived from HOST_REGISTRY, not a closed literal +// list. These four cases pin EXACTLY today's shape (name/label/file, in order) +// for every host-enablement combination, so the registry-derived rewrite below +// cannot silently change what ships. ──────────────────────────────────────── + +test('F-17 pin: claude-only (neither codex nor opencode present)', () => { + const cwd = '/tmp/proj-pin-claude-only'; + const missingCodex = path.join(os.tmpdir(), 'no-such-codex-pin'); + const missingOpencode = path.join(os.tmpdir(), 'no-such-opencode-pin'); + const targets = guidanceTargets({ cwd, codexRoot: missingCodex, opencodeRoot: missingOpencode }); + assert.deepEqual(targets, [ + { name: 'claude', label: 'CLAUDE.md', file: claudeMdPath() }, + { name: 'agents', label: 'AGENTS.md', file: path.join(cwd, 'AGENTS.md') }, + ]); +}); + +test('F-17 pin: +codex (codex dir present, opencode absent)', () => { + const cwd = '/tmp/proj-pin-codex'; + const codexRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-pin-codex-')); + const missingOpencode = path.join(os.tmpdir(), 'no-such-opencode-pin-2'); + const targets = guidanceTargets({ cwd, codexRoot, opencodeRoot: missingOpencode }); + assert.deepEqual(targets, [ + { name: 'claude', label: 'CLAUDE.md', file: claudeMdPath() }, + { name: 'agents', label: 'AGENTS.md', file: path.join(cwd, 'AGENTS.md') }, + { name: 'agents-user', label: '~/.codex/AGENTS.md', file: path.join(codexRoot, 'AGENTS.md') }, + ]); + fs.rmSync(codexRoot, { recursive: true, force: true }); +}); + +test('F-17 pin: +opencode (opencode dir present, codex absent)', () => { + const cwd = '/tmp/proj-pin-opencode'; + const missingCodex = path.join(os.tmpdir(), 'no-such-codex-pin-3'); + const opencodeRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-pin-opencode-')); + const targets = guidanceTargets({ cwd, codexRoot: missingCodex, opencodeRoot }); + assert.deepEqual(targets, [ + { name: 'claude', label: 'CLAUDE.md', file: claudeMdPath() }, + { name: 'agents', label: 'AGENTS.md', file: path.join(cwd, 'AGENTS.md') }, + { name: 'agents-opencode', label: 'opencode AGENTS.md', file: path.join(opencodeRoot, 'AGENTS.md') }, + ]); + fs.rmSync(opencodeRoot, { recursive: true, force: true }); +}); + +test('F-17 pin: all hosts present (codex + opencode)', () => { + const cwd = '/tmp/proj-pin-all'; + const codexRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-pin-codex-all-')); + const opencodeRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-pin-opencode-all-')); + const targets = guidanceTargets({ cwd, codexRoot, opencodeRoot }); + assert.deepEqual(targets, [ + { name: 'claude', label: 'CLAUDE.md', file: claudeMdPath() }, + { name: 'agents', label: 'AGENTS.md', file: path.join(cwd, 'AGENTS.md') }, + { name: 'agents-user', label: '~/.codex/AGENTS.md', file: path.join(codexRoot, 'AGENTS.md') }, + { name: 'agents-opencode', label: 'opencode AGENTS.md', file: path.join(opencodeRoot, 'AGENTS.md') }, + ]); + fs.rmSync(codexRoot, { recursive: true, force: true }); + fs.rmSync(opencodeRoot, { recursive: true, force: true }); +}); + +// ── F-17: synthetic-host derivation — proves the loop is genuinely driven by +// the `hosts` registry (default HOST_REGISTRY), not a hardcoded literal array. +// A registry entry this module has no bespoke path/label mapping for still +// joins the loop via the generic fallback, named after its own guidanceFile. ─ + +test('F-17: a synthetic nativeGuidance host with a guidanceFile joins the loop', () => { + const cwd = '/tmp/proj-synthetic'; + const syntheticHost = { + id: 'acme-cli', + label: 'Acme CLI', + capabilities: { nativeGuidance: true }, + legacy: { guidanceFile: 'agents-acme' }, + }; + const hosts = [...HOST_REGISTRY, syntheticHost]; + const targets = guidanceTargets({ cwd, hosts, codexRoot: '/no/such/codex', opencodeRoot: '/no/such/opencode' }); + const acme = targets.find((t) => t.name === 'agents-acme'); + assert.ok(acme, 'synthetic host contributes a target named after its guidanceFile'); + assert.equal(acme.file, path.join(cwd, 'agents-acme.md')); + // Real hosts' output is unaffected by the addition. + assert.deepEqual(targets.map((t) => t.name), ['claude', 'agents', 'agents-acme']); +}); + +test('F-17: a non-id-shaped guidanceFile contributes NO target (path-traversal hardening)', () => { + const cwd = '/tmp/proj-hardened'; + const hostile = (guidanceFile) => ({ + id: 'evil-cli', label: 'Evil CLI', + capabilities: { nativeGuidance: true }, + legacy: { guidanceFile }, + }); + for (const name of ['../../../../etc/cron.d/evil', '/etc/passwd', 'a/b', 'a\\b', 'UPPER', 'dot.dot', '']) { + const targets = guidanceTargets({ + cwd, hosts: [...HOST_REGISTRY, hostile(name)], + codexRoot: '/no/such/codex', opencodeRoot: '/no/such/opencode', + }); + // The hostile host is skipped entirely (no target at all — so no file + // path is ever derived from the hostile name); built-ins are untouched. + assert.deepEqual(targets.map((t) => t.name), ['claude', 'agents'], `skipped for ${JSON.stringify(name)}`); + } +}); + +test('F-17: a host WITHOUT nativeGuidance never contributes a target, even with a guidanceFile', () => { + const cwd = '/tmp/proj-no-native-guidance'; + const syntheticHost = { + id: 'silent-cli', + label: 'Silent CLI', + capabilities: { nativeGuidance: false }, + legacy: { guidanceFile: 'agents-silent' }, + }; + const targets = guidanceTargets({ cwd, hosts: [syntheticHost] }); + assert.deepEqual(targets, []); +}); + +test('F-17: HOST_REGISTRY is the real default — passing it explicitly matches the implicit default', () => { + const explicit = guidanceTargets({ cwd: '/tmp/z', hosts: HOST_REGISTRY }); + const implicit = guidanceTargets({ cwd: '/tmp/z' }); + assert.deepEqual(explicit, implicit); +}); + // ── retiredForTarget ───────────────────────────────────────────────────────── test('retiredForTarget returns rows NOT listing the target, with a false detector', async () => { @@ -73,6 +189,43 @@ test('retiredForTarget returns rows NOT listing the target, with a false detecto } }); +// ── F-17: retiredForTarget's optional `knownTargets` universe ─────────────── + +test('retiredForTarget without knownTargets is unchanged (2-arg call sites keep todays behavior)', () => { + // status.mjs / nudge.mjs / opencode.mjs all call retiredForTarget with 2 args + // and are outside this refactor's edit boundary — the 3rd param must default + // to exactly today's unconditional-strip behavior, even for a row naming a + // target unknown to any host. + const rows = [ + { slug: 'typo-target', guidanceFiles: ['not-a-real-target'], detector: { type: 'always' } }, + ]; + const retired = retiredForTarget(rows, 'claude'); + assert.deepEqual(retired.map((r) => r.slug), ['typo-target']); +}); + +test('retiredForTarget with knownTargets leaves an unrecognized-target row untouched', () => { + const knownTargets = ['claude', 'agents', 'agents-user', 'agents-opencode']; + const rows = [ + { slug: 'typo-target', guidanceFiles: ['not-a-real-target'], detector: { type: 'always' } }, + { slug: 'moved-away', guidanceFiles: ['agents-opencode'], detector: { type: 'always' } }, + ]; + const retired = retiredForTarget(rows, 'claude', knownTargets); + // 'typo-target' names nothing in the known universe → left alone, not force-stripped. + // 'moved-away' names a REAL known target (just not claude) → still force-stripped, + // preserving the original re-scoping/migration behavior. + assert.deepEqual(retired.map((r) => r.slug), ['moved-away']); +}); + +test('reconcileGuidance-style knownTargets derived from guidanceTargets() matches the real universe', () => { + const derived = guidanceTargets({ cwd: '/tmp/z', codexRoot: home ? codexDir() : '/no/codex' }).map((t) => t.name); + // Whatever the real machine's derived universe is, a row naming something + // entirely outside it is never force-stripped once knownTargets is passed. + const rows = [{ slug: 'outsider', guidanceFiles: ['definitely-not-a-real-target-xyz'], detector: { type: 'always' } }]; + for (const name of derived) { + assert.deepEqual(retiredForTarget(rows, name, derived), [], `outsider row must not be force-stripped for ${name}`); + } +}); + // ── registry re-scoping of the dual-mode block ─────────────────────────────── test('dual-mode block is re-scoped to claude + agents-user (machine files)', () => { diff --git a/tests/kit/lifecycle-registry.test.mjs b/tests/kit/lifecycle-registry.test.mjs new file mode 100644 index 00000000..d90e38de --- /dev/null +++ b/tests/kit/lifecycle-registry.test.mjs @@ -0,0 +1,86 @@ +// The lifecycle registry — host lifecycle adapters (detect/plan/apply/verify/ +// undo) reached by id lookup, never by a named import of a concrete host +// module. Only opencode has a lifecycle adapter today; this file pins that +// the registry wires it correctly at import time AND that the five call +// sites that used to `import { OPENCODE_LIFECYCLE_ADAPTER } from +// '../lib/opencode.mjs'` no longer do — the whole point of F-02 is that a +// second lifecycle host never needs a new named import anywhere. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + registerBuiltinLifecycle, lifecycleAdapterFor, hostsWithLifecycle, +} from '../../src/lib/adapters/lifecycle-registry.mjs'; +import { OPENCODE_LIFECYCLE_ADAPTER } from '../../src/lib/opencode.mjs'; +import { validateLifecycleAdapter } from '../../src/lib/adapters/lifecycle.mjs'; +import { HOST_REGISTRY } from '../../src/lib/adapters/index.mjs'; +import { fakeLifecycleAdapter, fakeSurface } from './helpers/lifecycle-harness.mjs'; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const src = (rel) => fs.readFileSync(path.join(ROOT, 'src', rel), 'utf8'); + +test('lifecycleAdapterFor(\'opencode\') returns the real, validated adapter', () => { + const adapter = lifecycleAdapterFor('opencode'); + assert.equal(adapter, OPENCODE_LIFECYCLE_ADAPTER, 'registry must hand back the SAME adapter instance opencode.mjs exports'); + assert.doesNotThrow(() => validateLifecycleAdapter(adapter)); +}); + +test('lifecycleAdapterFor of an unknown/unregistered host id returns null', () => { + assert.equal(lifecycleAdapterFor('codex'), null, 'codex has no lifecycle adapter yet'); + assert.equal(lifecycleAdapterFor('not-a-real-host'), null); +}); + +test('hostsWithLifecycle() returns only registered ids, in HOST_REGISTRY order', () => { + const ids = hostsWithLifecycle(); + assert.deepEqual(ids, ['opencode'], 'only opencode is registered in this wave'); + // order sanity: whatever is registered must appear in the same relative + // order it holds in HOST_REGISTRY, not Map-insertion order. + const registryOrder = HOST_REGISTRY.map((h) => h.id); + let cursor = -1; + for (const id of ids) { + const idx = registryOrder.indexOf(id); + assert.ok(idx > cursor, `${id} out of HOST_REGISTRY order`); + cursor = idx; + } +}); + +test('registering an unknown host id throws at registration (construction-time invariant)', () => { + const surface = fakeSurface({ enabled: false }); + const adapter = fakeLifecycleAdapter(surface); + assert.throws( + () => registerBuiltinLifecycle('not-in-the-registry', adapter, { hostRegistry: [{ id: 'claude' }, { id: 'opencode' }] }), + /not-in-the-registry/, + ); +}); + +test('registering an adapter that fails the lifecycle contract throws (construction-time invariant)', () => { + assert.throws( + () => registerBuiltinLifecycle('claude', { id: 'claude', detect() {} }, { hostRegistry: [{ id: 'claude' }] }), + /must be a function/, + ); +}); + +test('registering a known host id with a valid adapter succeeds and is retrievable', () => { + const surface = fakeSurface({ enabled: false }); + const adapter = fakeLifecycleAdapter(surface); + const registered = registerBuiltinLifecycle('claude', adapter, { hostRegistry: [{ id: 'claude' }] }); + assert.equal(registered, adapter); +}); + +// ── architecture guard: dispatch by registry, never by name ──────────────── +// Real import errors (a wiring bug in a built-in) are supposed to throw at +// module load — proven above via the hostRegistry-override seam rather than a +// fresh-module import, since the real HOST_REGISTRY/opencode wiring is +// correct and importing it a second time would just re-hit Node's ESM module +// cache (no fresh throw to observe). +for (const rel of [ + 'commands/sync.mjs', 'commands/setup.mjs', 'commands/x/host.mjs', 'commands/uninstall.mjs', +]) { + test(`${rel} no longer names OPENCODE_LIFECYCLE_ADAPTER — it goes through the registry`, () => { + const text = src(rel); + assert.ok(!text.includes('OPENCODE_LIFECYCLE_ADAPTER'), + `${rel} must reach opencode's lifecycle adapter via lifecycleAdapterFor(...), not a named import`); + }); +} diff --git a/tests/kit/settings-config.test.mjs b/tests/kit/settings-config.test.mjs index 27ce0fc2..070dbc65 100644 --- a/tests/kit/settings-config.test.mjs +++ b/tests/kit/settings-config.test.mjs @@ -102,3 +102,81 @@ test('loadKitConfig merges partial files over defaults (user file wins)', () => assert.deepEqual(cfg.mcp.excludeFamilies, ['browser']); fs.rmSync(tmp, { recursive: true, force: true }); }); + +// F-14: an unrecognized top-level key (e.g. a future ak's `hostAdapters`) +// must never silently round-trip as a no-op — loadKitConfig warns once, +// naming the key, without dropping or throwing on it. + +/** Runs `fn` with console.error/console.log captured; returns { errLines, outLines }. */ +function captureConsole(fn) { + const errLines = []; + const outLines = []; + const realError = console.error; + const realLog = console.log; + console.error = (...a) => errLines.push(a.map(String).join(' ')); + console.log = (...a) => outLines.push(a.map(String).join(' ')); + try { + fn(); + } finally { + console.error = realError; + console.log = realLog; + } + return { errLines, outLines }; +} + +test('loadKitConfig warns once on an unrecognized top-level key, naming it', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-cfg-unknown-')); + const f = tmpFile(tmp, 'kit.json'); + fs.writeFileSync(f, JSON.stringify({ hostAdapters: { foo: true } })); + const { errLines } = captureConsole(() => loadKitConfig(f)); + assert.equal(errLines.length, 1); + assert.match(errLines[0], /hostAdapters/); + assert.match(errLines[0], /not recognized/); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +test('loadKitConfig warns nothing for a config with only recognized keys', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-cfg-clean-')); + const f = tmpFile(tmp, 'kit.json'); + fs.writeFileSync(f, JSON.stringify({ security: false, mcp: { excludeFamilies: ['browser'] } })); + const { errLines } = captureConsole(() => loadKitConfig(f)); + assert.deepEqual(errLines, []); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +test('loadKitConfig warns only once per process for the same unknown key set across repeated loads', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-cfg-repeat-')); + const f = tmpFile(tmp, 'kit.json'); + fs.writeFileSync(f, JSON.stringify({ futureFeatureFlag: true })); + const { errLines } = captureConsole(() => { + loadKitConfig(f); + loadKitConfig(f); + }); + assert.equal(errLines.length, 1, 'a second load with the same unknown key set must not warn again'); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +test('loadKitConfig round-trips an unrecognized top-level key through save unchanged', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-cfg-roundtrip-')); + const f = tmpFile(tmp, 'kit.json'); + fs.writeFileSync(f, JSON.stringify({ legacyPluginConfig: { retained: true } })); + const { errLines } = captureConsole(() => { + const cfg = loadKitConfig(f); + assert.deepEqual(cfg.legacyPluginConfig, { retained: true }); + saveKitConfig(cfg, f); + }); + assert.equal(errLines.length, 1, 'the unrecognized key still warns on the initial load'); + const raw = JSON.parse(fs.readFileSync(f, 'utf8')); + assert.deepEqual(raw.legacyPluginConfig, { retained: true }, 'unknown key must round-trip through save exactly'); + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +test('the unknown-key warning goes to stderr only, never stdout (so --json consumers are unaffected)', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'kit-cfg-stderr-')); + const f = tmpFile(tmp, 'kit.json'); + fs.writeFileSync(f, JSON.stringify({ experimentalWidget: true })); + const { errLines, outLines } = captureConsole(() => loadKitConfig(f)); + assert.equal(errLines.length, 1); + assert.deepEqual(outLines, [], 'the unknown-key warning must never write to stdout'); + fs.rmSync(tmp, { recursive: true, force: true }); +}); diff --git a/tests/kit/setup-command.test.mjs b/tests/kit/setup-command.test.mjs index a1172ee3..95c732a3 100644 --- a/tests/kit/setup-command.test.mjs +++ b/tests/kit/setup-command.test.mjs @@ -13,6 +13,7 @@ import { sandboxHome, assertSandboxed, snapshot, assertUnchanged, captureLog, rmrf, sandboxProject, writeKitConfig, offlineKitConfig, fakeGlobalRoot, } from './helpers/home-sandbox.mjs'; +import { HOST_REGISTRY } from '../../src/lib/adapters/index.mjs'; const HOME = sandboxHome('ak-setup'); const paths = await import('../../src/lib/paths.mjs'); @@ -69,6 +70,49 @@ test('project permission manifest omits AQE grants when AQE is disabled', () => ['ruflo', 'ruflo', 'ruflo', 'ruflo']); }); +// F-04: the authorized/disclosed set is the UNION across every enabled +// host's trust manifest, not claude's alone — otherwise a second host's own +// project-scope auto-approve rule reads as "undisclosed" and +// removeUndisclosedPermissions would strip it (and fail setup). Driven +// through the same synthetic-host `hosts` seam trust-manifest.test.mjs uses +// for host-registry-construction tests, so no change to registries.mjs is +// needed to prove it. +test('projectPermissionManifest unions a second enabled host\'s auto-approve rules', () => { + const future = { + id: 'grok', label: 'Grok CLI', + trust: { + approvalPolicy: 'managed', + changes: [{ + id: 'grok-auto-approve', kind: 'auto-approve', scope: 'project', + owner: 'agentic-kit', value: 'Bash(grok *)', effect: 'allow the Grok CLI', + operations: ['setup'], features: ['project'], + }], + }, + }; + const cfg = { aqe: true, ruvnetBrain: true, integrations: { hosts: { claude: true, grok: true } } }; + const manifest = setup.projectPermissionManifest(cfg, { hosts: [...HOST_REGISTRY, future] }); + const rules = manifest.map((entry) => entry.rule); + assert.ok(rules.includes('Bash(grok *)'), 'the second host\'s auto-approve rule must be disclosed, not dropped'); + assert.ok(rules.includes('mcp__claude-flow__*'), 'claude\'s own rules must still be present alongside it (a union, not a swap)'); + + // and the union must survive removeUndisclosedPermissions as authorized — + // a second host's disclosed rule must never be stripped as an intruder. + const project = sandboxProject('ak-setup-second-host-permission'); + const file = path.join(project, '.claude', 'settings.json'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify({ permissions: { allow: ['Bash(grok *)'] } }, null, 2)); + const removed = setup.removeUndisclosedPermissions(file, new Set(), new Set(rules)); + assert.deepEqual(removed, [], 'a disclosed second-host rule must never be stripped as undisclosed'); + rmrf(project); +}); + +test('projectPermissionManifest at only claude enabled is unchanged from the pre-F-04 claude-only manifest', () => { + // byte-identical for the default (claude-only) machine: the union with an + // empty "other hosts" set is exactly what trustChangesForHost('claude', + // {kind:'auto-approve'}) produced before this rework. + assert.deepEqual(setup.projectPermissionManifest({ aqe: true }), setup.PROJECT_PERMISSION_MANIFEST); +}); + test('permission verification removes only newly introduced undisclosed grants', () => { const project = sandboxProject('ak-setup-permissions'); const file = path.join(project, '.claude', 'settings.json'); diff --git a/tests/kit/status-command.test.mjs b/tests/kit/status-command.test.mjs index 721442e6..4a5c4cad 100644 --- a/tests/kit/status-command.test.mjs +++ b/tests/kit/status-command.test.mjs @@ -515,4 +515,47 @@ test('--json carries the opencode rows with the same shape the dashboard consume } finally { process.chdir(cwd); } }); +// ADR-0028 F-29: local-openai is a local ($0) provider deliberately NOT +// projected to 'aqe' (unlike ollama, which is) — status must surface that +// asymmetry plainly instead of letting it read as a bug. +test('a local-openai binding surfaces an info row naming provider, host, endpoint, and the non-AQE fact', async () => { + seedHome(); + const cfg = loadKitConfig(); + cfg.integrations.bindings = [{ + id: 'local-openai-via-codex', host: 'codex', provider: 'local-openai', + transport: 'openai-compatible', endpoint: 'http://127.0.0.1:8080/v1', + provenance: 'configured', + }]; + writeKitConfig(HOME, cfg); + const rows = await collect(); + const hit = rowsFor(rows, 'providers').find((r) => /local-openai/.test(r.message)); + assert.ok(hit, `expected a local-openai row: ${rowsFor(rows, 'providers').map((r) => r.message)}`); + assert.equal(hit.level, 'info'); + assert.equal(hit.fix, null, 'advisory only — nothing for sync to fix'); + assert.match(hit.message, /codex/); + assert.match(hit.message, /http:\/\/127\.0\.0\.1:8080\/v1/); + assert.match(hit.message, /not an AQE provider/i); +}); + +test('an ollama-only binding (local AND aqe-projected) triggers no local-non-AQE row', async () => { + seedHome(); + const cfg = loadKitConfig(); + cfg.integrations.bindings = [{ + id: 'ollama-via-claude', host: 'claude', provider: 'ollama', + transport: 'anthropic-compatible', endpoint: 'http://127.0.0.1:11434', + provenance: 'configured', + }]; + writeKitConfig(HOME, cfg); + const rows = await collect(); + const stray = rowsFor(rows, 'providers').find((r) => /not an AQE provider/i.test(r.message)); + assert.equal(stray, undefined, `ollama is AQE-projected and must not trigger the note: ${JSON.stringify(stray)}`); +}); + +test('no bindings declared: no local-non-AQE row (status stays unchanged for existing users)', async () => { + seedHome(); + const rows = await collect(); + const stray = rowsFor(rows, 'providers').find((r) => /not an AQE provider/i.test(r.message)); + assert.equal(stray, undefined, `expected zero local-non-AQE rows with no bindings: ${JSON.stringify(stray)}`); +}); + test.after(() => rmrf(HOME, PROJECT)); diff --git a/tests/kit/uninstall-command.test.mjs b/tests/kit/uninstall-command.test.mjs index 1c33fd53..fdddc1eb 100644 --- a/tests/kit/uninstall-command.test.mjs +++ b/tests/kit/uninstall-command.test.mjs @@ -288,6 +288,51 @@ test('default uninstall strips ak opencode wiring + artifacts and restores user assert.equal(cfg.integrations.ownership.opencode.mcp, null, 'ownership markers nulled (kit.json kept without --purge)'); }); +test('a quiet-success undo still persists the nulled ownership markers (save is not gated on file changes)', async () => { + // The stale-marker case the save-gate bug stranded forever: ownership says + // mcp:'ak' but the tracked entries are ALREADY absent from opencode.json and + // no ak artifacts exist on disk — undo rewrites nothing (changed:false, + // ok:true) yet nulls cfg's markers in memory. Those nulls must reach + // kit.json anyway, exactly as x/host.mjs's off()/pick() persist them. + seedHome(); + const cfgDir = ocHome(); + fs.mkdirSync(cfgDir, { recursive: true }); + fs.writeFileSync(path.join(cfgDir, 'opencode.json'), JSON.stringify({ + model: 'opencode/kimi-k3', + mcp: { 'my-server': { type: 'local', command: ['x'] } }, + permission: { edit: 'ask' }, + }, null, 2)); + writeKitConfig(HOME, { + aqe: true, + integrations: { + version: 2, + hosts: { claude: true, codex: false, opencode: true }, + bindings: [], + ownership: { + opencode: { + mcp: 'ak', + managed: { + mcp: { 'claude-flow': { prior: null, written: { type: 'local', command: ['ruflo', 'mcp', 'start'], enabled: true } } }, + paths: [], + permissions: { 'claude-flow_*': { prior: null, written: 'allow' } }, + permissionScalar: null, + artifacts: { plugin: hash('never-on-disk'), agents: {}, agentStamp: null, skill: hash('never-on-disk') }, + }, + }, + }, + }, + routing: { version: 1, primaryHost: 'claude', routes: {} }, + providers: {}, + }); + const { result } = await captureLog(() => uninstall.run({ flags: { yes: true } })); + assert.equal(result, 0); + const cfg = JSON.parse(fs.readFileSync(paths.kitConfigPath(), 'utf8')); + assert.equal(cfg.integrations.ownership.opencode.mcp, null, + 'stale mcp:ak marker is nulled in kit.json even when undo rewrote no file'); + const doc = JSON.parse(fs.readFileSync(path.join(cfgDir, 'opencode.json'), 'utf8')); + assert.deepEqual(doc.mcp, { 'my-server': { type: 'local', command: ['x'] } }, 'user config untouched'); +}); + test('repeated uninstall is harmless for the opencode footprint', async () => { seedHome(); seedManagedOpencode();