From d21af6502c022e0bcfa8bc18c1fc14742c746bef Mon Sep 17 00:00:00 2001 From: Chris Phillipson Date: Wed, 23 Sep 2026 09:05:05 -0700 Subject: [PATCH] fix(sync): no false warnings outside a project; stop the Codex claude-flow re-import loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ak sync` run from a non-project folder (e.g. ~/.claude) reported a hard failure and warnings that were not true, and every sync repeated a Codex repair that never stayed fixed. Project scope: - The AQE embedding projection now uses the enclosing git repository root, the same project gate as every other project writer. Outside a repository only the user Codex target applies; values an earlier version wrote there are relinquished by receipt. The .mcp.json AQE registration is required only in an AQE-initialized project (.agentic-qe/ present). - helperStampStale follows ruflo's own precondition: no hook-handler.cjs means ruflo never refreshes that folder, so it is not stale. - runHelperRefresh reports refreshed/current/failed from ruflo's actual result; a no-op no longer prints "helpers refreshed". - A location with no ruflo helpers gets an info line, not a statusline warning. Helpers and statusline resolve at the repository root. Codex: - Codex's Claude config import (re-run on a schedule, most likely by the desktop app) adds any Claude MCP server whose name Codex lacks, so the deleted legacy [mcp_servers.claude-flow] came back within hours (14 repairs 2026-09-04 → 2026-09-23). The alias is now replaced in place by a disabled placeholder (enabled = false), with backup and verification. The name stays taken, nothing launches, and the placeholder is never a repair target. - Removing a table now also removes its own child tables, instead of leaving an orphaned [mcp_servers..env]. Docs: ADR-0033, ADR-0055, SETUP.md, UPGRADING.md. Co-Authored-By: Claude Opus 5.5 --- docs/SETUP.md | 5 +- docs/UPGRADING.md | 19 ++++- ...dex-mcp-and-bound-qe-court-participants.md | 9 ++- docs/adr/0055-aqe-embedding-lifecycle.md | 1 + src/commands/sync.mjs | 19 +++-- src/lib/aqe-embedding-projection.mjs | 36 ++++++--- src/lib/codex-mcp-reconcile.mjs | 5 +- src/lib/mcp.mjs | 79 +++++++++++++------ src/lib/statusline.mjs | 36 ++++++--- src/lib/trust-manifest.mjs | 4 +- tests/kit/aqe-embedding-projection.test.mjs | 53 +++++++++++++ tests/kit/codex-mcp-convergence.test.mjs | 51 +++++++++--- tests/kit/codex-mcp.test.mjs | 6 +- tests/kit/helper-stamp.test.mjs | 53 ++++++++++++- 14 files changed, 302 insertions(+), 74 deletions(-) diff --git a/docs/SETUP.md b/docs/SETUP.md index 55d9a787..8fdad7c0 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -125,9 +125,10 @@ Setup with Codex enabled inventories the effective user Codex MCP configuration even in machine-only mode. When project setup is active it inventories that project's Codex MCP configuration too. An exact recursive `[mcp_servers.codex]` entry and the exact deprecated `claude-flow` Ruflo -transport are listed in the setup trust manifest, backed up, removed only after +transport are listed in the setup trust manifest, backed up, corrected only after the setup confirmation (or `--yes`), and re-probed before setup may report -success. A fresh recovery copy captures the immediate pre-repair bytes; symlinked +success. The recursive entry is removed; the `claude-flow` alias is replaced by a +disabled placeholder so Codex's Claude config import cannot add it back. A fresh recovery copy captures the immediate pre-repair bytes; symlinked or otherwise non-regular config files remain report-only. Project files and the legacy Ruflo replacement stay outside this repair scope under `--minimal` or when setup is run outside a project. diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index f6af9d9c..e88a2f3e 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -51,8 +51,23 @@ configuration and offers to remember correction of the recognized user-scope manifest with `--yes`, authorizes later setup/sync runs to repeat this bounded correction. Historical approvals are not converted into remembered consent. -Legacy alias removal runs after provisioning, with an enabled canonical `ruflo` -replacement present. Each removal retains the live fingerprint check, creates a +The correction disables the alias in place rather than deleting it: + +```toml +[mcp_servers.claude-flow] +# agentic-kit: disabled placeholder — stops Codex's Claude import from +# re-adding a duplicate of [mcp_servers.ruflo]. Delete this table to undo. +command = "ruflo" +args = ["mcp", "start"] +enabled = false +``` + +Codex's Claude config import adds any Claude MCP server whose name Codex does +not already have. A deleted alias therefore returned within hours. The +placeholder keeps the name taken and launches nothing. + +The correction runs after provisioning, with an enabled canonical `ruflo` +replacement present. Each correction retains the live fingerprint check, creates a current-state backup, and verifies the result. The remembered correction is used only while agentic-kit owns the workspace-aware `ak x ruflo-mcp` replacement. It does not authorize removing project entries, other names, custom commands, diff --git a/docs/adr/0033-retire-codex-mcp-and-bound-qe-court-participants.md b/docs/adr/0033-retire-codex-mcp-and-bound-qe-court-participants.md index a7937c81..740fb58e 100644 --- a/docs/adr/0033-retire-codex-mcp-and-bound-qe-court-participants.md +++ b/docs/adr/0033-retire-codex-mcp-and-bound-qe-court-participants.md @@ -3,7 +3,7 @@ - **Status:** Implemented; handoff transport amended by [ADR-0034](https://github.com/pacphi/agentic-kit/blob/main/docs/adr/0034-schema-native-handoffs-and-hermetic-seats.md) - **Date:** 2026-08-25 -- **Updated:** 2026-09-10 +- **Updated:** 2026-09-23 - **Update note:** Initial implementation retires only receipt-owned legacy MCP state, diagnoses effective Codex MCP topology, extends POSIX cleanup to process groups, and adds fail-closed QE-Court readiness plus a reciprocal live participant-transport regression. @@ -22,6 +22,13 @@ removal can record consent for matching future repairs while the workspace-aware replacement remains agentic-kit-owned. Final topology verification rejects unresolved duplication and in-scope recursion; custom entries remain preserved. + 2026-09-23: the legacy `claude-flow` alias is now replaced in place by a disabled + placeholder (`enabled = false`, bounded exact-table edit with backup, child tables + included) instead of deleted. Codex's Claude config import (re-run on a schedule by + the desktop app) adds any Claude MCP server whose name Codex lacks, so deletion was + undone within hours and every sync repeated the repair (14 repairs observed + 2026-09-04 → 2026-09-23). The placeholder keeps the name taken and is never itself a + repair target. - **Deciders:** agentic-kit maintainers - **Related:** [ADR-0001](https://github.com/pacphi/agentic-kit/blob/main/docs/adr/0001-one-routing-policy-many-projections.md), [ADR-0006](https://github.com/pacphi/agentic-kit/blob/main/docs/adr/0006-primary-host-and-ambidextrous-mirroring.md), diff --git a/docs/adr/0055-aqe-embedding-lifecycle.md b/docs/adr/0055-aqe-embedding-lifecycle.md index 842e8a80..8e89a0d8 100644 --- a/docs/adr/0055-aqe-embedding-lifecycle.md +++ b/docs/adr/0055-aqe-embedding-lifecycle.md @@ -4,6 +4,7 @@ - **Release target:** `4.0.0-alpha.53` - **Date:** 2026-09-20 - **Updated:** 2026-09-20 — implemented explicit defaults, owned Claude/Codex/OpenCode projections and qualified runtime proof +- **Updated:** 2026-09-23 — project projections are scoped to the enclosing git repository root; outside a repository only the user Codex target applies (earlier project-scope values are relinquished by receipt), and the `.mcp.json` AQE registration is required only in an AQE-initialized project - **Related:** [ADR-0023](0023-fail-closed-operations-and-explicit-degradation.md), [September repair](../audits/2026-09-09-aqe-integration-repair.md) diff --git a/src/commands/sync.mjs b/src/commands/sync.mjs index 98f7aa98..33632d42 100644 --- a/src/commands/sync.mjs +++ b/src/commands/sync.mjs @@ -7,7 +7,7 @@ import readline from 'node:readline/promises'; import { collect } from './status.mjs'; import * as heal from '../lib/heal.mjs'; import { have } from '../lib/exec.mjs'; -import { fixStatusline, helperStampStale, refreshRufloHelpers } from '../lib/statusline.mjs'; +import { fixStatusline, helperStampStale, runHelperRefresh } from '../lib/statusline.mjs'; import { reconcileGuidance } from '../lib/blocks.mjs'; import { register as mcpRegister, applyExclusions, codexMcpTopology, codexMcpRepairPlan, @@ -446,9 +446,12 @@ export const SYNC_STEPS = [ id: 'ruflo-helpers', when: (subs) => subs.has('versions'), run: async (ctx) => { - const refreshed = await withProgress('ruflo helpers', async () => refreshRufloHelpers(ctx.cwd)); - if (refreshed) ok('ruflo helpers: signed generated helpers refreshed'); - else if (helperStampStale(ctx.cwd)) warn('ruflo helpers: refresh did not converge; generated helpers remain stale'); + // Helpers live at the project root, not a subdirectory cwd (repoRoot is + // the kit-wide project gate); outside a repository the cwd is used as-is. + const root = paths.repoRoot(ctx.cwd) ?? ctx.cwd; + const outcome = await withProgress('ruflo helpers', async () => runHelperRefresh(root)); + if (outcome === 'refreshed') ok('ruflo helpers: signed generated helpers refreshed'); + else if (helperStampStale(root)) warn('ruflo helpers: refresh did not converge; generated helpers remain stale'); else info('ruflo helpers: current'); }, }, @@ -466,14 +469,16 @@ export const SYNC_STEPS = [ // refresh, up to 30s). The interval can't animate through a synchronous // execFileSync, but the initial "⏳ statusline" render lands before the // block — a visible label beats a frozen prompt. - const r = await withProgress('statusline', async () => fixStatusline(ctx.cwd)); - (r.applied || !r.reason ? ok : warn)(`statusline: ${r.applied ? `footer injected (v${r.version})` : r.reason ?? 'in sync'}`); + const root = paths.repoRoot(ctx.cwd) ?? ctx.cwd; + const r = await withProgress('statusline', async () => fixStatusline(root)); + if (r.absent) info('statusline: no ruflo helpers here — nothing to patch'); + else (r.applied || !r.reason ? ok : warn)(`statusline: ${r.applied ? `footer injected (v${r.version})` : r.reason ?? 'in sync'}`); // Honest success: fixStatusline invokes ruflo's PRIVATE helper-refresh // internal, best-effort. If the stamp is STILL stale after the heal, that // refresh silently no-oped (e.g. upstream moved the dist module) and the // next ruflo command will wipe the footer we just injected — say so // instead of letting "footer injected" read as converged. - if (helperStampStale(ctx.cwd)) { + if (helperStampStale(root)) { warn('statusline: helper stamp still stale after heal — ruflo\'s refresh did not run; the footer may not survive the next ruflo command'); } }, diff --git a/src/lib/aqe-embedding-projection.mjs b/src/lib/aqe-embedding-projection.mjs index db00f7ae..e63980a5 100644 --- a/src/lib/aqe-embedding-projection.mjs +++ b/src/lib/aqe-embedding-projection.mjs @@ -6,7 +6,7 @@ import { contextHome } from './codex-context-config.mjs'; import { writePrivateFileAtomic } from './file-write.mjs'; import { aqeTomlEnvironment } from './aqe-embedding-toml.mjs'; import { recognizedAqeTransport, parseEmbeddingJson } from './aqe-embedding-transport.mjs'; -import { claudeUserMcpPath } from './paths.mjs'; +import { claudeUserMcpPath, projectAqeDir, repoRoot } from './paths.mjs'; export const AQE_ENDPOINT_KEY = 'AQE_EMBEDDER_ENDPOINT'; const receiptPath = file => `${file}.agentic-kit-aqe-embedding.json`; @@ -62,23 +62,35 @@ function jsonEnvironment(source, target) { return { current: state(env), replace(next) { container.env ??= {}; if (next.present) container.env[AQE_ENDPOINT_KEY] = next.value; - else delete container.env[AQE_ENDPOINT_KEY]; + else { + delete container.env[AQE_ENDPOINT_KEY]; + if (Object.keys(container.env).length === 0) delete container.env; + } return JSON.stringify(doc, null, 2) + '\n'; } }; } +// Project scope is the enclosing git repository (paths.repoRoot), the same gate +// every other project-scoped writer uses. Outside a repository (e.g. running from +// ~/.claude or $HOME) there is no project to project into: project targets are +// kept only for receipt-based cleanup of values an earlier version wrote there. +// The AQE MCP registration is required only in a project AQE initialized. function targets(cfg, cwd, codexHome) { const hosts = cfg.integrations?.hosts ?? { claude: true }; + const root = repoRoot(cwd); + const base = root ?? cwd; + const project = root !== null; + const aqeProject = project && fs.existsSync(projectAqeDir(root)); const result = []; // Include disabled targets only for receipt-based cleanup. Never fabricate a registration. - const add = (file, kind, enabled, required = false, boundary = cwd) => { + const add = (file, kind, enabled, required = false, boundary = base) => { if (enabled || fs.existsSync(receiptPath(file))) result.push({ file, kind, enabled, required, boundary }); }; - add(path.join(cwd, '.mcp.json'), 'mcp', !!hosts.claude, true); - add(path.join(cwd, '.claude', 'settings.local.json'), 'settings', !!hosts.claude); - add(path.join(cwd, '.codex', 'config.toml'), 'toml', !!hosts.codex); + add(path.join(base, '.mcp.json'), 'mcp', project && !!hosts.claude, aqeProject); + add(path.join(base, '.claude', 'settings.local.json'), 'settings', project && !!hosts.claude); + add(path.join(base, '.codex', 'config.toml'), 'toml', project && !!hosts.codex); const userCodex = path.join(codexHome, 'config.toml'); - if (path.resolve(userCodex) !== path.resolve(cwd, '.codex', 'config.toml')) add(userCodex, 'toml', !!hosts.codex, false, codexHome); + if (path.resolve(userCodex) !== path.resolve(base, '.codex', 'config.toml')) add(userCodex, 'toml', !!hosts.codex, false, codexHome); return result; } @@ -154,7 +166,10 @@ export function prepareAqeEmbeddingInitialization(cfg, cwd = process.cwd(), opti return reconcileAqeEmbeddingProjections({ ...cfg, aqeEmbedding: { mode: 'unmanaged' } }, cwd, options); } -function claudePrecedenceFindings(cfg, cwd, userFile, desired) { +const projectEntry = (doc, root) => (root === null ? undefined + : Object.entries(doc.projects ?? {}).find(([key]) => path.resolve(key) === root)?.[1]); + +function claudePrecedenceFindings(cfg, root, userFile, desired) { const hosts = cfg.integrations?.hosts ?? { claude: true }; if (!hosts.claude || !desired.present) return []; try { @@ -162,7 +177,7 @@ function claudePrecedenceFindings(cfg, cwd, userFile, desired) { if (source === null) return []; const doc = parseEmbeddingJson(source); if (!plain(doc) || (doc.projects !== undefined && !plain(doc.projects))) throw new Error('invalid Claude MCP configuration preserved'); - const project = Object.entries(doc.projects ?? {}).find(([root]) => path.resolve(root) === cwd)?.[1]; + const project = projectEntry(doc, root); const local = project?.mcpServers?.['agentic-qe']; const user = doc.mcpServers?.['agentic-qe']; const findings = []; @@ -188,7 +203,8 @@ export function reconcileAqeEmbeddingProjections(cfg, cwd = process.cwd(), { } = {}) { if (cfg.aqe === false) return { ok: true, changed: false, detail: 'AQE disabled; projections skipped', findings: [] }; const desired = state(aqeEmbeddingProjectionEnv(cfg)); - const findings = claudePrecedenceFindings(cfg, path.resolve(cwd), claudeUserFile, desired); + const root = repoRoot(cwd); + const findings = claudePrecedenceFindings(cfg, root === null ? null : path.resolve(root), claudeUserFile, desired); for (const target of targets(cfg, path.resolve(cwd), codexHome)) { try { const plan = targetPlan(target, desired); diff --git a/src/lib/codex-mcp-reconcile.mjs b/src/lib/codex-mcp-reconcile.mjs index 80930750..26e5f452 100644 --- a/src/lib/codex-mcp-reconcile.mjs +++ b/src/lib/codex-mcp-reconcile.mjs @@ -41,7 +41,10 @@ export function rememberCodexMcpRepairs(cfg, targets, topology) { export async function confirmCodexMcpRepairs(cfg, targets, topology, { yes, confirm }) { if (!targets.length) return true; console.log(`Codex repair plan (${targets.length} action(s)):`); - for (const entry of targets) console.log(` • remove ${entry.file} → [mcp_servers.${entry.name}] — ${entry.reason}`); + for (const entry of targets) { + const action = entry.repairKind === 'legacy-ruflo' ? 'disable' : 'remove'; + console.log(` • ${action} ${entry.file} → [mcp_servers.${entry.name}] — ${entry.reason}`); + } const pending = targets.filter(entry => !hasCodexMcpRepairConsent(cfg, entry, topology)); if (!pending.length) { console.log('Using remembered consent for the recognized legacy Ruflo correction.'); diff --git a/src/lib/mcp.mjs b/src/lib/mcp.mjs index de2d1f86..8337b317 100644 --- a/src/lib/mcp.mjs +++ b/src/lib/mcp.mjs @@ -175,6 +175,14 @@ function mcpTableName(table) { return match ? (match[1] ?? match[2]) : null; } +/** A table's own children ([mcp_servers..env] …) belong to its block: + * cutting only up to the next header would orphan them. */ +function ownedBlockEnd(headers, index, childTables, sourceLength) { + let last = index; + while (headers[last + 1] && childTables.includes(headers[last + 1])) last++; + return headers[last + 1]?.index ?? sourceLength; +} + /** Read the bounded base-table sections behind Codex MCP registrations. This * is deliberately not a general TOML parser: only a base * `[mcp_servers.]` table with string command and string-array args facts is @@ -210,6 +218,7 @@ function codexMcpSections(file, scope) { const childPrefixes = [`mcp_servers.${name}.`, `mcp_servers."${name}".`]; const childTables = headers.filter((candidate) => childPrefixes.some((prefix) => candidate[1].trim().startsWith(prefix))); + const blockEnd = ownedBlockEnd(headers, index, childTables, source.length); const hasChildren = childTables.length > 0; const managedBrowserChild = childTables.length === 1 && childTables[0][1].trim() === `mcp_servers.${name}.env` @@ -231,7 +240,7 @@ function codexMcpSections(file, scope) { return [{ name, scope, file, command, args, enabled, repairKind, regularFile, fingerprint: fingerprint(source.slice(header.index, bodyEnd)), - start: header.index, end: bodyEnd, source, + start: header.index, end: blockEnd, source, }]; }); } @@ -325,11 +334,29 @@ function validRepairTarget(target) { return false; } -function removeProjectCodexMcpTarget(target) { + +/** The legacy alias is disabled in place rather than deleted. Codex's Claude + * config import (the desktop app re-runs it on a schedule) adds a Claude MCP + * server whenever Codex has no server of that NAME, so a deleted + * [mcp_servers.claude-flow] came straight back and every sync removed it again. + * A disabled table keeps the name taken, launches nothing, and is not a repair + * target (its extra field and comment make it a non-exact shape). */ +export const LEGACY_RUFLO_PLACEHOLDER = [ + '[mcp_servers.claude-flow]', + '# agentic-kit: disabled placeholder — stops Codex\'s Claude import from', + '# re-adding a duplicate of [mcp_servers.ruflo]. Delete this table to undo.', + 'command = "ruflo"', + 'args = ["mcp", "start"]', + 'enabled = false', +].join('\n'); + +function replaceCodexMcpTarget(target, replacement) { const current = codexMcpSections(target.file, target.scope) .find((entry) => sameRepairIdentity(entry, target)); if (!current) return false; - writeFileWithBackup(target.file, current.source.slice(0, current.start) + current.source.slice(current.end)); + const tail = current.source.slice(current.end); + const block = replacement ? `${replacement}\n${tail ? '\n' : ''}` : ''; + writeFileWithBackup(target.file, current.source.slice(0, current.start) + block + tail); return true; } @@ -349,6 +376,26 @@ function createCurrentRepairBackup(file) { return backup; } +/** Mutate one identity-checked target; returns a failure detail or null. The + * legacy alias is disabled in place (see LEGACY_RUFLO_PLACEHOLDER); project + * tables are edited directly; other user tables go through `codex mcp remove`. */ +async function applyCodexRepairTarget(target, live, runner, cwd) { + const disable = target.repairKind === 'legacy-ruflo'; + if (target.scope === 'project' || disable) { + try { + if (!replaceCodexMcpTarget(live, disable ? LEGACY_RUFLO_PLACEHOLDER : null)) { + throw new Error('exact confirmed table was not found'); + } + return null; + } catch (error) { + return `could not ${disable ? 'disable' : 'remove'} [mcp_servers.${target.name}] in ${target.file}: ${error.message}`; + } + } + const result = await runner('codex', ['mcp', 'remove', target.name], { cwd }); + if (result.code === 0) return null; + return `could not remove [mcp_servers.${target.name}] from ${target.file}: ${(result.stderr || result.stdout || `exit ${result.code}`).split('\n')[0].slice(0, 160)}`; +} + /** Apply a previously disclosed repair plan. Project-scoped tables are edited * through a bounded, backup-first exact-section removal because Codex's MCP * command writes only the user config. User-scoped tables go through Codex's @@ -387,27 +434,11 @@ export async function repairCodexMcpTopology(targets, cwd = process.cwd(), { }; } } - if (target.scope === 'project') { - try { - if (!removeProjectCodexMcpTarget(live)) throw new Error('exact confirmed table was not found'); - } catch (error) { - return { - ok: false, changed: removed.length > 0, - detail: `could not remove [mcp_servers.${target.name}] from ${target.file}: ${error.message}`, - }; - } - } else { - const result = await runner('codex', ['mcp', 'remove', target.name], { cwd }); - if (result.code !== 0) { - return { - ok: false, changed: removed.length > 0, - detail: `could not remove [mcp_servers.${target.name}] from ${target.file}: ${(result.stderr || result.stdout || `exit ${result.code}`).split('\n')[0].slice(0, 160)}`, - }; - } - } + const failure = await applyCodexRepairTarget(target, live, runner, cwd); + if (failure) return { ok: false, changed: removed.length > 0, detail: failure }; const remaining = inspect({ cwd }).registrations.find((entry) => entry.file === target.file && entry.scope === target.scope && entry.name === target.name); - if (remaining) { + if (target.repairKind === 'legacy-ruflo' ? remaining?.enabled !== false || remaining.repairKind : remaining) { return { ok: false, changed: true, detail: `Codex MCP repair could not be verified; [mcp_servers.${target.name}] remains`, @@ -415,7 +446,9 @@ export async function repairCodexMcpTopology(targets, cwd = process.cwd(), { } removed.push(target); } - return { ok: true, changed: removed.length > 0, detail: `removed ${removed.map((target) => `[mcp_servers.${target.name}]`).join(', ')}` }; + const verb = (target) => (target.repairKind === 'legacy-ruflo' ? 'disabled' : 'removed'); + return { ok: true, changed: removed.length > 0, + detail: removed.map((target) => `${verb(target)} [mcp_servers.${target.name}]`).join(', ') }; } /** diff --git a/src/lib/statusline.mjs b/src/lib/statusline.mjs index a304ae9c..96a209b9 100644 --- a/src/lib/statusline.mjs +++ b/src/lib/statusline.mjs @@ -149,6 +149,10 @@ function rufloCliVersion() { export function helperStampStale(root = process.cwd()) { const installed = rufloCliVersion(); if (!installed) return false; // no ruflo cli → nothing will refresh anything + // Mirror ruflo's own precondition (helper-refresh.js refreshOneHelpersDir): + // a directory without hook-handler.cjs is never refreshed, so there is no + // armed wipe to report — e.g. ~/.claude or a project ruflo never initialized. + if (!fs.existsSync(path.join(root, '.claude', 'helpers', 'hook-handler.cjs'))) return false; try { // Tolerate a `v` prefix: ruflo writes the stamp bare today, but a prefixed // stamp fed raw into cmpVersions goes NaN and reads as PERMANENTLY stale — @@ -170,25 +174,37 @@ export function helperStampStale(root = process.cwd()) { * failure returns false and injection proceeds on the file as-is (no worse * than the pre-fix behavior). */ export function refreshRufloHelpers(root = process.cwd(), { timeoutMs = 30_000 } = {}) { + return runHelperRefresh(root, { timeoutMs }) !== 'failed'; +} + +/** refreshRufloHelpers with the outcome kept: 'refreshed' when ruflo actually + * rewrote a helper set (project or global), 'current' when it ran unblocked + * and had nothing to write (stamp current, or no ruflo helpers in either + * location), 'failed' when the module is absent, rejected, hung or was + * blocked. Sync reports from this so a no-op never reads as a heal. */ +export function runHelperRefresh(root = process.cwd(), { timeoutMs = 30_000 } = {}) { const mod = helperRefreshModule(); - if (!fs.existsSync(mod)) return false; + if (!fs.existsSync(mod)) return 'failed'; try { - // A failed import / rejecting refresh exits 1; a BLOCKED refresh — upstream - // resolves {blocked:'…signature invalid'} rather than rejecting when the - // signed-manifest gate refuses to copy — exits 2. Both surface as false: - // "true = the refresh ran unblocked", never "a child spawned". (A resolved - // {refreshed:false} without blocked is a current-stamp no-op — success.) + // exit 1 = import failed / refresh rejected; 2 = BLOCKED (upstream resolves + // {blocked:'…signature invalid'} rather than rejecting when the + // signed-manifest gate refuses to copy); 3 = ran unblocked, wrote nothing. execFileSync(process.execPath, ['-e', - 'import(process.argv[2]).then((m)=>m.autoRefreshHelpersIfStale(process.argv[1],{alsoRefreshGlobal:true})).then((r)=>{if(r&&r.blocked)process.exit(2)},()=>process.exit(1))', + 'import(process.argv[2]).then((m)=>m.autoRefreshHelpersIfStale(process.argv[1],{alsoRefreshGlobal:true})).then((r)=>{if(r&&r.blocked)process.exit(2);if(!(r&&(r.refreshed||(r.global&&r.global.refreshed))))process.exit(3)},()=>process.exit(1))', root, pathToFileURL(mod).href, ], { stdio: 'ignore', timeout: timeoutMs }); - return true; - } catch { return false; } + return 'refreshed'; + } catch (error) { return error?.status === 3 ? 'current' : 'failed'; } } export function fixStatusline(root = process.cwd(), { dryRun = false } = {}) { const file = projectStatusline(root); - if (!fs.existsSync(file)) return { file, applied: false, reason: 'no statusline.cjs (created by ruflo init)' }; + if (!fs.existsSync(file)) { + // No ruflo helpers directory at all = not a ruflo-initialized location + // (e.g. ~/.claude): nothing to patch, which is not a defect. + return { file, applied: false, absent: !fs.existsSync(path.dirname(file)), + reason: 'no statusline.cjs (created by ruflo init)' }; + } // Order matters: refresh ruflo's helpers BEFORE reading, so we inject onto the // freshly-stamped copy and nothing rewrites it until the next ruflo upgrade diff --git a/src/lib/trust-manifest.mjs b/src/lib/trust-manifest.mjs index cb9086d4..ed136c9f 100644 --- a/src/lib/trust-manifest.mjs +++ b/src/lib/trust-manifest.mjs @@ -133,7 +133,7 @@ export function codexMcpRepairTrustManifest(plan = []) { && ((entry?.repairKind === 'recursive-codex' && entry?.name === 'codex') || (entry?.repairKind === 'legacy-ruflo' && entry?.name === 'claude-flow')) )).map((entry) => { - const mechanism = entry.scope === 'user' + const mechanism = entry.scope === 'user' && entry.repairKind === 'recursive-codex' ? 'through `codex mcp`' : 'with a bounded exact-table edit'; return { @@ -144,7 +144,7 @@ export function codexMcpRepairTrustManifest(plan = []) { value: `[mcp_servers.${entry.name}]`, effect: entry.repairKind === 'recursive-codex' ? `create a current-state recovery copy, remove this deprecated recursive Codex transport ${mechanism}, and verify its absence` - : `create a current-state recovery copy, remove this duplicate legacy Ruflo transport ${mechanism}, and verify its absence${entry.scope === 'user' ? '; remember this recognized correction for future setup/sync runs while the managed workspace-aware replacement remains present' : ''}`, + : `create a current-state recovery copy, replace this duplicate legacy Ruflo transport ${mechanism} with a disabled placeholder (so Codex's Claude config import cannot re-add it), and verify it is disabled${entry.scope === 'user' ? '; remember this recognized correction for future setup/sync runs while the managed workspace-aware replacement remains present' : ''}`, }; }); if (!changes.length) return []; diff --git a/tests/kit/aqe-embedding-projection.test.mjs b/tests/kit/aqe-embedding-projection.test.mjs index ab247803..cb2fc813 100644 --- a/tests/kit/aqe-embedding-projection.test.mjs +++ b/tests/kit/aqe-embedding-projection.test.mjs @@ -12,6 +12,7 @@ const reconcileAqeEmbeddingProjections = (cfg, cwd, opts = {}) => reconcile(cfg, function fixture(t) { const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-aqe-projection-')); t.after(() => fs.rmSync(cwd, { recursive: true, force: true })); + fs.mkdirSync(path.join(cwd, '.git')); const write = (name, value) => { const file = path.join(cwd, name); fs.mkdirSync(path.dirname(file), { recursive: true }); fs.writeFileSync(file, typeof value === 'string' ? value : JSON.stringify(value)); return file; @@ -195,3 +196,55 @@ test('reports explicit user scope conflict and refuses malformed precedence evid assert.equal(result.ok, false); assert.doesNotMatch(JSON.stringify(result), /DO_NOT_PRINT_SECRET/); }); + +test('outside a git repository no project target is required or written', t => { + const { cwd, cfg } = fixture(t); + fs.rmSync(path.join(cwd, '.git'), { recursive: true }); + cfg.integrations.hosts = { claude: true, codex: true }; + const codexHome = path.join(cwd, 'codex-home'); + fs.mkdirSync(codexHome); + fs.writeFileSync(path.join(codexHome, 'config.toml'), '[mcp_servers.agentic-qe]\ncommand = "aqe-mcp"\nargs = []\n'); + const result = reconcileAqeEmbeddingProjections(cfg, cwd, { codexHome }); + assert.equal(result.ok, true, JSON.stringify(result)); + assert.deepEqual(result.findings.map(f => f.file), [path.join(codexHome, 'config.toml')]); + assert.equal(fs.existsSync(path.join(cwd, '.claude')), false); + assert.equal(fs.existsSync(path.join(cwd, '.mcp.json')), false); + assert.match(fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8'), /AQE_EMBEDDER_ENDPOINT/); +}); + +test('outside a git repository a previously owned stray value is relinquished', t => { + const { cwd, write, cfg } = fixture(t); + fs.rmSync(path.join(cwd, '.git'), { recursive: true }); + const file = write('.claude/settings.local.json', { env: { AQE_EMBEDDER_ENDPOINT: cfg.aqeEmbedding.endpoint } }); + write('.claude/settings.local.json.agentic-kit-aqe-embedding.json', { + version: 1, before: { present: false }, after: { present: true, value: cfg.aqeEmbedding.endpoint }, pending: false, + }); + const result = reconcileAqeEmbeddingProjections(cfg, cwd, { codexHome: path.join(cwd, 'none') }); + assert.equal(result.ok, true, JSON.stringify(result)); + assert.deepEqual(JSON.parse(fs.readFileSync(file)), {}); + assert.equal(fs.existsSync(`${file}.agentic-kit-aqe-embedding.json`), false); + assert.equal(inspectAqeEmbeddingProjections(cfg, cwd, { codexHome: path.join(cwd, 'none') }).findings.length, 0); +}); + +test('AQE registration is required only in an AQE-initialized repository', t => { + const { cwd, cfg } = fixture(t); + const opts = { codexHome: path.join(cwd, 'none') }; + const plain = inspectAqeEmbeddingProjections(cfg, cwd, opts); + assert.equal(plain.ok, true, JSON.stringify(plain)); + assert.equal(plain.findings.find(f => f.file.endsWith('.mcp.json')).status, 'absent'); + fs.mkdirSync(path.join(cwd, '.agentic-qe')); + const initialized = inspectAqeEmbeddingProjections(cfg, cwd, opts); + assert.equal(initialized.ok, false); + assert.equal(initialized.findings.find(f => f.file.endsWith('.mcp.json')).status, 'missing-registration'); +}); + +test('a repository subdirectory projects into the repository root', t => { + const { cwd, write, cfg } = fixture(t); + const file = write('.mcp.json', { mcpServers: { 'agentic-qe': { command: 'aqe-mcp' } } }); + const sub = path.join(cwd, 'src', 'deep'); + fs.mkdirSync(sub, { recursive: true }); + const result = reconcileAqeEmbeddingProjections(cfg, sub, { codexHome: path.join(cwd, 'none'), claudeUserFile: path.join(cwd, 'claude-user.json') }); + assert.equal(result.ok, true, JSON.stringify(result)); + assert.equal(JSON.parse(fs.readFileSync(file)).mcpServers['agentic-qe'].env.AQE_EMBEDDER_ENDPOINT, cfg.aqeEmbedding.endpoint); + assert.equal(fs.existsSync(path.join(sub, '.mcp.json')), false); +}); diff --git a/tests/kit/codex-mcp-convergence.test.mjs b/tests/kit/codex-mcp-convergence.test.mjs index 0403e103..0fc7b86c 100644 --- a/tests/kit/codex-mcp-convergence.test.mjs +++ b/tests/kit/codex-mcp-convergence.test.mjs @@ -13,7 +13,8 @@ assertSandboxed(paths, sandbox); const sync = await import('../../src/commands/sync.mjs'); const setup = await import('../../src/commands/setup.mjs'); const { loadKitConfig } = await import('../../src/lib/config.mjs'); -const { codexMcpTopology, repairCodexMcpTopology, register, claudeMcpTopology } = await import('../../src/lib/mcp.mjs'); +const { codexMcpTopology, repairCodexMcpTopology, register, claudeMcpTopology, LEGACY_RUFLO_PLACEHOLDER } = await import('../../src/lib/mcp.mjs'); +const { managedAgentBrowserEnv } = await import('../../src/lib/agent-browser.mjs'); const { ensureRufloMcpInCodex } = await import('../../src/lib/providers.mjs'); const { reconcileCodexMcp } = await import('../../src/lib/codex-mcp-reconcile.mjs'); const project = sandboxProject('ak-mcp-convergence'); @@ -39,20 +40,25 @@ function seed(source = canonical + legacy + unrelated) { removals = 0; } +// The legacy alias is disabled in place (a bounded file edit), never removed +// through `codex mcp remove`: Codex's Claude config import re-adds any Claude +// server whose name Codex lacks, so a deleted alias came straight back. async function repair(plan, cwd) { - return repairCodexMcpTopology(plan, cwd, { + const result = await repairCodexMcpTopology(plan, cwd, { inspect, - runner: async (command, args) => { - assert.equal(command, 'codex'); - assert.deepEqual(args, ['mcp', 'remove', 'claude-flow']); - removals++; - const source = fs.readFileSync(file, 'utf8'); - fs.writeFileSync(file, source.replace(/\[mcp_servers\.claude-flow\][\s\S]*?(?=\[mcp_servers\.|$)/, '')); - return { code: 0, stdout: '', stderr: '' }; - }, + runner: async () => { throw new Error('legacy alias must be disabled in place, not removed through codex mcp'); }, }); + if (result.ok) removals += plan.filter(entry => entry.repairKind === 'legacy-ruflo').length; + return result; } +// An external writer (upstream initializer, Codex's Claude import) turning the +// placeholder back into the live legacy transport. +const restoreLegacy = () => fs.writeFileSync(file, + fs.readFileSync(file, 'utf8').replace(LEGACY_RUFLO_PLACEHOLDER, legacy.trimEnd())); + +const disabledAlias = () => inspect().registrations.find(entry => entry.name === 'claude-flow'); + function rows() { return inspect().duplicateRuflo ? [{ subsystem: 'codex-mcp', level: 'warn', message: 'duplicate Ruflo transports', fix: 'ak sync repairs duplicates' }] @@ -78,8 +84,10 @@ test('sync repairs an approved alias and remembers only the disclosed correction assert.equal(prompts, 1); assert.equal(inspect().duplicateRuflo, false); assert.ok(fs.readFileSync(file, 'utf8').includes(unrelated)); + assert.equal(disabledAlias()?.enabled, false, 'alias stays as a disabled placeholder'); + assert.equal(disabledAlias()?.repairKind, null, 'the placeholder is never itself a repair target'); // An external upgrader restores the same recognized legacy transport. - fs.appendFileSync(file, legacy); + restoreLegacy(); const second = await run({ confirm: async () => { throw new Error('must reuse explicit repair consent'); } }); assert.equal(second.result, 0, second.out); assert.equal(inspect().duplicateRuflo, false); @@ -101,7 +109,7 @@ test('declining repair leaves configuration and consent unchanged', async () => test('remembered repair never consumes an alias with a custom environment', async () => { seed(); assert.equal((await run({ yes: true })).result, 0); - fs.appendFileSync(file, legacy + '[mcp_servers.claude-flow.env]\nPRIVATE_DB = "keep"\n'); + fs.writeFileSync(file, canonical + unrelated + legacy + '[mcp_servers.claude-flow.env]\nPRIVATE_DB = "keep"\n'); const before = fs.readFileSync(file, 'utf8'); const result = await run(); assert.equal(result.result, 1, result.out); @@ -279,3 +287,22 @@ test('native repair refuses a Codex home that differs from the approved config', assert.equal(result.ok, false); assertUnchanged(before, sandbox, 'mismatched native Codex home'); }); + +test('the kit-managed browser env child is folded into the placeholder and Codex import finds the name taken', async () => { + const browser = managedAgentBrowserEnv().AGENT_BROWSER_CONFIG; + seed(canonical + legacy + `\n[mcp_servers.claude-flow.env]\nAGENT_BROWSER_CONFIG = ${JSON.stringify(browser)}\n\n` + unrelated); + assert.equal(inspect().duplicateRuflo, true); + const result = await run({ yes: true }); + assert.equal(result.result, 0, result.out); + const source = fs.readFileSync(file, 'utf8'); + assert.doesNotMatch(source, /mcp_servers\.claude-flow\.env/, 'no orphaned child table'); + assert.ok(source.includes(LEGACY_RUFLO_PLACEHOLDER)); + assert.ok(source.includes(unrelated)); + assert.equal(inspect().duplicateRuflo, false); + // Codex's importer adds a Claude server only when no Codex server has its name. + const importerWouldAdd = !inspect().registrations.some(entry => entry.name === 'claude-flow'); + assert.equal(importerWouldAdd, false); + const again = await run({ confirm: async () => { throw new Error('nothing left to repair'); } }); + assert.equal(again.result, 0, again.out); + assert.equal(fs.readFileSync(file, 'utf8'), source, 'a converged placeholder is left alone'); +}); diff --git a/tests/kit/codex-mcp.test.mjs b/tests/kit/codex-mcp.test.mjs index 6750661c..dbfc315f 100644 --- a/tests/kit/codex-mcp.test.mjs +++ b/tests/kit/codex-mcp.test.mjs @@ -249,8 +249,10 @@ test('Codex MCP repair plans only remove recursive and legacy duplicate entries' 'repair must preserve one current-state recovery copy before mutation'); assert.deepEqual(calls.map(({ command, args }) => [command, args]), [ ['codex', ['mcp', 'remove', 'codex']], - ['codex', ['mcp', 'remove', 'claude-flow']], - ]); + ], 'the legacy alias is disabled in place, not removed through codex mcp'); + const alias = codexMcpTopology({ cwd: dir, home }).registrations.find(({ name }) => name === 'claude-flow'); + assert.equal(alias?.enabled, false); + assert.equal(result.detail, 'removed [mcp_servers.codex], disabled [mcp_servers.claude-flow]'); } finally { rm(dir); rm(home); } }); diff --git a/tests/kit/helper-stamp.test.mjs b/tests/kit/helper-stamp.test.mjs index a681f381..a5050d14 100644 --- a/tests/kit/helper-stamp.test.mjs +++ b/tests/kit/helper-stamp.test.mjs @@ -23,7 +23,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { _setGlobalRootForTest } from '../../src/lib/paths.mjs'; -import { helperStampStale, refreshRufloHelpers, fixStatusline } from '../../src/lib/statusline.mjs'; +import { helperStampStale, refreshRufloHelpers, runHelperRefresh, fixStatusline } from '../../src/lib/statusline.mjs'; // Minimal host: just the shapes fixStatusline keys off, runnable post-injection. const HOST = `#!/usr/bin/env node @@ -65,7 +65,21 @@ const BLOCKED_REFRESH = `export async function autoRefreshHelpersIfStale() { } `; -const REFRESH_BODIES = { faithful: FAKE_REFRESH, throws: THROWING_REFRESH, hangs: HANGING_REFRESH, blocked: BLOCKED_REFRESH }; +// Upstream's no-op: ran unblocked, wrote nothing (stamp current, or no +// hook-handler.cjs in either the project or the global helpers directory). +const NOOP_REFRESH = `export async function autoRefreshHelpersIfStale() { + return { refreshed: false, global: { refreshed: false } }; +} +`; + +// Only the global ~/.claude/helpers pass rewrote anything. +const GLOBAL_REFRESH = `export async function autoRefreshHelpersIfStale() { + return { refreshed: false, global: { refreshed: true } }; +} +`; + +const REFRESH_BODIES = { faithful: FAKE_REFRESH, throws: THROWING_REFRESH, hangs: HANGING_REFRESH, + blocked: BLOCKED_REFRESH, noop: NOOP_REFRESH, global: GLOBAL_REFRESH }; function fixture({ cliVersion = '3.32.7', stamp, refreshModule } = {}) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ak-stamp-')); @@ -73,6 +87,7 @@ function fixture({ cliVersion = '3.32.7', stamp, refreshModule } = {}) { const helpers = path.join(proj, '.claude', 'helpers'); fs.mkdirSync(helpers, { recursive: true }); fs.writeFileSync(path.join(helpers, 'statusline.cjs'), HOST); + fs.writeFileSync(path.join(helpers, 'hook-handler.cjs'), ''); if (stamp !== undefined) fs.writeFileSync(path.join(helpers, '.helpers-version'), stamp); const groot = path.join(dir, 'groot'); @@ -129,6 +144,16 @@ test('missing stamp with a resolvable CLI counts as stale (first refresh pending assert.equal(helperStampStale(proj), true); }); +test('no ruflo helpers (no hook-handler.cjs) → never stale: ruflo refreshes nothing there', () => { + // e.g. `ak sync` run from ~/.claude: no stamp exists, but ruflo's own refresh + // returns early without hook-handler.cjs, so "stale" would be a false alarm. + const { proj } = fixture({ cliVersion: '3.32.7' }); + fs.rmSync(path.join(proj, '.claude', 'helpers', 'hook-handler.cjs')); + assert.equal(helperStampStale(proj), false); + fs.rmSync(path.join(proj, '.claude'), { recursive: true }); + assert.equal(helperStampStale(proj), false); +}); + test('no installed CLI → never stale (nothing exists to refresh anything)', () => { const { proj } = fixture({ cliVersion: null, stamp: '3.32.2' }); assert.equal(helperStampStale(proj), false); @@ -184,3 +209,27 @@ test('dryRun never triggers the refresh (status stays read-only)', () => { fixStatusline(proj, { dryRun: true }); assert.equal(fs.existsSync(path.join(proj, 'REFRESHED')), false); }); + +// --- runHelperRefresh: a no-op must never read as a heal ---------------------- + +test('runHelperRefresh distinguishes refreshed, current and failed', () => { + assert.equal(runHelperRefresh(fixture({ refreshModule: 'faithful' }).proj), 'refreshed'); + assert.equal(runHelperRefresh(fixture({ refreshModule: 'global' }).proj), 'refreshed'); + assert.equal(runHelperRefresh(fixture({ refreshModule: 'noop' }).proj), 'current'); + assert.equal(runHelperRefresh(fixture({ refreshModule: 'blocked' }).proj), 'failed'); + assert.equal(runHelperRefresh(fixture({ refreshModule: 'throws' }).proj), 'failed'); + assert.equal(runHelperRefresh(fixture({}).proj), 'failed'); + // The boolean wrapper keeps its "ran unblocked" contract. + assert.equal(refreshRufloHelpers(fixture({ refreshModule: 'noop' }).proj), true); +}); + +test('fixStatusline marks a location without ruflo helpers as absent, not a defect', () => { + const { proj } = fixture({}); + fs.rmSync(path.join(proj, '.claude'), { recursive: true }); + const r = fixStatusline(proj); + assert.equal(r.applied, false); + assert.equal(r.absent, true); + const { proj: partial, sl } = fixture({}); + fs.rmSync(sl); + assert.equal(fixStatusline(partial).absent, false, 'helpers without statusline.cjs stays a warning'); +});