diff --git a/CHANGELOG.md b/CHANGELOG.md index 3506960..e10b30b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,9 +68,57 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - **Safer installer transactions (HI-10).** `install.sh` backs up a user-owned symlink at a destination instead of silently replacing it, and stops (`Uninstall INCOMPLETE`, non-zero) rather than removing assets that settings hooks still reference when settings cleanup fails. +- **Verification surfaces monorepo suites (ME-03).** `detectStack()` now reports `workspaces` + (declared workspace globs) and `packageRoots` (nested package roots found via a bounded, + capped scan), so a root test command can no longer silently claim to cover a whole + npm/pnpm/Turborepo/Maven/Gradle/Python monorepo. +- **Evidence resolution strength gates confidence (ME-05).** `ci:`/`human:`/`file:` refs are + format- and existence-checked, and unresolved pointers like `test:` or a non-existent + `file:` path can no longer lift a claim's confidence into the trusted/serving band; a + resolvable git-ref confirmation still counts exactly as before. +- **Secret-bearing ledger metadata is refused (ME-06).** Secret-shaped evidence refs, authors, + tombstone reasons, and provenance metadata are refused before they can be written to the + ledger — the same detector already applied to claim content — and quarantined records are + stored redacted. +- **Trusted quarantine identity (ME-07).** Quarantine keys rejected records by a trusted content + hash instead of the record's own (attacker-controllable) hash, so distinct forgeries sharing a + fake hash are both retained and malformed lines with no hash are captured instead of dropped. +- **Per-target MCP ownership and atomic add/remove (ME-08, ME-10).** Adoption is tracked per + `{server, target}`, so adopting a same-name server for one tool never authorizes overwriting + another tool's entry (legacy `adopted: [name]` still honored and migrated); `integrations add` + records nothing on a partial emit and `remove` keeps the entry (reporting an incomplete + transaction) if any target cleanup fails, so config and disk never drift silently. +- **Divergent registry-name MCP entries preserved (ME-09).** A user's own MCP server sharing a + built-in name (e.g. `forge-cortex`) is no longer overwritten without explicit `--adopt`. +- **Validated MCP names and safe serialization (ME-11).** Managed server names must match + `^[a-z0-9][a-z0-9_-]{0,63}$`, `foo`/`forge-foo` Continue-file collisions are rejected before + any write, and command strings are quoted safely into YAML and TOML. +- **Non-object JSON is treated as corrupt (ME-12).** Repo config and global settings now reject + valid-but-non-object JSON (`null`, `[]`, `"x"`, `42`) as corrupt instead of silently + overwriting it, preserving the original bytes. +- **Crash-safe config writes (ME-13).** `forge.config.json` writes take a timestamped backup and + go through a temp file + atomic rename. +- **Complete tool list (ME-14).** `roo` is now a selectable primary tool, and + aider/continue/windsurf/roo are auto-detected from their on-disk markers, reconciling + `forge tools` with the MCP emit targets. +- **Functional doctor health probes (ME-15, ME-16, ME-17, ME-18).** `forge doctor` validates the + actual Forge hook wiring against the template (not just the `_forge` marker), verifies + `~/.forge` is a symlink/dir whose required guard assets resolve (catching plain-file shadows + and dangling links), runs a real redactor self-test instead of trusting Node's presence, and + records a partial `--fix` sync (nested `action:"error"` rows) as a failed repair. +- **Explicit PARTIAL sync status (ME-19).** `forge sync` returns and reports an aggregate status; + if any target fails mid-run the result is `PARTIAL` and `sync`/`init` say so and exit non-zero + instead of implying every tool is configured. +- **Fail-safe legacy rules (ME-20).** A corrupt legacy `.forge/rules.json` no longer aborts + `forge sync` — it warns once, falls back to default rules, and leaves the file's bytes intact. +- **Exact gitattributes rule detection (ME-21).** The ledger union-merge rule is detected by its + exact active line, so a comment mentioning `.forge/ledger/` no longer suppresses the real rule. ### Changed +- **Disclose the global settings merge before it happens (ME-22).** `forge init` now prints the + global `~/.claude/settings.json` merge disclosure before the merge mutates the file, not after. + - **Honest positioning (ME-24).** README no longer claims "one brain for every AI coding agent", "enforced guardrails", or that every task passes a deterministic gate: automatic hooks are Claude Code-specific (advisory by default, enforcement opt-in via diff --git a/docs/GUIDE.md b/docs/GUIDE.md index a06d765..7ce123f 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -570,9 +570,10 @@ tools` fixes that without changing what `sync` emits. - `forge tools` — show the detected/primary tool (from `.forge/config.json`, else auto-detected from which agent folder exists — `CLAUDE.md`, `.cursor/`, `.gemini/`, - `.codex/`, `.zed/`, `.vscode/`) and which targets are currently gitignored. + `.codex/`, `.zed/`, `.vscode/`, `.aider.conf.yml`, `.continue/`, `.windsurf/`, `.roo/`) + and which targets are currently gitignored. - `forge tools ` — record `` (`claude` · `cursor` · `gemini` · `codex` · - `zed` · `vscode` · `aider` · `continue` · `windsurf`) as this repo's primary tool in + `zed` · `vscode` · `aider` · `continue` · `windsurf` · `roo`) as this repo's primary tool in `.forge/config.json`, then write a **marked, reversible** block into `.gitignore` (`# forge:gitignore:begin … # forge:gitignore:end`) that ignores every OTHER tool's emitted artifacts. Your own `.gitignore` lines are never touched, and the shared diff --git a/src/cli.js b/src/cli.js index f50587f..4754714 100755 --- a/src/cli.js +++ b/src/cli.js @@ -124,22 +124,23 @@ async function run(argv) { // --settings-only: wire hooks + permissions into ~/.claude/settings.json ONLY (the // idempotent, marker-guarded merge install.sh calls) — no repo emit, no AGENTS.md. if (argv.includes("--settings-only")) { + // ME-22: heading + disclosure precede the merge — onSettingsNotice fires inside init + // BEFORE mergeSettings touches the file, never after. + heading(`${BRAND.brand} init — settings merge only\n`); const { settings } = init({ settingsOnly: true, noSettings, settingsPath, + onSettingsNotice: consentLine, }); - heading(`${BRAND.brand} init — settings merge only\n`); if ( (settings?.action === "merged" || settings?.action === "created") && "added" in settings ) { - consentLine(settings.path); const verb = settings.action === "created" ? "created" : "merged"; const what = settings.added.length ? settings.added.join(", ") : "defaults"; console.log(` settings: ${verb} ${what} into ${settings.path}`); } else if (settings?.action === "unchanged" && "path" in settings) { - consentLine(settings.path); console.log(` settings: already up to date (${settings.path})`); } else if (settings?.action === "skipped") { console.log(" settings: skipped (--no-settings)"); @@ -153,6 +154,10 @@ async function run(argv) { } const profileIdx = argv.indexOf("--profile"); const profile = profileIdx >= 0 ? argv[profileIdx + 1] : undefined; + // ME-22: emit the GLOBAL-settings disclosure BEFORE the merge mutates the file. init + // forwards `onSettingsNotice` to mergeSettings, which fires it right before touching + // ~/.claude/settings.json — so the notice always precedes the mutation it describes. + heading(`${BRAND.brand} init — this repo now speaks every AI tool from one source.\n`); const { report, bytes, @@ -165,6 +170,7 @@ async function run(argv) { noSettings, profile, settingsPath, + onSettingsNotice: consentLine, }) ); if (profileResult?.error) { @@ -173,11 +179,21 @@ async function run(argv) { return; } const wrote = report.filter((r) => r.action === "written").map((r) => r.target); - heading(`${BRAND.brand} init — this repo now speaks every AI tool from one source.\n`); console.log(` emitted: ${wrote.length ? wrote.join(", ") : "(all up to date)"}`); console.log( ` source: AGENTS.md (${bytes} B) — edit rules in source/, re-run \`${BRAND.cli} sync\``, ); + // ME-19: sync is non-transactional — if a target failed mid-emit, say so instead of + // implying every tool is ready, and fail the command. + const failedTargets = report.filter((r) => r.action === "error"); + if (failedTargets.length) { + console.error( + ` status: PARTIAL — ${failedTargets.length} target(s) failed: ${failedTargets + .map((r) => r.tool) + .join(", ")} (re-run \`${BRAND.cli} sync\` after fixing)`, + ); + process.exitCode = 1; + } if (profileResult?.profile) { console.log(` profile: ${profileResult.profile} → .forge/forge.config.json`); if (profileResult.deprecated) { @@ -187,14 +203,12 @@ async function run(argv) { } } if ((settings?.action === "merged" || settings?.action === "created") && "added" in settings) { - consentLine(settings.path); const verb = settings.action === "created" ? "created" : "merged"; const what = settings.added.length ? settings.added.join(", ") : "defaults"; console.log(` settings: ${verb} ${what} into ${settings.path}`); if ("backup" in settings && settings.backup) console.log(` backup: ${settings.backup}`); } else if (settings?.action === "unchanged" && "path" in settings) { - consentLine(settings.path); console.log(` settings: already up to date (${settings.path})`); } else if (settings?.action === "skipped") { console.log(" settings: skipped (--no-settings)"); @@ -377,7 +391,9 @@ async function run(argv) { } if (cmd === "sync") { const { sync } = await import("./sync.js"); - const { report, warnings, bytes } = sync({ targetRoot: process.cwd() }); + const { report, warnings, bytes, partial, status } = sync({ + targetRoot: process.cwd(), + }); heading(`${BRAND.brand} sync — one source → every tool\n`); for (const r of report) { console.log( @@ -386,7 +402,15 @@ async function run(argv) { } for (const w of warnings) console.warn(` ! ${w}`); const written = report.filter((r) => r.action === "written").length; - console.log(`\n${written} file(s) written · canonical ${bytes} B`); + console.log(`\n${written} file(s) written · canonical ${bytes} B · status: ${status}`); + // ME-19: a mid-way target failure must NOT exit 0 as if every tool were configured. + if (partial) { + const failed = report.filter((r) => r.action === "error"); + console.error( + ` ! PARTIAL sync — ${failed.length} target(s) failed: ${failed.map((r) => r.tool).join(", ")}`, + ); + process.exitCode = 1; + } return; } if (cmd === "doctor") { diff --git a/src/doctor.js b/src/doctor.js index dfea165..c703691 100644 --- a/src/doctor.js +++ b/src/doctor.js @@ -1,12 +1,15 @@ // forge doctor — turn silent misconfiguration into an actionable pass/fail list // (chezmoi-doctor pattern). Exits non-zero only on hard failures, not warnings. +import { execFileSync } from "node:child_process"; import { accessSync, chmodSync, constants, existsSync, + lstatSync, readdirSync, readFileSync, + realpathSync, statSync, } from "node:fs"; import { homedir } from "node:os"; @@ -17,7 +20,7 @@ import { summary as cortexSummary } from "./cortex.js"; import { docsCheck } from "./docs_check.js"; import { hashContent, mdHeader } from "./emit/_shared.js"; import { gatewayBase, gatewayModelMap } from "./gateway_model_map.js"; -import { ensureLedgerGitattributes, mergeSettings } from "./init.js"; +import { ensureLedgerGitattributes, guardKey, mergeSettings } from "./init.js"; import { verify as ledgerVerify, repoLedger } from "./ledger_store.js"; import { PRICING_VERIFIED } from "./model_tiers.js"; import { activeProvider, envModelOverride } from "./providers.js"; @@ -42,30 +45,82 @@ const readJsonSafe = (p) => { } }; +// The REQUIRED Forge hook guard identities, derived from the settings template — the SAME +// keys `mergeSettings` installs. `guardKey` normalizes every command (quoting + the legacy +// `~/.forge/` prefix) down to `basename.sh args`, so the template's `~/.forge/guards/cortex.sh +// prompt` and an INSTALLED, path-resolved `bash '/x/global/guards/cortex.sh' prompt` both +// reduce to `cortex.sh prompt` — the raw template compares cleanly against a resolved install +// without re-running resolveManagedPaths. A failed template load returns [] (fail-open). +function templateGuardKeys() { + try { + const tpl = readJson(join(BRAND.root, "global", "settings.template.json")); + const keys = new Set(); + for (const entries of Object.values(tpl.hooks || {})) { + for (const entry of Array.isArray(entries) ? entries : []) { + for (const h of entry?.hooks || []) if (h?.command) keys.add(guardKey(h.command)); + } + } + return [...keys]; + } catch { + return []; + } +} + +// Every guard identity actually wired into a settings file's hook tree (quote-normalized). +function installedGuardKeys(hooks) { + const keys = new Set(); + if (hooks && typeof hooks === "object") { + for (const entries of Object.values(hooks)) { + for (const entry of Array.isArray(entries) ? entries : []) { + for (const h of entry?.hooks || []) + if (typeof h?.command === "string") keys.add(guardKey(h.command)); + } + } + } + return keys; +} + // The user's ~/.claude/settings.json must carry Forge's hooks + permissions or none of the -// session-start rehydrate / advisory hooks fire — the silent-onboarding failure. Fixable by -// re-running the same idempotent merge init uses (mergeSettings), marker-guarded so it never -// clobbers hand-written entries. +// session-start rehydrate / advisory hooks fire — the silent-onboarding failure. A truthy +// `_forge` marker alone is FALSE-GREEN (ME-15): a file with the marker and one unrelated hook +// would pass. So verify the ACTUAL wiring — that every REQUIRED Forge guard identity from the +// template is present AND permissions are installed — not just that the marker exists. Fixable +// by re-running the same idempotent merge init uses (`mergeSettings`), marker-guarded so it +// never clobbers hand-written entries. function checkSettings(out, settingsPath) { const path = settingsPath || join(homedir(), ".claude", "settings.json"); const data = readJsonSafe(path); - const managed = !!data?._forge; - const hasHooks = !!(data?.hooks && Object.keys(data.hooks).length); - if (managed && hasHooks) { - out.push(ok("settings", "forge-managed hooks + permissions present")); + const fix = { + id: "settings", + label: "merge forge hooks + permissions into settings.json", + run: () => mergeSettings({ settingsPath }), + }; + if (!data) { + out.push({ + ...warn("settings", "missing — run `forge doctor --fix` or `forge init`"), + fix, + }); return; } - const note = data - ? "not forge-managed — hooks/permissions missing; run `forge doctor --fix` or `forge init`" - : "missing — run `forge doctor --fix` or `forge init`"; - out.push({ - ...warn("settings", note), - fix: { - id: "settings", - label: "merge forge hooks + permissions into settings.json", - run: () => mergeSettings({ settingsPath }), - }, - }); + const required = templateGuardKeys(); + const installed = installedGuardKeys(data.hooks); + const missing = required.filter((k) => !installed.has(k)); + const perms = data.permissions; + const hasPerms = !!(perms && (perms.allow?.length || perms.deny?.length || perms.ask?.length)); + // ACTIVE only when EVERY required guard identity is wired AND permissions are present — + // a stale/partial install (marker set, guards missing) reports DEGRADED, not green. + if (required.length && missing.length === 0 && hasPerms) { + out.push( + ok("settings", `forge-managed — ${required.length} hook guard(s) + permissions wired`), + ); + return; + } + const note = missing.length + ? `forge hooks missing/stale (${missing.length}/${required.length} guard(s) absent) — run \`forge doctor --fix\` or \`forge init\`` + : !hasPerms + ? "forge permissions missing — run `forge doctor --fix` or `forge init`" + : "not forge-managed — run `forge doctor --fix` or `forge init`"; + out.push({ ...warn("settings", note), fix }); } // External tools the guards/commands depend on. secret-redact now runs in Node (no jq), @@ -290,15 +345,138 @@ function checkPluginCompatibility(out) { } } -function checkInstall(out) { - const forgeHome = join(homedir(), ".forge"); +// The guard assets install.sh symlinks `~/.forge` at MUST resolve, or every hook that shells +// `~/.forge/guards/*.sh` silently no-ops. Existence alone is false-green (ME-16): a plain file +// named `~/.forge`, or a dangling symlink left by a moved/removed checkout, both "exist". So +// verify it is a symlink (or the expected install dir) whose contents include the required +// guard files, that they are readable and (for the `.sh` launchers) executable. +const REQUIRED_INSTALL_ASSETS = [ + join("guards", "protect-paths.sh"), + join("guards", "secret-redact.sh"), + join("guards", "secret-redact.mjs"), +]; + +function checkInstall(out, forgeHomeOverride) { + const forgeHome = forgeHomeOverride || join(homedir(), ".forge"); + let lst; + try { + lst = lstatSync(forgeHome); + } catch { + // Genuinely absent — UNAVAILABLE, not a failure (an uninstalled Forge is not broken). + out.push(na("~/.forge", "not installed — run install.sh or the plugin")); + return; + } + if (lst.isSymbolicLink()) { + // A symlink that lstat sees but realpath can't resolve is dangling (moved/removed checkout). + try { + realpathSync(forgeHome); + } catch { + out.push(fail("~/.forge", "dangling symlink — target is gone; re-run install.sh")); + return; + } + } else if (!lst.isDirectory()) { + out.push( + fail( + "~/.forge", + "present but not a symlink or directory — a stray file shadows the install; re-run install.sh", + ), + ); + return; + } + const missing = []; + const unreadable = []; + const notExec = []; + for (const rel of REQUIRED_INSTALL_ASSETS) { + const abs = join(forgeHome, rel); + if (!existsSync(abs)) { + missing.push(rel); + continue; + } + try { + accessSync(abs, constants.R_OK); + } catch { + unreadable.push(rel); + } + // The `.sh` launchers are run as `bash …` from hooks but must be executable when invoked + // directly; the `.mjs` is run via `node file` so only needs to be readable. + if (rel.endsWith(".sh")) { + try { + accessSync(abs, constants.X_OK); + } catch { + notExec.push(rel); + } + } + } + if (missing.length || unreadable.length || notExec.length) { + const parts = []; + if (missing.length) parts.push(`${missing.length} missing (${missing.join(", ")})`); + if (unreadable.length) parts.push(`${unreadable.length} unreadable`); + if (notExec.length) parts.push(`${notExec.length} not executable`); + out.push(fail("~/.forge", `install incomplete — ${parts.join("; ")}; re-run install.sh`)); + return; + } out.push( - existsSync(forgeHome) - ? ok("~/.forge", "linked") - : warn("~/.forge", "not installed — run install.sh or the plugin"), + ok( + "~/.forge", + lst.isSymbolicLink() ? "linked — guard assets resolve" : "installed — guard assets resolve", + ), ); } +// Node presence does NOT prove the redactor works (ME-17): the PostToolUse secret-redact guard +// can be missing, or produce the wrong shape, and still leave `node` reporting green. Run a +// SAFE, side-effect-free self-test — feed a clearly-FAKE, runtime-assembled credential (never a +// literal in source, so nothing secret-shaped is committed or trips push-protection) through the +// REAL `secret-redact.mjs` and assert it both redacts the secret and preserves the output shape. +// ACTIVE only when the round-trip passes; DEGRADED (warn) when node is absent, the redactor is +// missing, or the output is wrong — never silently ACTIVE. +function checkRedaction(out, guardsDirOverride) { + const guardsDir = guardsDirOverride || join(BRAND.root, "global", "guards"); + const mjs = join(guardsDir, "secret-redact.mjs"); + if (!existsSync(mjs)) { + out.push( + warn("secret-redact", "redactor script missing — tool output is NOT scanned for secrets"), + ); + return; + } + if (!hasBin("node")) { + out.push(warn("secret-redact", "node not found — secret-redact CANNOT run; output unscanned")); + return; + } + // Assembled at runtime (same approach as test/_fixtures.js) — the canonical Anthropic key + // shape the redactor is proven to mask, with no secret-format literal left in this source. + const fake = ["sk", "ant", "api03", "AAAABBBBCCCCDDDDEEEE"].join("-"); + const payload = JSON.stringify({ tool_response: `token ${fake} end` }); + try { + const stdout = execFileSync(process.execPath, [mjs], { + input: payload, + encoding: "utf8", + timeout: 5000, + stdio: ["pipe", "pipe", "ignore"], + }); + const red = JSON.parse(stdout)?.hookSpecificOutput?.updatedToolOutput; + // Shape preserved (a string stays a string, surrounding text intact) AND the secret gone. + const passed = + typeof red === "string" && + red.includes("[REDACTED]") && + !red.includes(fake) && + red.startsWith("token ") && + red.endsWith(" end"); + out.push( + passed + ? ok("secret-redact", "self-test passed — fake secret redacted, output shape preserved") + : warn( + "secret-redact", + "self-test FAILED — redactor ran but did not redact/reshape; tool output may leak secrets", + ), + ); + } catch (err) { + out.push( + warn("secret-redact", `self-test could not run (${err?.message ?? err}); output unscanned`), + ); + } +} + function checkDrift(out, targetRoot) { const syncFix = { id: "agents", @@ -478,7 +656,9 @@ function checkUpdate(out) { // The important subsystems and the check label each derives its health from. const HEALTH_SUBSYSTEMS = { - "secret-redaction": "node", // secret-redact runs the JS redactor; no node → FAILED + // Functional self-test row (ME-17), not the generic node check — node presence alone never + // proves the redactor actually masks a secret. ACTIVE only when the round-trip passes. + "secret-redaction": "secret-redact", guards: "guards exec", atlas: "atlas", "managed-config": "AGENTS.md", @@ -507,7 +687,13 @@ export function subsystemHealth(results) { return health; } -function runChecks(targetRoot, settingsPath) { +/** + * @param {string} targetRoot + * @param {string|undefined} settingsPath + * @param {{forgeHome?: string, guardsDir?: string}} [probes] test seams for the install-dir + * and guards-dir probes (default to the real `~/.forge` and the packaged guards). + */ +function runChecks(targetRoot, settingsPath, { forgeHome, guardsDir } = {}) { const results = []; checkNode(results); checkSettings(results, settingsPath); @@ -518,7 +704,8 @@ function runChecks(targetRoot, settingsPath) { checkGuardsExecutable(results); checkPluginCompatibility(results); checkTooling(results); - checkInstall(results); + checkRedaction(results, guardsDir); + checkInstall(results, forgeHome); checkDrift(results, targetRoot); checkDocs(results, targetRoot); checkAtlas(results, targetRoot); @@ -530,31 +717,65 @@ function runChecks(targetRoot, settingsPath) { return results; } +// A repair whose top-level result — or ANY nested emitter/report row — reports failure must +// NOT be recorded as ok (ME-18, building on RA-20). `mergeSettings` fails via a returned +// `{action:"error", reason}`; `writeForgeConfig`-style helpers via `{ok:false, reason}`; and a +// partial `sync` succeeds overall while individual `report` rows carry `action:"error"` (a +// per-tool emit threw). Returns a reason string when the repair failed, else null. +export function repairFailure(detail) { + if (!detail || typeof detail !== "object") return null; + if (detail.action === "error") return detail.reason || detail.note || "repair reported an error"; + if (detail.ok === false) return detail.reason || "repair returned ok:false"; + const rows = []; + const collect = (v) => { + if (Array.isArray(v)) for (const x of v) collect(x); + else if (v && typeof v === "object") rows.push(v); + }; + for (const key of ["report", "results", "rows", "emitted", "reports"]) collect(detail[key]); + const errored = rows.filter((r) => r && (r.action === "error" || r.ok === false)); + if (!errored.length) return null; + const reasons = errored + .slice(0, 3) + .map( + (r) => + `${r.tool || r.name || r.target || "step"}: ${r.note || r.reason || r.error || "error"}`, + ); + return `${errored.length} sub-step(s) failed — ${reasons.join("; ")}`; +} + /** * Health-check this repo + the user's config. With `fix:true`, each warn/fail result carrying * a `{id,label,run}` descriptor has its idempotent repair run (mergeSettings / ensureLedger- * gitattributes / sync / chmod — all safe, no-op if already applied), then every check re-runs * so the returned `results` reflect the repaired state. Unsafe findings (provider keys, MCP, * pricing, gateway) carry no descriptor and stay report-only. - * @param {{targetRoot?: string, fix?: boolean, settingsPath?: string}} [opts] + * @param {{targetRoot?: string, fix?: boolean, settingsPath?: string, forgeHome?: string, guardsDir?: string}} [opts] */ -export function doctor({ targetRoot = process.cwd(), fix = false, settingsPath } = {}) { - let results = runChecks(targetRoot, settingsPath); +export function doctor({ + targetRoot = process.cwd(), + fix = false, + settingsPath, + forgeHome, + guardsDir, +} = {}) { + const probes = { forgeHome, guardsDir }; + let results = runChecks(targetRoot, settingsPath, probes); const repairs = []; if (fix) { for (const r of results) { if ((r.status === "warn" || r.status === "fail") && r.fix) { try { const detail = r.fix.run(); - // Repairs like mergeSettings report failure as a RETURNED {action:"error", reason} - // instead of throwing — an errored result must not be recorded as ok (RA-20). - const errored = detail && typeof detail === "object" && detail.action === "error"; + // A repair fails not only when it throws or returns {action:"error"} (RA-20), but also + // when a NESTED report row reports an error (a partial sync) — repairFailure catches + // both, so a partially-errored repair is never recorded as success (ME-18). + const failure = repairFailure(detail); repairs.push({ id: r.fix.id, label: r.fix.label, - ok: !errored, - error: errored ? detail.reason : undefined, - detail: errored ? undefined : detail, + ok: !failure, + error: failure || undefined, + detail: failure ? undefined : detail, }); } catch (err) { repairs.push({ @@ -566,7 +787,7 @@ export function doctor({ targetRoot = process.cwd(), fix = false, settingsPath } } } } - results = runChecks(targetRoot, settingsPath); + results = runChecks(targetRoot, settingsPath, probes); } return { results, diff --git a/src/emit/mcp.js b/src/emit/mcp.js index 7830371..e3b4e07 100644 --- a/src/emit/mcp.js +++ b/src/emit/mcp.js @@ -1,10 +1,19 @@ // Emit the managed MCP server set into each tool's MCP config, in that tool's REAL -// format (all verified 2026-07). Non-destructive by construction (RA-03, RA-21): -// every write is scoped to an entry/block/file forge OWNS — a name in the persistent -// managed∪adopted record (see integrations.js) or a region carrying a forge marker. -// Anything else is user-owned and is reported, never rewritten. JSON tools differ only -// by their top-level key; Codex is TOML (forge-marked blocks) and Continue gets one -// forge-marked YAML file per managed server. (Windsurf is global-only — nothing to emit.) +// format (all verified 2026-07). Non-destructive by construction (RA-03, RA-21, ME-08): +// every write is scoped to an entry/block/file forge OWNS — a PER-TARGET adoption record +// (see integrations.js) or a region carrying a forge marker. Anything else is user-owned +// and is reported, never rewritten. JSON tools differ only by their top-level key; Codex +// is TOML (forge-marked blocks) and Continue gets one forge-marked YAML file per managed +// server. (Windsurf is global-only — nothing to emit.) +// +// Ownership is decided PER TARGET (ME-08): the caller supplies `owns(target, name)`, so a +// same-name entry adopted for one tool's config never authorises overwriting another +// tool's same-name entry. Registry names are NOT implicitly owned (ME-09): a divergent +// on-disk entry that forge did not write is preserved and reported, exactly like any other +// same-name collision — forge only refreshes what it can prove it wrote (byte-identical to +// its own spec, a forge marker, or an explicit adoption record). Every managed NAME is +// validated against a strict grammar and Continue filenames are checked for injectivity +// BEFORE any write (ME-11), and command strings are serialised safely into YAML/TOML. import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { BRAND } from "../brand.js"; @@ -22,18 +31,63 @@ const JSON_TARGETS = [ const CONTINUE_DIR = join(".continue", "mcpServers"); const LEGACY_CONTINUE_FILE = "forge-mcp.yaml"; // pre-RA-03 combined file const CODEX_FILE = join(".codex", "config.toml"); +/** The Codex config's canonical relative target string (matches JSON_TARGETS `file`s). */ +export const CODEX_TARGET = ".codex/config.toml"; + +/** Every relative config path forge may own an entry in — the domain of `owns(target,…)` + * and of the per-target adoption record. Continue files are owned by marker, not record. */ +export const MCP_TARGET_FILES = [...JSON_TARGETS.map((t) => t.file), CODEX_TARGET]; const adoptHint = (name) => `adopt with: ${BRAND.cli} integrations add ${name} --adopt`; -/** Per-server Continue file name. Servers already namespaced `forge-*` keep their name. */ +// --------------------------------------------------------------------------- +// Name/definition validation (ME-11). Runs before any write. +// --------------------------------------------------------------------------- + +/** Strict server-name grammar. Rejects path traversal (`../x`), whitespace/newlines, + * TOML/YAML-special punctuation (`.`, `]`, `:`, `#`, …) and uppercase — so a validated + * name is a safe TOML bare key, a safe YAML key, and a safe filename component. */ +const NAME_RE = /^[a-z0-9][a-z0-9_-]{0,63}$/; + +/** Throw unless `name` satisfies the strict grammar. */ +export function validateServerName(name) { + if (typeof name !== "string" || !NAME_RE.test(name)) + throw new Error( + `invalid MCP server name: ${JSON.stringify(name)} — must match ${String(NAME_RE)}`, + ); +} + +/** Validate the whole managed set before emitting: every name must satisfy the grammar, + * and no two names may collide on the same Continue filename (`foo` vs `forge-foo`). */ +function validateServers(servers) { + const byFile = new Map(); + for (const name of Object.keys(servers)) { + validateServerName(name); + const file = continueFileFor(name); + if (byFile.has(file)) + throw new Error( + `MCP server names ${JSON.stringify(byFile.get(file))} and ${JSON.stringify(name)} ` + + `collide on Continue file ${file} — rename one`, + ); + byFile.set(file, name); + } +} + +/** Serialise an arbitrary string as a YAML flow scalar. A JSON double-quoted string is a + * valid YAML double-quoted scalar, so this safely quotes commands containing `:`, `#`, + * leading indicators, newlines, etc. */ +const yamlScalar = (s) => JSON.stringify(String(s)); + +/** Per-server Continue file name. Servers already namespaced `forge-*` keep their name. + * Injectivity across a managed set is enforced by validateServers (`foo`/`forge-foo`). */ export const continueFileFor = (name) => name.startsWith("forge-") ? `${name}.yaml` : `forge-${name}.yaml`; // --------------------------------------------------------------------------- -// JSON targets — entry-level ownership (RA-21). +// JSON targets — entry-level ownership (RA-21, ME-08). // --------------------------------------------------------------------------- -function mergeJson(path, key, servers, owned) { +function mergeJson(path, key, servers, owns) { let obj = {}; if (existsSync(path)) { try { @@ -54,7 +108,7 @@ function mergeJson(path, key, servers, owned) { bucket[name] = def; changed += 1; } else if (JSON.stringify(cur) !== JSON.stringify(def)) { - if (owned.has(name)) { + if (owns(name)) { bucket[name] = def; changed += 1; } else { @@ -70,8 +124,12 @@ function mergeJson(path, key, servers, owned) { ? { action: "skipped", note: skipNote } : { action: "unchanged", note: "present" }; } - mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, `${JSON.stringify(obj, null, 2)}\n`); + try { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, `${JSON.stringify(obj, null, 2)}\n`); + } catch (err) { + return { action: "error", note: `write failed: ${err.message}` }; + } const note = `${changed} server(s) written/updated${skipNote ? `; ${skipNote}` : ""}`; return { action: "written", note }; } @@ -142,7 +200,7 @@ function unmarkedRegion(text, name) { return { start: idx, end }; } -function emitCodexToml(path, servers, owned) { +function emitCodexToml(path, servers, owns) { let text = existsSync(path) ? readFileSync(path, "utf8") : ""; let changed = 0; const notes = []; @@ -164,10 +222,10 @@ function emitCodexToml(path, servers, owned) { const unmarked = unmarkedRegion(text, name); if (unmarked) { const cur = text.slice(unmarked.start, unmarked.end); - // Migration/ownership for a pre-existing plain block: claim it only when the name - // is in the owned record OR the block byte-matches a past forge emission. Anything - // else is the user's TOML — leave it byte-identical. - if (owned.has(name) || cur.trimEnd() === legacyCodexBlock(name, def).trimEnd()) { + // Migration/ownership for a pre-existing plain block: claim it only when this target + // adopted the name OR the block byte-matches a past forge emission. Anything else is + // the user's TOML — leave it byte-identical. + if (owns(name) || cur.trimEnd() === legacyCodexBlock(name, def).trimEnd()) { text = text.slice(0, unmarked.start) + block + text.slice(unmarked.end); changed += 1; notes.push(`${name}: adopted unmarked block (forge markers added)`); @@ -186,8 +244,12 @@ function emitCodexToml(path, servers, owned) { note: notes.join("; ") || "present", }; } - mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, text); + try { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, text); + } catch (err) { + return { action: "error", note: `write failed: ${err.message}` }; + } return { action: "written", note: [`${changed} managed block(s)`, ...notes].join("; "), @@ -216,7 +278,7 @@ function continueBody(name, def) { "mcpServers:", ` - name: ${name}`, " type: stdio", - ` command: ${def.command}`, + ` command: ${yamlScalar(def.command)}`, " args:", ]; for (const a of def.args || []) lines.push(` - ${JSON.stringify(a)}`); @@ -229,8 +291,12 @@ function emitContinueServer(dir, name, def) { if (existing !== null && !isManaged(existing)) return { action: "skipped", note: "existing unmanaged file" }; const body = continueBody(name, def); - const action = writeManaged(path, yamlHeader(hashContent(body)), body); - return { action, note: "per-server YAML" }; + try { + const action = writeManaged(path, yamlHeader(hashContent(body)), body); + return { action, note: "per-server YAML" }; + } catch (err) { + return { action: "error", note: `write failed: ${err.message}` }; + } } /** Migrate away the pre-RA-03 combined forge-mcp.yaml once per-server files exist. @@ -258,16 +324,20 @@ function migrateLegacyContinue(dir) { // --------------------------------------------------------------------------- /** - * Emit `servers` (the full managed set) into every target. `owned` is the set of names - * forge may OVERWRITE when a same-name entry already exists and drifted — the persistent - * managed∪adopted record plus the built-in registry names. Absent entries are always - * written; entries outside `owned` are preserved and reported. + * Emit `servers` (the full managed set) into every target. `owns(target, name)` decides, + * PER TARGET, whether forge may OVERWRITE an already-present same-name entry that drifted + * (ME-08): the default owns nothing implicitly, so absent entries are always written but a + * pre-existing divergent entry is preserved and reported unless that exact target adopted + * the name. Every name is validated and Continue filenames checked for collisions before + * any write (ME-11); a per-target write failure surfaces as an `error` row (never a throw) + * so callers can keep disk and the managed-set record consistent (ME-10). * @param {{targetRoot:string, servers:Record, - * owned?:Set}} opts + * owns?:(target:string, name:string)=>boolean}} opts */ -export function emitMcp({ targetRoot, servers, owned = new Set(Object.keys(servers)) }) { +export function emitMcp({ targetRoot, servers, owns = () => true }) { + validateServers(servers); const rows = JSON_TARGETS.map((t) => { - const r = mergeJson(join(targetRoot, t.file), t.key, servers, owned); + const r = mergeJson(join(targetRoot, t.file), t.key, servers, (name) => owns(t.file, name)); return { tool: `${t.tool} MCP`, target: t.file, @@ -275,10 +345,12 @@ export function emitMcp({ targetRoot, servers, owned = new Set(Object.keys(serve note: r.note, }; }); - const codex = emitCodexToml(join(targetRoot, CODEX_FILE), servers, owned); + const codex = emitCodexToml(join(targetRoot, CODEX_FILE), servers, (name) => + owns(CODEX_TARGET, name), + ); rows.push({ tool: "Codex MCP", - target: ".codex/config.toml", + target: CODEX_TARGET, action: codex.action, note: codex.note, }); @@ -306,16 +378,17 @@ export function emitMcp({ targetRoot, servers, owned = new Set(Object.keys(serve /** * Reverse one server's emission. JSON entries are deleted only when `removeJsonEntry` - * says forge owns them (adopted, or byte-identical to forge's own spec); the Codex block - * only when forge-marked; the Continue file only when forge-managed. Idempotent. + * says forge owns them for THAT target (adopted, or byte-identical to forge's own spec); + * the Codex block only when forge-marked; the Continue file only when forge-managed. A + * per-target cleanup failure surfaces as an `error` row (never a throw) so the caller can + * keep the managed-set record until a later remove/sync finishes the job (ME-10). Idempotent. * @param {{targetRoot:string, name:string, - * removeJsonEntry:(current:any)=>boolean}} opts + * removeJsonEntry:(target:string, current:any)=>boolean}} opts */ export function removeMcp({ targetRoot, name, removeJsonEntry }) { const rows = []; for (const t of JSON_TARGETS) { const path = join(targetRoot, t.file); - let r; let current; if (existsSync(path)) { try { @@ -324,13 +397,18 @@ export function removeMcp({ targetRoot, name, removeJsonEntry }) { current = undefined; } } - if (current !== undefined && !removeJsonEntry(current)) { - r = { - action: "skipped", - note: `${name} left in place: user-owned entry`, - }; - } else { - r = removeFromJson(path, t.key, name); + let r; + try { + if (current !== undefined && !removeJsonEntry(t.file, current)) { + r = { + action: "skipped", + note: `${name} left in place: user-owned entry`, + }; + } else { + r = removeFromJson(path, t.key, name); + } + } catch (err) { + r = { action: "error", note: `remove failed: ${err.message}` }; } rows.push({ tool: `${t.tool} MCP`, @@ -339,21 +417,31 @@ export function removeMcp({ targetRoot, name, removeJsonEntry }) { note: r.note, }); } - const codex = removeCodexBlock(join(targetRoot, CODEX_FILE), name); + let codex; + try { + codex = removeCodexBlock(join(targetRoot, CODEX_FILE), name); + } catch (err) { + codex = { action: "error", note: `remove failed: ${err.message}` }; + } rows.push({ tool: "Codex MCP", - target: ".codex/config.toml", + target: CODEX_TARGET, action: codex.action, note: codex.note, }); const contPath = join(targetRoot, CONTINUE_DIR, continueFileFor(name)); - const existing = readIfExists(contPath); let cont; - if (existing === null) cont = { action: "unchanged", note: "absent" }; - else if (!isManaged(existing)) cont = { action: "skipped", note: "unmanaged file left in place" }; - else { - rmSync(contPath, { force: true }); - cont = { action: "written", note: "per-server file removed" }; + try { + const existing = readIfExists(contPath); + if (existing === null) cont = { action: "unchanged", note: "absent" }; + else if (!isManaged(existing)) + cont = { action: "skipped", note: "unmanaged file left in place" }; + else { + rmSync(contPath, { force: true }); + cont = { action: "written", note: "per-server file removed" }; + } + } catch (err) { + cont = { action: "error", note: `remove failed: ${err.message}` }; } rows.push({ tool: "Continue MCP", @@ -364,16 +452,21 @@ export function removeMcp({ targetRoot, name, removeJsonEntry }) { return rows; } -/** True when any JSON target or the Codex TOML already holds a same-name entry that - * differs from forge's spec — i.e. an entry forge did NOT just create and must not - * claim without --adopt. Drives add-time auto-ownership in integrations.js. */ -export function hasForeignEntry(targetRoot, name, def) { +/** + * The set of relative target paths that already hold a DIVERGENT same-name entry for + * `name` — an entry forge did NOT just create and must not claim without adoption. Drives + * per-target add-time auto-ownership in integrations.js (ME-08/ME-09): targets NOT in this + * set are absent-or-forge-written and are auto-owned; targets in it need explicit --adopt. + * @returns {Set} + */ +export function foreignTargets(targetRoot, name, def) { + const out = new Set(); for (const t of JSON_TARGETS) { const path = join(targetRoot, t.file); if (!existsSync(path)) continue; try { const cur = JSON.parse(readFileSync(path, "utf8"))?.[t.key]?.[name]; - if (cur !== undefined && JSON.stringify(cur) !== JSON.stringify(def)) return true; + if (cur !== undefined && JSON.stringify(cur) !== JSON.stringify(def)) out.add(t.file); } catch { // invalid JSON: mergeJson will skip the file entirely — not a claimable entry } @@ -383,7 +476,7 @@ export function hasForeignEntry(targetRoot, name, def) { const unmarked = unmarkedRegion(text, name); if (unmarked) { const cur = text.slice(unmarked.start, unmarked.end); - if (cur.trimEnd() !== legacyCodexBlock(name, def).trimEnd()) return true; + if (cur.trimEnd() !== legacyCodexBlock(name, def).trimEnd()) out.add(CODEX_TARGET); } - return false; + return out; } diff --git a/src/init.js b/src/init.js index c921c02..6b3fef1 100644 --- a/src/init.js +++ b/src/init.js @@ -25,7 +25,22 @@ import { list as tasteList } from "./taste.js"; export function ensureLedgerGitattributes(targetRoot = process.cwd()) { const path = join(targetRoot, ".gitattributes"); const existing = existsSync(path) ? readFileSync(path, "utf8") : ""; - if (existing.includes(".forge/ledger/")) return { written: false }; + // ME-21: only a REAL, active attribute line counts as "already present" — not a comment + // or prose that merely mentions the path. A substring check would let a comment mentioning + // `.forge/ledger/` suppress the actual rule forever. Parse the effective rule (the sole + // non-comment line of GITATTRIBUTES_RULE) into + , then look for a + // non-comment line that binds the SAME pattern to (at least) the same attribute. + const ruleLine = GITATTRIBUTES_RULE.split("\n").find( + (l) => l.trim() && !l.trim().startsWith("#"), + ); + const [pattern, ...attrs] = ruleLine.trim().split(/\s+/); + const active = existing.split("\n").some((line) => { + const t = line.trim(); + if (!t || t.startsWith("#")) return false; + const [pat, ...rest] = t.split(/\s+/); + return pat === pattern && attrs.every((a) => rest.includes(a)); + }); + if (active) return { written: false }; appendFileSync( path, `${existing && !existing.endsWith("\n") ? "\n" : ""}${GITATTRIBUTES_RULE}\n`, @@ -118,7 +133,12 @@ function readExistingSettings(path) { return { status: "missing", data: {} }; } try { - return { status: "ok", data: JSON.parse(raw) }; + const parsed = JSON.parse(raw); + // ME-12: valid JSON that is not a top-level object (null / [] / "x" / 42) is corrupt, + // not empty settings — refusing to overwrite preserves the user's real bytes. + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) + return { status: "corrupt", data: null }; + return { status: "ok", data: parsed }; } catch { return { status: "corrupt", data: null }; } @@ -255,11 +275,16 @@ function legacyOwnedScan(settings, template) { /** * Merge Forge settings (hooks, permissions, statusline) into the user's * ~/.claude/settings.json. Preserves all existing entries. Idempotent. - * @param {{settingsPath?: string, noSettings?: boolean}} [opts] + * + * ME-22: `onNotice(target)`, when given, is invoked with the resolved settings path BEFORE + * any read/mutation of that GLOBAL file — so the CLI's consent/disclosure line always + * precedes the merge it describes, never trails it. Not called when the merge is skipped. + * @param {{settingsPath?: string, noSettings?: boolean, onNotice?: (target: string) => void}} [opts] */ -export function mergeSettings({ settingsPath, noSettings } = {}) { +export function mergeSettings({ settingsPath, noSettings, onNotice } = {}) { if (noSettings) return { action: "skipped", reason: "--no-settings" }; const target = settingsPath || join(homedir(), ".claude", "settings.json"); + if (typeof onNotice === "function") onNotice(target); const template = loadTemplate(); const { status, data } = readExistingSettings(target); // Present-but-unparseable: refuse rather than overwrite the user's real (if broken) file. @@ -565,7 +590,9 @@ function writeProfile(targetRoot, profile) { * `settingsOnly` runs the idempotent, marker-guarded `mergeSettings` ONLY — no repo * emit, no AGENTS.md, no gitattributes. That is the surface `install.sh` calls to wire * hooks + permissions into ~/.claude/settings.json without ever touching the user's repo. - * @param {{targetRoot?: string, noSettings?: boolean, profile?: string, settingsOnly?: boolean, settingsPath?: string}} [opts] + * `onSettingsNotice(target)` is forwarded to `mergeSettings` so the GLOBAL-settings + * disclosure is emitted BEFORE the merge mutates ~/.claude/settings.json (ME-22). + * @param {{targetRoot?: string, noSettings?: boolean, profile?: string, settingsOnly?: boolean, settingsPath?: string, onSettingsNotice?: (target: string) => void}} [opts] */ export function init({ targetRoot = process.cwd(), @@ -573,10 +600,15 @@ export function init({ profile, settingsOnly = false, settingsPath, + onSettingsNotice, } = {}) { if (settingsOnly) { return { - settings: mergeSettings({ noSettings, settingsPath }), + settings: mergeSettings({ + noSettings, + settingsPath, + onNotice: onSettingsNotice, + }), settingsOnly: true, }; } @@ -593,7 +625,11 @@ export function init({ if (profileResult?.error) return { profile: profileResult, aborted: true }; const r = sync({ targetRoot }); ensureLedgerGitattributes(targetRoot); - const settings = mergeSettings({ noSettings, settingsPath }); + const settings = mergeSettings({ + noSettings, + settingsPath, + onNotice: onSettingsNotice, + }); const detected = autoDetectProvider(); return { ...r, settings, detected, profile: profileResult }; } diff --git a/src/integrations.js b/src/integrations.js index 6f2c24d..e796a0f 100644 --- a/src/integrations.js +++ b/src/integrations.js @@ -3,17 +3,30 @@ // via `forge integrations add `, which first shows the package, its network behaviour, // and the files it will touch, then requires confirmation (--yes) before writing anything. // -// Installed integrations are RECORDED in `.forge/forge.config.json` under -// `mcp.integrations` (RA-03): every emit — sync or add — computes the same full managed -// set (registry ∪ recorded integrations), so one command can no longer delete another's -// servers. `mcp.adopted` records the names forge may OVERWRITE when a same-name entry -// already exists (RA-21): adding over a user's own entry requires --adopt; a fresh add -// that forge itself creates is auto-adopted (forge owns what it wrote, so later catalog -// updates can refresh it). +// Installed integrations are RECORDED in `.forge/forge.config.json` under `mcp.integrations` +// (RA-03): every emit — sync or add — computes the same full managed set (registry ∪ +// recorded integrations), so one command can no longer delete another's servers. Ownership +// (what forge may OVERWRITE when a same-name entry already exists) is recorded PER TARGET in +// `mcp.adopted` as `{server, target}` pairs (ME-08): adopting a same-name entry for one +// tool's config never authorises overwriting another tool's same-name entry. Legacy bare +// names (`adopted: ["context7"]`) are still honoured — treated as "adopted for every +// target" — and migrated to per-target pairs on the next write. A fresh add that forge +// itself creates owns exactly the targets where it wrote (auto-adopt); a pre-existing +// DIVERGENT same-name entry needs an explicit --adopt for THAT target. Registry names carry +// no implicit ownership (ME-09): a divergent user entry named e.g. `forge-cortex` is +// preserved and reported like any other collision. Add/remove are ordered so disk and the +// record never drift silently (ME-10): add emits first and records only on success; remove +// cleans disk first and drops the record only when every target cleanup succeeded. import { existsSync, readFileSync } from "node:fs"; import { join } from "node:path"; import { BRAND } from "./brand.js"; -import { emitMcp, hasForeignEntry, removeMcp } from "./emit/mcp.js"; +import { + emitMcp, + foreignTargets, + MCP_TARGET_FILES, + removeMcp, + validateServerName, +} from "./emit/mcp.js"; import { readForgeConfig, writeForgeConfig } from "./repo_config.js"; /** The catalog of known optional integrations. Keep each entry honest about what running it @@ -43,11 +56,61 @@ export function planIntegration(name) { return { ok: true, name, pkg: m.pkg, network: m.network, why: m.why }; } -/** The `mcp` record from the repo config, shape-validated. */ +/** + * The `mcp` record from the repo config, shape-validated. `adopted` is returned as a mixed + * list of legacy bare names (strings) and per-target `{server, target}` pairs — both are + * honoured by `adoptionOwns`; `expandAdopted` migrates them to pairs on write. + */ function mcpRecord(cfg) { const raw = cfg.mcp && typeof cfg.mcp === "object" && !Array.isArray(cfg.mcp) ? cfg.mcp : {}; - const names = (v) => (Array.isArray(v) ? v.filter((n) => typeof n === "string") : []); - return { integrations: names(raw.integrations), adopted: names(raw.adopted) }; + const integrations = Array.isArray(raw.integrations) + ? raw.integrations.filter((n) => typeof n === "string") + : []; + const adopted = Array.isArray(raw.adopted) + ? raw.adopted.filter( + (a) => + typeof a === "string" || + (a && + typeof a === "object" && + typeof a.server === "string" && + typeof a.target === "string"), + ) + : []; + return { integrations, adopted }; +} + +/** + * Build the per-target ownership predicate `owns(target, name)` from an adoption record. + * A legacy bare name is a wildcard (adopted for every target); a `{server, target}` pair is + * scoped to exactly that target — the crux of ME-08. + */ +function adoptionOwns(adopted) { + const wildcard = new Set(); + const pairs = new Set(); + for (const a of adopted) { + if (typeof a === "string") wildcard.add(a); + else pairs.add(`${a.server}\t${a.target}`); + } + return (target, name) => wildcard.has(name) || pairs.has(`${name}\t${target}`); +} + +/** Normalise an adoption record to deduped per-target pairs — migrating legacy bare names + * (expanded across every ownable target) to `{server, target}` on write (ME-08). */ +function expandAdopted(adopted) { + const out = []; + const seen = new Set(); + const push = (server, target) => { + const k = `${server}\t${target}`; + if (!seen.has(k)) { + seen.add(k); + out.push({ server, target }); + } + }; + for (const a of adopted) { + if (typeof a === "string") for (const t of MCP_TARGET_FILES) push(a, t); + else push(a.server, a.target); + } + return out; } /** The built-in server registry (source/mcp.json) — implicitly managed, always emitted. */ @@ -57,73 +120,115 @@ function registryServers() { return JSON.parse(readFileSync(p, "utf8")); } +/** Registry ∪ recorded integrations (that exist in the catalog) — the full managed set. */ +function managedServers(rec) { + const servers = { ...registryServers() }; + for (const name of rec.integrations) + if (INTEGRATIONS[name]) servers[name] = INTEGRATIONS[name].server; + return servers; +} + /** * The full managed MCP state for a repo — the ONE computation both `sync` and * `integrations add` share, so they can never oscillate (RA-03): * servers = registry ∪ (recorded integrations that exist in the catalog) - * owned = registry names ∪ recorded adopted names (what forge may overwrite) - * A corrupt repo config is NEVER treated as "no integrations installed": the result - * falls back to registry-only and says so, so no recorded server gets clobbered. + * owns = adoptionOwns(recorded adopted) — PER TARGET (ME-08); registry names carry NO + * implicit ownership (ME-09), so a divergent same-name entry is preserved. + * A corrupt repo config is NEVER treated as "no integrations installed": the result falls + * back to registry-only, owns nothing implicitly, and says so, so nothing gets clobbered. * @param {string} targetRoot - * @returns {{servers:Record, owned:Set, corrupt:boolean, warning?:string}} + * @returns {{servers:Record, owns:(target:string,name:string)=>boolean, + * corrupt:boolean, warning?:string}} */ export function managedMcpState(targetRoot) { - const registry = registryServers(); - const servers = { ...registry }; - const owned = new Set(Object.keys(registry)); const cfg = readForgeConfig(targetRoot); if (cfg.corrupt) { return { - servers, - owned, + servers: { ...registryServers() }, + owns: () => false, corrupt: true, warning: `${cfg.path} is not valid JSON — MCP emitted registry-only; recorded integrations left untouched (fix or delete it)`, }; } const rec = mcpRecord(cfg); - for (const name of rec.integrations) { - if (INTEGRATIONS[name]) servers[name] = INTEGRATIONS[name].server; - } - for (const name of rec.adopted) owned.add(name); - return { servers, owned, corrupt: false }; + return { + servers: managedServers(rec), + owns: adoptionOwns(rec.adopted), + corrupt: false, + }; } /** - * Install an integration: record it in `.forge/forge.config.json` FIRST (the persistent - * managed set), then emit the FULL set so nothing previously installed is dropped. - * `adopt: true` (--adopt) additionally claims a pre-existing same-name entry as - * forge-owned; without it such an entry is preserved and reported. + * Install an integration. ME-10 order: emit the FULL managed set into every target FIRST, + * then record the install in `.forge/forge.config.json` ONLY if every target emitted without + * error — a partial write never leaves a fully-installed record behind. Ownership is decided + * PER TARGET (ME-08): forge auto-adopts exactly the targets where it wrote a fresh/identical + * entry; a pre-existing DIVERGENT same-name entry on a target is preserved and reported + * unless `adopt: true` (--adopt) claims it. `res.adopted` reports whether pre-existing + * divergent entries were claimed. */ export function addIntegration(name, { targetRoot = process.cwd(), adopt = false } = {}) { const m = INTEGRATIONS[name]; if (!m) return { ok: false, reason: `unknown integration: ${name}` }; - // Fresh create → forge owns it (auto-adopt). A pre-existing DIVERGENT same-name entry - // is someone else's config: claiming it requires the explicit --adopt. - const foreign = hasForeignEntry(targetRoot, name, m.server); - const own = adopt || !foreign; - const rec = writeForgeConfig(targetRoot, (cfg) => { - const cur = mcpRecord(cfg); - if (!cur.integrations.includes(name)) cur.integrations.push(name); - if (own && !cur.adopted.includes(name)) cur.adopted.push(name); - cfg.mcp = { - ...(typeof cfg.mcp === "object" && cfg.mcp ? cfg.mcp : {}), - ...cur, - }; - return cfg; - }); + try { + validateServerName(name); + } catch (err) { + return { ok: false, reason: err.message }; + } + const cfg = readForgeConfig(targetRoot); // Corrupt config: refuse rather than emit an unrecorded (and therefore un-removable, // oscillation-prone) server. The config module already warned on stderr. - if (rec.ok === false) return { ok: false, reason: rec.reason }; - const { servers, owned } = managedMcpState(targetRoot); - const rows = emitMcp({ targetRoot, servers, owned }); - return { ok: true, name, adopted: own, rows }; + if (cfg.corrupt) + return { + ok: false, + reason: `${cfg.path} is not valid JSON — cannot update the managed-set record (fix or delete it)`, + }; + const rec = mcpRecord(cfg); + const servers = { ...managedServers(rec), [name]: m.server }; + // Per-target ownership: forge owns the targets where it writes fresh (or byte-identical); + // a pre-existing divergent entry is owned only with --adopt. + const foreign = foreignTargets(targetRoot, name, m.server); + const anyForeign = foreign.size > 0; + const newAdoptions = []; + for (const t of MCP_TARGET_FILES) + if (!foreign.has(t) || adopt) newAdoptions.push({ server: name, target: t }); + const owns = adoptionOwns([...rec.adopted, ...newAdoptions]); + // Emit FIRST (ME-10). A per-target write failure surfaces as an `error` row. + const rows = emitMcp({ targetRoot, servers, owns }); + const failed = rows.filter((r) => r.action === "error"); + if (failed.length) + return { + ok: false, + incomplete: true, + name, + rows, + reason: `incomplete: MCP emission failed for ${failed.map((r) => r.target).join(", ")} — install NOT recorded; fix and re-run \`${BRAND.cli} integrations add ${name} --yes\``, + }; + // Record only after a fully successful emit. + const w = writeForgeConfig(targetRoot, (c) => { + const cur = mcpRecord(c); + const integrations = cur.integrations.includes(name) + ? cur.integrations + : [...cur.integrations, name]; + const adopted = expandAdopted([...cur.adopted, ...newAdoptions]); + c.mcp = { + ...(typeof c.mcp === "object" && c.mcp ? c.mcp : {}), + integrations, + adopted, + }; + return c; + }); + if (w.ok === false) return { ok: false, reason: w.reason }; + return { ok: true, name, adopted: anyForeign ? adopt : true, rows }; } /** - * Reverse `add `: drop it from the persistent record, then remove only what forge - * owns — its per-server Continue file, its forge-marked Codex block, and JSON entries - * that are adopted or byte-identical to forge's spec. A user's own divergent entry is - * left in place and reported. Second remove is a no-op. + * Reverse `add `. ME-10 order: clean every target FIRST (per-server Continue file, + * forge-marked Codex block, and JSON entries adopted for that target or byte-identical to + * forge's spec), then drop it from the persistent record ONLY if every target cleanup + * succeeded. If any target cleanup fails the record is KEPT and an incomplete transaction is + * reported, so a later remove/sync can finish. A user's own divergent entry is left in place + * and reported. Second remove is a no-op. */ export function removeIntegration(name, { targetRoot = process.cwd() } = {}) { const m = INTEGRATIONS[name]; @@ -135,22 +240,40 @@ export function removeIntegration(name, { targetRoot = process.cwd() } = {}) { reason: `${cfg.path} is not valid JSON — cannot update the managed-set record (fix or delete it)`, }; const before = mcpRecord(cfg); - const wasRecorded = before.integrations.includes(name) || before.adopted.includes(name); + const adoptedName = (a) => (typeof a === "string" ? a : a.server); + const wasRecorded = + before.integrations.includes(name) || before.adopted.some((a) => adoptedName(a) === name); if (!wasRecorded) return { ok: true, name, removed: false, rows: [] }; - const rec = writeForgeConfig(targetRoot, (c) => { - const cur = mcpRecord(c); - cur.integrations = cur.integrations.filter((n) => n !== name); - cur.adopted = cur.adopted.filter((n) => n !== name); - c.mcp = { ...(typeof c.mcp === "object" && c.mcp ? c.mcp : {}), ...cur }; - return c; - }); - if (rec.ok === false) return { ok: false, reason: rec.reason }; - const wasAdopted = before.adopted.includes(name); + const owns = adoptionOwns(before.adopted); + // Clean disk FIRST (ME-10). A per-target cleanup failure surfaces as an `error` row. const rows = removeMcp({ targetRoot, name, - removeJsonEntry: (current) => - wasAdopted || JSON.stringify(current) === JSON.stringify(m.server), + removeJsonEntry: (target, current) => + owns(target, name) || JSON.stringify(current) === JSON.stringify(m.server), + }); + const failed = rows.filter((r) => r.action === "error"); + if (failed.length) + return { + ok: false, + incomplete: true, + name, + removed: false, + rows, + reason: `incomplete: cleanup failed for ${failed.map((r) => r.target).join(", ")} — kept in the managed set; fix and re-run \`${BRAND.cli} integrations remove ${name}\` to finish`, + }; + // Drop from the record only after a fully successful cleanup. + const w = writeForgeConfig(targetRoot, (c) => { + const cur = mcpRecord(c); + const integrations = cur.integrations.filter((n) => n !== name); + const adopted = expandAdopted(cur.adopted).filter((a) => a.server !== name); + c.mcp = { + ...(typeof c.mcp === "object" && c.mcp ? c.mcp : {}), + integrations, + adopted, + }; + return c; }); + if (w.ok === false) return { ok: false, reason: w.reason }; return { ok: true, name, removed: true, rows }; } diff --git a/src/ledger.js b/src/ledger.js index 6bd7b7d..2e06924 100644 --- a/src/ledger.js +++ b/src/ledger.js @@ -143,10 +143,22 @@ export function mintClaim({ kind, body, scope = {}, provenance = {}, t = 0 }) { // Typed evidence refs are `:`. Only these types are recognized; anything // else (or a ref with no `type:` prefix) is treated as an untyped/legacy ref and accepted // unchanged for back-compat. `git:` is the one type forge can cheaply AND soundly resolve — -// the object must exist in THIS repo — so it is ALWAYS resolved when a resolver is supplied; -// the rest are format-checked (a non-empty value) since resolving them is not cheap/sound. +// the object must exist in THIS repo — so it is ALWAYS resolved when a resolver is supplied. +// The rest now carry real FORMAT grammars (ME-05): `ci:` must be a CI locator, `human:` +// must be an explicit ratification, `file:` must resolve to an existing path when a repo +// root is available. A `test:` run id remains format-only — it is unverifiable — which is +// why it can never lift confidence into the trusted band (see refStrength/val). export const REF_TYPES = new Set(["git", "file", "test", "ci", "human"]); +// A `ci:` ref must be a real CI locator: an http(s) URL, an `owner/repo@run` reference, +// or a bare numeric run id. Prose like "not-a-url" is refused so a made-up string can +// never masquerade as evidence. +const CI_REF_RE = /^(https?:\/\/\S+|[\w.-]+\/[\w.-]+@\S+|\d+)$/; +// A `human:` ref is an EXPLICIT ratification: a named person `@` the thing they ratified +// (e.g. `alice@decision-42`). "the-model-said-yes" is not a human ratifying anything — +// the model's own assertion is refused. +const HUMAN_REF_RE = /^[^@\s]+@\S+$/; + /** Parse a typed ref into {type, value}, or null for an untyped/legacy ref. */ export function parseRef(ref) { const m = /^([a-z]+):(.*)$/.exec(String(ref ?? "")); @@ -155,14 +167,20 @@ export function parseRef(ref) { } /** - * Validate an evidence ref. Untyped/legacy → accepted. Typed-but-empty → rejected. - * `git:` → resolved through the injected `resolveGit(sha)` predicate (execFileSync lives in - * the impure store, keeping this module pure); a git ref that does not resolve is rejected. + * Validate an evidence ref (record-integrity FORMAT + optional resolution). Untyped/legacy + * → accepted. Typed-but-empty → rejected. Typed refs are format-checked purely (`ci:` is a + * CI locator, `human:` is a ratification) and, for the two I/O-resolvable types, resolved + * through injected predicates (execFileSync/existsSync live in the impure store, keeping + * this module pure): `git:` via `resolveGit(sha)`, `file:` via `resolveFile(path)`. A typed + * ref whose resolver is supplied but returns false is rejected. + * + * NOTE: passing FORMAT is not the same as the evidence being TRUE — see refStrength/val for + * how merely-format-valid evidence is weighted below the serving threshold. * @param {string} ref - * @param {{resolveGit?: (sha:string)=>boolean}} [opts] + * @param {{resolveGit?: (sha:string)=>boolean, resolveFile?: (path:string)=>boolean}} [opts] * @returns {{ok: boolean, reason?: string}} */ -export function validateRef(ref, { resolveGit } = {}) { +export function validateRef(ref, { resolveGit, resolveFile } = {}) { const parsed = parseRef(ref); if (!parsed) return { ok: true }; // untyped/legacy — kept for back-compat if (!parsed.value) return { ok: false, reason: `evidence ref "${ref}" is typed but empty` }; @@ -171,19 +189,71 @@ export function validateRef(ref, { resolveGit } = {}) { ok: false, reason: `evidence ref "${ref}" is unresolvable (no such git object)`, }; + if (parsed.type === "ci" && !CI_REF_RE.test(parsed.value)) + return { + ok: false, + reason: `evidence ref "${ref}" is not a CI locator (URL, owner/repo@run, or run id)`, + }; + if (parsed.type === "human" && !HUMAN_REF_RE.test(parsed.value)) + return { + ok: false, + reason: `evidence ref "${ref}" is not an explicit human ratification (author@ref)`, + }; + if (parsed.type === "file" && typeof resolveFile === "function" && !resolveFile(parsed.value)) + return { + ok: false, + reason: `evidence ref "${ref}" is unresolvable (no such file)`, + }; return { ok: true }; } +// The trust model (ME-05): record-integrity validity (validateRef/validOutcome) is NOT the +// same as evidence being RESOLVED. Only resolved evidence may lift confidence into the +// trusted/serving band. Two tiers, both re-derived PURELY from the ref so a forged log line +// can never buy a strength it isn't entitled to (same discipline as the ORACLES weights): +// - RESOLVED: untyped/legacy (historical trust), `git:` (resolved at the append gate — +// the one soundly-resolvable type), `ci:` (only well-formed CI locators pass validateRef), +// and `human:` (only explicit ratifications pass). These count at full weight. +// - FORMAT-ONLY: `file:` and `test:`. A path existing or a run id being well-formed is +// record integrity, NOT proof the claim is true, and pure val() cannot re-check existence. +// These count at a REDUCED weight and, on their own, are capped below the serving floor. +const RESOLVED_REF_TYPES = new Set(["git", "ci", "human"]); + +/** Resolution strength of a ref for confidence weighting: "resolved" or "format". + * Pure and total — never throws, never does I/O. */ +export function refStrength(ref) { + const parsed = parseRef(ref); + if (!parsed) return "resolved"; // untyped/legacy — historical full trust + return RESOLVED_REF_TYPES.has(parsed.type) ? "resolved" : "format"; +} + +/** Weight multiplier applied to merely-format-valid (unresolved) evidence in val(). */ +export const UNRESOLVED_WEIGHT = 0.5; +/** A claim whose confirming evidence is ALL format-only may never be lifted to/above the + * serving band. This cap sits below both the reuse SERVE_FLOOR (0.6) and the trusted band + * (1 − DORMANT_VAL = 0.65): such a claim stays retrievable but is never "trusted". */ +export const UNRESOLVED_VAL_CAP = 0.55; + /** * Build an evidence outcome. Evidence without a verifiable ref (commit SHA, test-run * id, episode id, CI URL) is rejected — "the model said so" is not evidence. A typed ref - * (`git:`/`file:`/`test:`/`ci:`/`human:`) is validated; a `git:` ref is resolved when a - * `resolveGit` predicate is supplied. The oracle's table weight is recorded for audit, - * but val() re-reads the table. - * @param {{oracle:string, result:"confirm"|"contradict", ref:string, author?:string, t?:number, resolveGit?:(sha:string)=>boolean}} f + * (`git:`/`file:`/`test:`/`ci:`/`human:`) is validated; `git:`/`file:` are resolved when a + * `resolveGit`/`resolveFile` predicate is supplied. A secret-shaped ref or author is refused + * here too (ME-06) — the same detector putClaim runs over claim content — so credentials + * never enter the evidence log. The oracle's table weight is recorded for audit, but val() + * re-reads the table. + * @param {{oracle:string, result:"confirm"|"contradict", ref:string, author?:string, t?:number, resolveGit?:(sha:string)=>boolean, resolveFile?:(path:string)=>boolean}} f * @returns {{ok:true, outcome:any}|{ok:false, reason:string}} */ -export function outcomeRecord({ oracle, result, ref, author = "", t = 0, resolveGit }) { +export function outcomeRecord({ + oracle, + result, + ref, + author = "", + t = 0, + resolveGit, + resolveFile, +}) { const o = ORACLES[oracle]; if (!o) return { ok: false, reason: `unknown oracle: ${oracle}` }; if (result !== "confirm" && result !== "contradict") @@ -193,8 +263,13 @@ export function outcomeRecord({ oracle, result, ref, author = "", t = 0, resolve }; if (!ref || typeof ref !== "string") return { ok: false, reason: "evidence requires a verifiable ref" }; - const v = validateRef(ref, { resolveGit }); + const v = validateRef(ref, { resolveGit, resolveFile }); if (!v.ok) return { ok: false, reason: v.reason ?? "invalid evidence ref" }; + if (hasSecret(ref) || hasSecret(author)) + return { + ok: false, + reason: "refused: evidence ref/author looks like a secret/credential", + }; return { ok: true, outcome: sealRecord({ author, oracle, ref, result, t, w: o.w }), @@ -231,6 +306,12 @@ const decayed = (outcome, nowDay, halfLife) => * trusted. Optional `trust` (author → u, see authorTrust) scales each record by its * appender's earned reliability. Pure function of (evidence set, trust map) ⇒ * identical after any merge order. + * + * Resolution strength (ME-05): merely-format-valid evidence (`file:`/`test:` — a pointer, + * not a demonstration) counts at UNRESOLVED_WEIGHT, and a claim with NO resolved + * confirmation is capped at UNRESOLVED_VAL_CAP so `test:made-up-run` (and friends) can + * never lift confidence into the trusted/serving band. The cap only lowers — contradictions + * still sink val toward 0 as before. * @param {any} claim * @param {number} [nowDay] * @param {{halfLife?: number, trust?: Record}} [opts] @@ -238,13 +319,20 @@ const decayed = (outcome, nowDay, halfLife) => export function val(claim, nowDay = 0, { halfLife = DEFAULT_HALF_LIFE_DAYS, trust } = {}) { let confirms = 0; let all = 0; + let resolvedConfirm = false; for (const e of claim.evidence ?? []) { if (!validOutcome(e)) continue; - const d = decayed(e, nowDay, halfLife) * (trust?.[e.author ?? ""] ?? 1); + const resolved = refStrength(e.ref) === "resolved"; + const strength = resolved ? 1 : UNRESOLVED_WEIGHT; + const d = decayed(e, nowDay, halfLife) * (trust?.[e.author ?? ""] ?? 1) * strength; all += d; - if (e.result === "confirm") confirms += d; + if (e.result === "confirm") { + confirms += d; + if (resolved) resolvedConfirm = true; + } } - return (1 + confirms) / (2 + all); + const v = (1 + confirms) / (2 + all); + return resolvedConfirm ? v : Math.min(v, UNRESOLVED_VAL_CAP); } /** diff --git a/src/ledger_store.js b/src/ledger_store.js index 32886fd..56efd58 100644 --- a/src/ledger_store.js +++ b/src/ledger_store.js @@ -14,7 +14,7 @@ import { renameSync, writeFileSync, } from "node:fs"; -import { dirname, join } from "node:path"; +import { dirname, isAbsolute, join } from "node:path"; import { authorTrust, canonicalize, @@ -32,6 +32,8 @@ import { validateRef, validOutcome, } from "./ledger.js"; +import { redactSecrets } from "./secrets.js"; +import { contentHash } from "./util.js"; /** The canonical repo ledger. (recall's global store keeps its own sibling ledger.) */ export const repoLedger = (root = process.cwd()) => join(root, ".forge", "ledger"); @@ -63,6 +65,17 @@ const gitResolver = (root) => (sha) => { } }; +// `file:` ref resolver (ME-05): the referenced path must exist. Relative paths resolve +// against the repo root; absolute paths are used as-is. Pure existence check — no read, +// no throw — so a `file:/does/not/exist` ref is rejected before it can buy confidence. +const fileResolver = (root) => (p) => { + try { + return existsSync(isAbsolute(p) ? p : join(root, p)); + } catch { + return false; + } +}; + const LOGS = ["evidence", "provenance", "tombstones"]; const claimPath = (dir, id) => join(dir, "claims", id.slice(0, 2), `${id}.json`); const logPath = (dir, log, id) => join(dir, log, `${id}.log`); @@ -111,7 +124,11 @@ function readLog(dir, log, id, { verifyHashes = true } = {}) { } /** Append one sealed record to a log iff its hash isn't already present. The seal is - * RECHECKED here — a record whose `h` does not match its own content never lands. */ + * RECHECKED here — a record whose `h` does not match its own content never lands — and + * the record is scanned for secrets (ME-06): a credential in an evidence ref/author, a + * tombstone reason, or provenance metadata is refused BEFORE it can touch disk, exactly + * as putClaim refuses secret-bearing claim content. This is the single append choke point + * for every metadata log, so no channel can smuggle a secret onto disk (or into a merge). */ function appendRecord(dir, log, id, record) { if (!record?.h) return { ok: false, reason: "record missing content hash" }; const { h, ...rest } = record; @@ -120,6 +137,11 @@ function appendRecord(dir, log, id, record) { ok: false, reason: "record content hash mismatch (forged/corrupt)", }; + if (hasSecret(canonicalize(record))) + return { + ok: false, + reason: "refused: record metadata looks like a secret/credential", + }; if (!existsSync(claimPath(dir, id))) return { ok: false, reason: `no such claim in ledger: ${id}` }; if (readLog(dir, log, id).some((e) => e.h === record.h)) return { ok: true, deduped: true }; @@ -187,8 +209,10 @@ export function putClaim(dir, claim) { * reach val() and buy confidence. */ export function appendEvidence(dir, id, outcome) { if (!validOutcome(outcome)) return { ok: false, reason: "invalid outcome (use outcomeRecord)" }; + const root = repoRootOf(dir); const v = validateRef(outcome.ref, { - resolveGit: gitResolver(repoRootOf(dir)), + resolveGit: gitResolver(root), + resolveFile: fileResolver(root), }); if (!v.ok) return { ok: false, reason: v.reason ?? "unresolvable evidence ref" }; return appendRecord(dir, "evidence", id, outcome); @@ -283,14 +307,63 @@ export function getClaimByPrefix(dir, prefix) { return liveClaims(state)[0]; } +/** Try to import one raw source log line into `dir`; returns {ok, deduped} on success or + * {reason} on rejection, so mergeDirs can quarantine what it can't import. Unlike the + * state-based path, this NEVER loses a line to the read-path hash-dedup or the no-`h` + * drop: every source line is either imported or quarantined by trusted identity. */ +function tryImportLine(dir, log, id, rec) { + if (!rec?.h) + return { + reason: "malformed: unparseable log line or missing content hash", + }; + const a = log === "evidence" ? appendEvidence(dir, id, rec) : appendRecord(dir, log, id, rec); + return a.ok ? { ok: true, deduped: a.deduped } : { reason: a.reason ?? "rejected" }; +} + /** `forge ledger merge ` — semilattice merge of another on-disk ledger into * this one. Idempotent and order-independent by the CRDT property, so merging a - * teammate's checkout, a backup, or a branch worktree is always safe. */ + * teammate's checkout, a backup, or a branch worktree is always safe. + * + * The SOURCE is read RAW, line by line (ME-07): every candidate record is re-validated + * against THIS ledger and either appended (deduped) or quarantined under a trusted + * identity. Reading raw — instead of through loadState's hash-dedup — is what lets two + * forged records sharing one fake `h`, and malformed no-`h` lines, all reach quarantine + * instead of being silently collapsed or dropped. */ export function mergeDirs(dstDir, srcDir) { - // The SOURCE is read raw (hashes unverified) on purpose: importState re-validates - // every record against THIS ledger, so forged/invalid lines end up quarantined with - // a named reason instead of being silently dropped at read time. - return importState(dstDir, loadState(srcDir, { verifyHashes: false })); + let claims = 0; + let records = 0; + let quarantined = 0; + // 1. Bring over claim files (pure content). Corrupt source claim files are named by + // verify(), not merged — putClaim would reject a bad address anyway. + for (const { claim } of walkClaimFiles(srcDir)) { + if (!claim) continue; + const r = putClaim(dstDir, claim); + if (r.ok && !r.existed) claims++; + } + // 2. For every claim now in the destination, merge the source's log lines RAW. + const ids = []; + for (const { id, claim } of walkClaimFiles(dstDir)) if (claim) ids.push(id); + for (const id of ids) { + for (const log of LOGS) { + const path = logPath(srcDir, log, id); + if (!existsSync(path)) continue; + for (const line of readFileSync(path, "utf8").split(/\r?\n/)) { + if (!line.trim()) continue; + let rec = null; + try { + rec = JSON.parse(line); + } catch {} + const res = tryImportLine(dstDir, log, id, rec); + if (res.ok) { + if (!res.deduped) records++; + } else { + quarantined += quarantineRecord(dstDir, id, rec ?? { raw: line }, res.reason); + } + } + } + } + reindex(dstDir); + return { claims, records, quarantined }; } /** @@ -323,17 +396,29 @@ export function blame(dir, prefix, nowDay = 0) { }; } -/** Quarantine one rejected import record — an append-only audit line ({reason, rec, t}, - * sealed like every other log record) under quarantine/.log, deduped by the - * OFFENDING record's own hash so re-merges stay idempotent. Returns 1 when newly - * quarantined, 0 on a dupe. The timestamp is the record's own day (records never - * read the clock here — same rule as mintClaim/outcomeRecord). */ +/** Quarantine one rejected import record — an append-only audit line under + * quarantine/.log. Two hardening rules (ME-07): + * - The stored `rec` is REDACTED first: the quarantine log is an audit trail, never a + * place to persist the very credential we just refused (ME-06). A malformed line that + * never parsed is captured as {raw:"…"} so nothing is silently dropped. + * - Dedup is by a TRUSTED `qhash` computed with contentHash over the (redacted) record + * PLUS the rejection reason — NEVER the rejected record's own attacker-chosen `h`. Two + * distinct forged records that share one fake `h` therefore get DISTINCT identities and + * both survive; a malformed line with no `h` gets one too. Returns 1 when newly + * quarantined, 0 on a trusted-identity dupe (re-merges stay idempotent). */ function quarantineRecord(dir, id, rec, reason) { - if (readLog(dir, "quarantine", id).some((q) => q.rec?.h === rec?.h)) return 0; + let redacted; + try { + redacted = JSON.parse(redactSecrets(canonicalize(rec ?? null))); + } catch { + redacted = { redacted: true }; + } + const qhash = contentHash(canonicalize({ reason, rec: redacted })); + if (readLog(dir, "quarantine", id).some((q) => q.qhash === qhash)) return 0; mkdirSync(join(dir, "quarantine"), { recursive: true }); appendFileSync( logPath(dir, "quarantine", id), - `${canonicalize(sealRecord({ reason, rec, t: rec?.t ?? 0 }))}\n`, + `${canonicalize(sealRecord({ qhash, reason, rec: redacted, t: rec?.t ?? 0 }))}\n`, ); return 1; } @@ -389,7 +474,9 @@ export function verify(dir) { let claims = 0; let outcomes = 0; const ids = []; - const resolveGit = gitResolver(repoRootOf(dir)); + const root = repoRootOf(dir); + const resolveGit = gitResolver(root); + const resolveFile = fileResolver(root); for (const { id, raw, claim } of walkClaimFiles(dir)) { if (!claim) issues.push(`claim ${id}: unparseable or id mismatch`); else { @@ -424,7 +511,7 @@ export function verify(dir) { else { // Typed, unresolvable refs (e.g. a `git:` sha absent from this repo) are named // so CI catches evidence that can never be re-derived. - const v = validateRef(o.ref, { resolveGit }); + const v = validateRef(o.ref, { resolveGit, resolveFile }); if (!v.ok) issues.push(`${where}: ${v.reason ?? "unresolvable evidence ref"}`); else outcomes++; } diff --git a/src/repo_config.js b/src/repo_config.js index 4e969f9..26909c5 100644 --- a/src/repo_config.js +++ b/src/repo_config.js @@ -10,7 +10,15 @@ // writes REFUSE (`{ok:false, reason}`) rather than replace bytes a human may still want // to fix. When no config exists the primary tool is auto-detected from which agent // folders/files are present (mirrors autoDetectProvider in providers.js). -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { + copyFileSync, + existsSync, + mkdirSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from "node:fs"; import { dirname, join } from "node:path"; import { BRAND } from "./brand.js"; import { ensureGitignoreBlock } from "./gitignore.js"; @@ -27,9 +35,14 @@ const DETECT = [ { tool: "codex", marker: ".codex" }, { tool: "zed", marker: ".zed" }, { tool: "vscode", marker: ".vscode" }, + { tool: "aider", marker: ".aider.conf.yml" }, + { tool: "continue", marker: ".continue" }, + { tool: "windsurf", marker: ".windsurf" }, + { tool: "roo", marker: ".roo" }, ]; -/** Tool names accepted by `forge tools `. */ +/** Tool names accepted by `forge tools `. Kept in lockstep with the emit targets + * (rowToolKey / TOOL_KEYS) — every tool Forge can emit for is selectable as primary. */ export const KNOWN_TOOLS = [ "claude", "cursor", @@ -40,6 +53,7 @@ export const KNOWN_TOOLS = [ "aider", "continue", "windsurf", + "roo", ]; // Map a sync-report row's tool label to a canonical tool key. The shared source @@ -64,6 +78,10 @@ const legacyConfigPath = (root) => join(root, LEGACY_CONFIG_REL); // One loud warning per process (not per read) — the cortex hooks re-read config on every // event, and repeating the same corruption warning on each hook fire would be spam. let warnedCorruptConfig = false; + +/** Filesystem-safe timestamp for backup filenames. */ +const stamp = () => new Date().toISOString().replace(/[:.]/g, "-"); + function warnCorrupt(path) { if (warnedCorruptConfig) return; warnedCorruptConfig = true; @@ -81,8 +99,12 @@ function readConfigFile(path) { if (!existsSync(path)) return { status: "missing", data: {} }; try { const parsed = JSON.parse(readFileSync(path, "utf8")); - const ok = parsed && typeof parsed === "object" && !Array.isArray(parsed); - return { status: "ok", data: ok ? parsed : {} }; + // ME-12: valid JSON that is NOT a top-level object (null / [] / "x" / 42) is treated + // as corrupt, exactly like unparseable bytes — never coerced to `{}`, or a later write + // would silently replace bytes a human may still want (violating the config invariant). + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) + return { status: "corrupt", data: {} }; + return { status: "ok", data: parsed }; } catch { return { status: "corrupt", data: {} }; } @@ -139,7 +161,14 @@ export function writeForgeConfig(root, mutator) { const draft = { ...legacy.data, ...unified.data }; const next = mutator(draft) ?? draft; mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, `${JSON.stringify(next, null, 2)}\n`); + // ME-13: back up an existing valid config before overwriting, then write via a temp file + // + atomic rename (same pattern as mergeSettings) so a crash mid-write can never truncate + // the unified config. First write (no existing file) needs no backup. + if (unified.status === "ok" && existsSync(path)) + copyFileSync(path, `${path}.forge-bak-${stamp()}`); + const tmp = `${path}.forge-tmp-${process.pid}`; + writeFileSync(tmp, `${JSON.stringify(next, null, 2)}\n`); + renameSync(tmp, path); return { ok: true, path, config: next }; } diff --git a/src/stack.js b/src/stack.js index 45fa9c2..900b716 100644 --- a/src/stack.js +++ b/src/stack.js @@ -255,6 +255,130 @@ const DETECTORS = [ detectDotnet, ]; +// --------------------------------------------------------------------------- +// Monorepo / workspace detection (ME-03). The DETECTORS above read manifests only at +// the repo ROOT, so npm/pnpm/yarn workspaces and Turborepo/lerna/Maven/Gradle +// subprojects (and nested Python packages) are invisible — a single root test command +// may or may not cover them, and forge never verified that. This surfaces the declared +// workspace globs (`workspaces`) and the nested package roots actually on disk +// (`packageRoots`) so the caller/verifier can see there is more than one suite. It is +// deliberately BOUNDED — a shallow, budgeted BFS, never a deep walk of a huge tree. +// --------------------------------------------------------------------------- + +// Manifests that mark a directory as its own package/suite root. +const WORKSPACE_MANIFESTS = [ + "package.json", + "pyproject.toml", + "setup.py", + "go.mod", + "Cargo.toml", + "pom.xml", + "build.gradle", + "build.gradle.kts", + "Gemfile", + "composer.json", +]; +// Never descend into these — vendored deps, VCS metadata, build output, caches. +const WALK_SKIP = new Set([ + "node_modules", + ".git", + ".hg", + ".svn", + ".forge", + "dist", + "build", + "out", + "target", + "vendor", + ".venv", + "venv", + "__pycache__", + ".next", + ".nuxt", + ".turbo", + ".cache", + "coverage", +]); +const MONO_MAX_DEPTH = 3; // deepest nested dir considered (e.g. apps/web, packages/*/pkg) +const MONO_SCAN_BUDGET = 200; // hard ceiling on dirs stat-ed — bounds cost on large trees +const MONO_MAX_ROOTS = 50; // most nested package roots surfaced + +// Zero-dep: pull the `packages:` list from a pnpm-workspace.yaml. Ignores negations (`!…`). +function parsePnpmPackages(text) { + const out = []; + let inBlock = false; + for (const raw of text.split(/\r?\n/)) { + const line = raw.replace(/\s+#.*$/, ""); + if (/^packages:\s*$/.test(line)) { + inBlock = true; + continue; + } + if (!inBlock) continue; + const m = /^\s*-\s*(.+?)\s*$/.exec(line); + if (m) { + const v = m[1].trim().replace(/^["']|["']$/g, ""); + if (v && !v.startsWith("!")) out.push(v); + } else if (/^\S/.test(line)) { + inBlock = false; // a new top-level key ends the list + } + } + return out; +} + +// Declared workspace globs from every root config that declares them. Never throws. +function workspaceGlobs(root) { + const globs = new Set(); + const pkg = readJson(root, "package.json"); + if (pkg) { + const ws = pkg.workspaces; + const arr = Array.isArray(ws) ? ws : Array.isArray(ws?.packages) ? ws.packages : []; + for (const g of arr) if (typeof g === "string") globs.add(g); + } + const lerna = readJson(root, "lerna.json"); + if (lerna && Array.isArray(lerna.packages)) + for (const g of lerna.packages) if (typeof g === "string") globs.add(g); + const pnpm = read(root, "pnpm-workspace.yaml"); + if (pnpm) for (const g of parsePnpmPackages(pnpm)) globs.add(g); + return [...globs].sort(); +} + +// Bounded BFS for nested package roots below `root`. Returns POSIX-relative dir paths, +// deduped and sorted. Never descends past MONO_MAX_DEPTH, never stats more than +// MONO_SCAN_BUDGET dirs, never returns more than MONO_MAX_ROOTS — so a giant repo is +// sampled, never fully walked. Fail-safe: an unreadable dir is skipped. +function nestedPackageRoots(root) { + const found = []; + let budget = MONO_SCAN_BUDGET; + /** @type {{dir:string, rel:string, depth:number}[]} */ + const queue = []; + const enqueueChildren = (absDir, relDir, depth) => { + if (depth > MONO_MAX_DEPTH) return; + let entries; + try { + entries = readdirSync(absDir, { withFileTypes: true }); + } catch { + return; + } + for (const e of entries) { + if (!e.isDirectory()) continue; + if (e.name.startsWith(".") || WALK_SKIP.has(e.name)) continue; + queue.push({ + dir: join(absDir, e.name), + rel: relDir ? `${relDir}/${e.name}` : e.name, + depth, + }); + } + }; + enqueueChildren(root, "", 1); + while (queue.length && budget > 0 && found.length < MONO_MAX_ROOTS) { + const { dir, rel, depth } = queue.shift(); + budget--; + if (WORKSPACE_MANIFESTS.some((m) => existsSync(join(dir, m)))) found.push(rel); + enqueueChildren(dir, rel, depth + 1); + } + return [...new Set(found)].sort(); +} + /** * One detected test runner. `label` is the human-readable command string (always * mirrored into `testCommands` for back-compat). `bin`/`args` are the structured, @@ -271,10 +395,14 @@ const DETECTORS = [ * Detect the repo's real stack by reading its manifests. Pure aside from fs reads; * every detector is fail-safe. Returns deduped, deterministic (sorted) arrays; * `testRunners` is deduped by label and sorted by label. + * Additive monorepo fields (ME-03): `workspaces` are the declared workspace globs and + * `packageRoots` the nested package/suite roots found on disk (bounded, capped) — either + * being non-empty signals the root suite does NOT necessarily cover the whole repo. Both + * are `[]` for a plain single-root repo, so the pre-existing shape is unchanged. * @param {string} [root] * @returns {{languages:string[], frameworks:string[], packageManagers:string[], * testCommands:string[], testRunners:TestRunner[], tools:string[], notes:string[], - * evidence:string[]}} + * evidence:string[], workspaces:string[], packageRoots:string[]}} */ export function detectStack(root = process.cwd()) { const sets = { @@ -318,5 +446,7 @@ export function detectStack(root = process.cwd()) { tools: sort(sets.tools), notes: sort(sets.notes), evidence: sort(sets.evidence), + workspaces: workspaceGlobs(root), + packageRoots: nestedPackageRoots(root), }; } diff --git a/src/sync.js b/src/sync.js index 1f7bbda..27e0085 100644 --- a/src/sync.js +++ b/src/sync.js @@ -62,6 +62,44 @@ export function loadConfig(targetRoot) { // Warn once per process when a stored legacy profile name is read (RA-14) — loadRules // runs on every sync/drift check and the hooks would otherwise repeat the warning. let warnedLegacyProfile = false; +// Same one-warning-per-process discipline for a corrupt legacy `.forge/rules.json` (ME-20). +let warnedCorruptRules = false; + +/** + * Read the optional legacy `.forge/rules.json` through a guarded parser (ME-20). The + * unified config is already fail-safe (loadConfig), but this override used to be parsed + * with a bare `JSON.parse`, so a single typo threw and ABORTED the whole `forge sync`. + * Now invalid JSON warns once on stderr and falls back to no override — never throws, + * and the file's bytes are left exactly as the user wrote them (nothing is rewritten). + * @param {string} targetRoot + * @returns {{sections:any[], corrupt:boolean, path:string}} + */ +function readLegacyRules(targetRoot) { + const path = join(targetRoot, ".forge/rules.json"); + if (!existsSync(path)) return { sections: [], corrupt: false, path }; + let text; + try { + text = readFileSync(path, "utf8"); + } catch { + return { sections: [], corrupt: false, path }; + } + try { + const extra = JSON.parse(text); + return { + sections: Array.isArray(extra?.sections) ? extra.sections : [], + corrupt: false, + path, + }; + } catch { + if (!warnedCorruptRules) { + warnedCorruptRules = true; + process.stderr.write( + `${BRAND.cli}: ${path} is not valid JSON — ignoring it (fix or delete it); using default rules\n`, + ); + } + return { sections: [], corrupt: true, path }; + } +} /** * Resolve the rule set for a repo with explicit, deterministic override semantics (P1-03): @@ -90,11 +128,8 @@ function loadRules(targetRoot) { const drop = new Set(cfg.disableSections); base.sections = (base.sections || []).filter((s) => !drop.has(s.id) && !drop.has(s.title)); } - const override = join(targetRoot, ".forge/rules.json"); - if (existsSync(override)) { - const extra = JSON.parse(readFileSync(override, "utf8")); - base.sections = [...(base.sections || []), ...(extra.sections || [])]; - } + const legacy = readLegacyRules(targetRoot); + if (legacy.sections.length) base.sections = [...(base.sections || []), ...legacy.sections]; if (Array.isArray(cfg.rules) && cfg.rules.length) { base.sections = [...(base.sections || []), ...cfg.rules]; } @@ -164,9 +199,9 @@ export function sync({ targetRoot = process.cwd() } = {}) { const mcpFile = join(BRAND.root, "source", "mcp.json"); if (existsSync(mcpFile)) { try { - const { servers, owned, warning } = managedMcpState(targetRoot); + const { servers, owns, warning } = managedMcpState(targetRoot); if (warning) warnings.push(warning); - for (const row of emitMcp({ targetRoot, servers, owned })) report.push(row); + for (const row of emitMcp({ targetRoot, servers, owns })) report.push(row); } catch (err) { report.push({ tool: "MCP", @@ -183,6 +218,13 @@ export function sync({ targetRoot = process.cwd() } = {}) { warnings.push( `${cfg.path} is not valid JSON — config ignored, default rules used (fix or delete it)`, ); + // Legacy `.forge/rules.json`: corrupt JSON is ignored (fail-safe, ME-20) — say so in the + // report instead of only on stderr, mirroring the corrupt-config warning above. + const legacyRules = readLegacyRules(targetRoot); + if (legacyRules.corrupt) + warnings.push( + `${legacyRules.path} is not valid JSON — ignored, default rules used (fix or delete it)`, + ); if (backedUp) warnings.push( "existing AGENTS.md was not Forge-managed — backed up to AGENTS.md.forge-bak; move any custom rules into source/rules.json or a per-repo .forge/rules.json", @@ -191,7 +233,21 @@ export function sync({ targetRoot = process.cwd() } = {}) { warnings.push( `canonical is ${bytes} B (> ${SIZE_BUDGET_BYTES} B budget) — trim source/rules.json`, ); - return { hash, bytes, report, warnings, backedUp }; + // Aggregate status (ME-19): sync writes AGENTS.md, then per-tool files, then MCP files + // and is NOT transactional — a mid-way failure is recorded as an `action:"error"` row but + // used to be reported per-target only, so a caller (init/CLI) could still imply every + // tool is ready. Surface an explicit aggregate: if ANY target errored, the whole result + // is PARTIAL. Callers must reflect PARTIAL rather than claim unconditional success. + const partial = report.some((r) => r.action === "error"); + return { + hash, + bytes, + report, + warnings, + backedUp, + partial, + status: partial ? "PARTIAL" : "OK", + }; } /** The assembled canonical body for a repo — same builder sync writes, so drift-check matches. */ diff --git a/test/cli_init.test.js b/test/cli_init.test.js index 8c6266a..71d5aaa 100644 --- a/test/cli_init.test.js +++ b/test/cli_init.test.js @@ -53,6 +53,21 @@ test("init --settings-only announces the GLOBAL merge (informed consent) and exi assert.match(second.stdout, /already up to date/); }); +test("ME-22: full init prints the GLOBAL disclosure BEFORE the merge result line", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-cli-me22-")); + const settingsPath = join(tmp, "home-settings.json"); + const r = runCli(["init"], { cwd: tmp, settingsPath }); + assert.equal(r.status, 0, r.stderr); + const disclosureAt = r.stdout.indexOf("GLOBAL — affects all repos"); + const mergeResultAt = r.stdout.search(/settings: (merged|created) /); + assert.ok(disclosureAt >= 0, "disclosure printed"); + assert.ok(mergeResultAt >= 0, "merge result printed"); + assert.ok( + disclosureAt < mergeResultAt, + "the disclosure must precede the merge it describes (ME-22)", + ); +}); + test("init --remove-settings reverses a merge (exit 0) and fails loudly on a corrupt file", () => { const tmp = mkdtempSync(join(tmpdir(), "forge-cli-remove-")); const settingsPath = join(tmp, "settings.json"); diff --git a/test/doctor.test.js b/test/doctor.test.js index d4afa7d..38bedf1 100644 --- a/test/doctor.test.js +++ b/test/doctor.test.js @@ -1,10 +1,19 @@ import assert from "node:assert/strict"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; +import { BRAND } from "../src/brand.js"; import { processSession } from "../src/cortex_hook.js"; -import { doctor } from "../src/doctor.js"; +import { doctor, repairFailure } from "../src/doctor.js"; +import { mergeSettings } from "../src/init.js"; const fixture = () => mkdtempSync(join(tmpdir(), "forge-doctor-")); @@ -176,3 +185,124 @@ test("doctor --fix records a returned {action:'error'} repair as ok:false (RA-20 assert.equal(rep.ok, false, "a returned error object is not success"); assert.match(rep.error, /not valid JSON/, "the returned reason is surfaced"); }); + +// ME-15: a `_forge` marker plus one unrelated hook is FALSE-GREEN — settings is only ACTIVE +// when the ACTUAL required Forge guard identities from the template are wired. +test("doctor: settings with _forge marker but WITHOUT the required forge hooks is not ACTIVE (ME-15)", () => { + const root = fixture(); + const settingsPath = join(fixture(), "settings.json"); + writeFileSync( + settingsPath, + JSON.stringify({ + _forge: "forge-managed", + hooks: { + Stop: [{ hooks: [{ command: "bash /somewhere/unrelated.sh" }] }], + }, + permissions: { allow: ["Read"] }, + }), + ); + const s = doctor({ targetRoot: root, settingsPath }).results.find((r) => r.label === "settings"); + assert.ok(s, "settings check ran"); + assert.equal(s.status, "warn", "marker + one unrelated hook must not read as healthy"); + assert.match(s.note, /hooks missing|not forge-managed/); +}); + +test("doctor: settings with the real merged forge wiring is ACTIVE (ME-15)", () => { + const root = fixture(); + const settingsPath = join(fixture(), "settings.json"); + // mergeSettings installs the full template hook set + permissions — the genuine wiring. + mergeSettings({ settingsPath }); + const s = doctor({ targetRoot: root, settingsPath }).results.find((r) => r.label === "settings"); + assert.equal(s.status, "ok", "a fully-wired settings file is ACTIVE"); + assert.match(s.note, /guard\(s\) \+ permissions wired/); +}); + +// ME-16: `~/.forge` must be a symlink/dir whose required guard assets resolve — a plain file +// or a dangling symlink is not a working install. +test("doctor: ~/.forge as a plain file is not ACTIVE (ME-16)", () => { + const root = fixture(); + const forgeHome = join(fixture(), ".forge"); + writeFileSync(forgeHome, "not a real install"); + const r = doctor({ targetRoot: root, forgeHome }).results.find((x) => x.label === "~/.forge"); + assert.equal(r.status, "fail", "a stray file shadowing the install is a failure"); + assert.match(r.note, /not a symlink or directory/); +}); + +test("doctor: a dangling ~/.forge symlink is not ACTIVE (ME-16)", () => { + const root = fixture(); + const dir = fixture(); + const forgeHome = join(dir, ".forge"); + symlinkSync(join(dir, "does-not-exist"), forgeHome); // dangling + const r = doctor({ targetRoot: root, forgeHome }).results.find((x) => x.label === "~/.forge"); + assert.equal(r.status, "fail"); + assert.match(r.note, /dangling symlink/); +}); + +test("doctor: a correct ~/.forge install with resolvable guard assets is ACTIVE (ME-16)", () => { + const root = fixture(); + const forgeHome = join(fixture(), ".forge"); + // install.sh symlinks ~/.forge -> /global; point at the real assets so they resolve. + symlinkSync(join(BRAND.root, "global"), forgeHome); + const r = doctor({ targetRoot: root, forgeHome }).results.find((x) => x.label === "~/.forge"); + assert.equal(r.status, "ok", "required guard assets resolve → ACTIVE"); + assert.match(r.note, /guard assets resolve/); +}); + +// ME-17: the redaction subsystem is ACTIVE only when a real self-test through secret-redact.mjs +// masks a fake secret; a missing redactor degrades it instead of reading green off `node`. +test("doctor: a working redactor self-test reports ACTIVE (ME-17)", () => { + const r = doctor({ targetRoot: fixture() }); + const row = r.results.find((x) => x.label === "secret-redact"); + assert.ok(row, "redaction self-test ran"); + assert.equal(row.status, "ok"); + assert.match(row.note, /self-test passed/); + assert.equal(r.health["secret-redaction"], "ACTIVE"); +}); + +test("doctor: a broken redactor (guards dir missing the mjs) is DEGRADED, not ACTIVE (ME-17)", () => { + const emptyGuards = fixture(); // no secret-redact.mjs here + const r = doctor({ targetRoot: fixture(), guardsDir: emptyGuards }); + const row = r.results.find((x) => x.label === "secret-redact"); + assert.equal(row.status, "warn"); + assert.match(row.note, /redactor script missing/); + assert.equal(r.health["secret-redaction"], "DEGRADED"); +}); + +// ME-18: a repair that "succeeds" overall while a nested report row is action:"error" must be +// recorded as a failure with the reason surfaced. +test("doctor: a repair whose report has a nested action:'error' row is a failure (ME-18)", () => { + // sync-shaped report: overall object is fine, but one emitter row errored. + const partialSync = { + hash: "abc", + bytes: 100, + report: [ + { + tool: "shared source", + target: "AGENTS.md", + action: "wrote", + note: "100 B", + }, + { + tool: "cursor", + target: "-", + action: "error", + note: "ENOSPC: no space left on device", + }, + ], + warnings: [], + }; + const reason = repairFailure(partialSync); + assert.ok(reason, "a nested errored row makes the repair a failure"); + assert.match(reason, /sub-step\(s\) failed/); + assert.match(reason, /cursor/); + assert.match(reason, /ENOSPC/); + + // A fully-clean report is not a failure. + const cleanSync = { + report: [{ tool: "cursor", target: ".cursorrules", action: "wrote", note: "ok" }], + }; + assert.equal(repairFailure(cleanSync), null, "a clean report is not a failure"); + + // The {ok:false} (writeForgeConfig) contract is also caught. + assert.match(repairFailure({ ok: false, reason: "corrupt config" }), /corrupt config/); +}); diff --git a/test/init.test.js b/test/init.test.js index 508b6a0..2e76199 100644 --- a/test/init.test.js +++ b/test/init.test.js @@ -13,6 +13,7 @@ import { test } from "node:test"; import { BRAND } from "../src/brand.js"; import { catalog, + ensureLedgerGitattributes, guardKey, init, LEGACY_PROFILES, @@ -22,6 +23,7 @@ import { shellQuote, validateProfile, } from "../src/init.js"; +import { GITATTRIBUTES_RULE } from "../src/ledger_store.js"; test("init emits the shared config for a fresh repo in one call", () => { const root = mkdtempSync(join(tmpdir(), "forge-init-")); @@ -93,6 +95,88 @@ test("mergeSettings refuses to overwrite a present-but-unparseable settings file assert.equal(readFileSync(settingsPath, "utf8"), original, "original bytes preserved"); }); +// ME-12: a settings file that is valid JSON but NOT an object ([] / "x" / 42 / null) is +// corrupt, not empty settings — it must be refused and its bytes preserved, exactly like +// unparseable JSON. Otherwise the merge would silently replace the user's real file. +for (const body of ["[]", '"x"', "42", "null"]) { + test(`ME-12: non-object settings JSON (${body}) is treated as corrupt, bytes preserved`, () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-settings-nonobj-")); + const settingsPath = join(tmp, ".claude", "settings.json"); + mkdirSync(dirname(settingsPath), { recursive: true }); + writeFileSync(settingsPath, body); + const result = mergeSettings({ settingsPath }); + assert.equal(result.action, "error", "non-object settings must not be overwritten"); + assert.equal(readFileSync(settingsPath, "utf8"), body, "original bytes preserved"); + }); +} + +// ME-21: presence of the ledger merge rule is decided by parsing the EXACT active +// attribute line, not a substring — a comment that merely mentions the path must not +// suppress the real rule. +test("ME-21: a .gitattributes with only a COMMENT mentioning the ledger still gets the real rule", () => { + const root = mkdtempSync(join(tmpdir(), "forge-ga-comment-")); + writeFileSync(join(root, ".gitattributes"), "# note: .forge/ledger/ logs use union merge\n"); + const r = ensureLedgerGitattributes(root); + assert.equal(r.written, true, "the real rule was added despite the comment"); + const text = readFileSync(join(root, ".gitattributes"), "utf8"); + assert.match(text, /\.forge\/ledger\/\*\/\*\.log merge=union/, "active rule present"); +}); + +test("ME-21: an existing REAL rule is not duplicated", () => { + const root = mkdtempSync(join(tmpdir(), "forge-ga-real-")); + writeFileSync(join(root, ".gitattributes"), `${GITATTRIBUTES_RULE}\n`); + const r = ensureLedgerGitattributes(root); + assert.equal(r.written, false, "already-present rule is a no-op"); + const text = readFileSync(join(root, ".gitattributes"), "utf8"); + const occurrences = text.split(".forge/ledger/*/*.log merge=union").length - 1; + assert.equal(occurrences, 1, "rule appears exactly once"); +}); + +test("ME-21: a real rule alongside an unrelated comment still counts as present", () => { + const root = mkdtempSync(join(tmpdir(), "forge-ga-mixed-")); + writeFileSync( + join(root, ".gitattributes"), + "# ledger config below\n.forge/ledger/*/*.log merge=union\n", + ); + assert.equal( + ensureLedgerGitattributes(root).written, + false, + "present rule detected, not re-added", + ); +}); + +// ME-22: the GLOBAL-settings disclosure must be emitted BEFORE the merge mutates the file, +// never after. mergeSettings fires onNotice with the target path prior to any write. +test("ME-22: mergeSettings fires the disclosure BEFORE it mutates the global file", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-me22-")); + const settingsPath = join(tmp, ".claude", "settings.json"); + let noticedPath = null; + let fileExistedAtNotice = null; + const r = mergeSettings({ + settingsPath, + onNotice: (p) => { + noticedPath = p; + fileExistedAtNotice = existsSync(settingsPath); + }, + }); + assert.equal(noticedPath, settingsPath, "notice carries the resolved target path"); + assert.equal(fileExistedAtNotice, false, "notice fired before the file was created"); + assert.ok(existsSync(settingsPath), "the merge then created the file"); + assert.ok(r.action === "created" || r.action === "merged"); +}); + +test("ME-22: no disclosure fires when the merge is skipped (--no-settings)", () => { + let fired = false; + const r = mergeSettings({ + noSettings: true, + onNotice: () => { + fired = true; + }, + }); + assert.equal(r.action, "skipped"); + assert.equal(fired, false, "nothing to disclose when nothing is merged"); +}); + test("mergeSettings backs up an existing valid file and resolves guard paths absolutely", () => { const tmp = mkdtempSync(join(tmpdir(), "forge-backup-")); const settingsPath = join(tmp, ".claude", "settings.json"); diff --git a/test/ledger.test.js b/test/ledger.test.js index 6a2f214..2844ef0 100644 --- a/test/ledger.test.js +++ b/test/ledger.test.js @@ -13,13 +13,16 @@ import { mintClaim, outcomeRecord, rec, + refStrength, retrieve, score, sealRecord, shingles, sketch, + UNRESOLVED_VAL_CAP, val, } from "../src/ledger.js"; +import { SERVE_FLOOR } from "../src/reuse.js"; import { fakeAnthropic } from "./_fixtures.js"; // --- canonicalization & content addressing ----------------------------------------- @@ -123,6 +126,85 @@ test("outcomeRecord: typed git ref is resolved; unresolvable is rejected, untype assert.ok(outcomeRecord({ oracle: "test.run", result: "confirm", ref: "run:1" }).ok); }); +test("validateRef: ci: must be a locator, human: must be a ratification (ME-05 format grammars)", () => { + const ok = (ref) => outcomeRecord({ oracle: "ci.run", result: "confirm", ref }).ok; + // A CI locator: URL, owner/repo@run, or a bare run id — but not prose. + assert.ok(ok("ci:https://ci.example.com/run/7"), "URL accepted"); + assert.ok(ok("ci:acme/app@1234"), "owner/repo@run accepted"); + assert.ok(ok("ci:42"), "bare run id accepted (back-compat)"); + assert.equal(ok("ci:not-a-url"), false, "made-up CI string refused on format"); + // human: is an explicit ratification (author@ref), never the model's own say-so. + const okH = (ref) => outcomeRecord({ oracle: "human.accept", result: "confirm", ref }).ok; + assert.ok(okH("human:alice@decision-42"), "explicit human ratification accepted"); + assert.equal(okH("human:the-model-said-yes"), false, "self-assertion refused on format"); +}); + +test("refStrength: resolved (git/ci/human/legacy) vs format-only (file/test)", () => { + assert.equal(refStrength("run:1"), "resolved", "untyped/legacy keeps historical trust"); + assert.equal(refStrength("git:cafebabe"), "resolved"); + assert.equal(refStrength("ci:42"), "resolved"); + assert.equal(refStrength("human:alice@d1"), "resolved"); + assert.equal(refStrength("test:made-up-run"), "format", "a run id is a pointer, not a proof"); + assert.equal(refStrength("file:/some/path"), "format"); +}); + +test("val: format-only evidence (test:/file:) cannot lift confidence into the serving band", () => { + // A single confirm on an UNTYPED (resolved-trust) ref clears the serving floor as before. + const resolved = mkClaim([ + outcomeRecord({ oracle: "test.run", result: "confirm", ref: "run:legit" }).outcome, + ]); + assert.ok(val(resolved, 0) >= SERVE_FLOOR, "resolved evidence still earns trust (no regression)"); + + // But test:/file: refs — however many — are capped below the serving/trusted band. + const madeUp = mkClaim( + Array.from( + { length: 6 }, + (_, i) => + outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: `test:made-up-run-${i}`, + }).outcome, + ), + ); + assert.ok(val(madeUp, 0) < SERVE_FLOOR, "test:made-up-run never reaches the serving floor"); + assert.ok(val(madeUp, 0) <= UNRESOLVED_VAL_CAP + 1e-9, "capped at UNRESOLVED_VAL_CAP"); + + const fileGhost = mkClaim([ + outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: "file:/does/not/exist", + }).outcome, + ]); + assert.ok(val(fileGhost, 0) < SERVE_FLOOR, "file:/does/not/exist cannot lift into serving band"); +}); + +test("val: a resolvable git-ref confirmation raises confidence as before (no regression)", () => { + const git = mkClaim([ + outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: "git:cafebabe", + }).outcome, + ]); + assert.ok(val(git, 0) >= SERVE_FLOOR, "git evidence lifts confidence past the serving floor"); + // A single resolved confirm lifts the whole claim even if a format-only one rides along. + const mixed = mkClaim([ + outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: "git:cafebabe", + }).outcome, + outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: "test:made-up", + }).outcome, + ]); + assert.ok(val(mixed, 0) >= SERVE_FLOOR, "one resolved confirm removes the format-only cap"); +}); + // --- confidence: the decayed Beta posterior ----------------------------------------- const mkClaim = (evidence = []) => { diff --git a/test/ledger_store.test.js b/test/ledger_store.test.js index 6603093..5c9b495 100644 --- a/test/ledger_store.test.js +++ b/test/ledger_store.test.js @@ -30,6 +30,7 @@ import { tombstone, verify, } from "../src/ledger_store.js"; +import { fakeAnthropic, fakeGithubPat } from "./_fixtures.js"; const tmp = () => mkdtempSync(join(tmpdir(), "forge-ledger-")); const fact = (name, text, t = 0) => @@ -344,6 +345,109 @@ test("blame: full accountability view — mints, evidence, retractions, per-auth assert.equal(blame(dir, "zz", 3), null); }); +// --------------------------------------------------------------------------- +// ME-06 — secret-shaped evidence metadata (ref/author), tombstone reasons, and +// provenance are refused BEFORE append; nothing secret ever reaches disk. +// --------------------------------------------------------------------------- + +test("secret metadata is refused before append — evidence ref/author, tombstone reason, provenance", () => { + const dir = tmp(); + const c = fact("target", "no secrets in metadata"); + putClaim(dir, c); + + // (a) a secret in an evidence ref is refused (outcomeRecord won't even build it) + const secretRef = outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: `test:${fakeAnthropic()}`, + }); + assert.equal(secretRef.ok, false, "outcomeRecord refuses a secret-shaped ref"); + assert.match(secretRef.reason, /secret/); + + // (b) a secret in an evidence AUTHOR is refused at the append gate + const secretAuthor = outcomeRecord({ + oracle: "test.run", + result: "confirm", + ref: "run:1", + author: fakeAnthropic(), + }); + assert.equal(secretAuthor.ok, false, "outcomeRecord refuses a secret-shaped author"); + + // Even a hand-built outcome (bypassing outcomeRecord) is refused by appendRecord. + const forgedAuthor = sealRecord({ + author: fakeAnthropic(), + oracle: "test.run", + ref: "run:1", + result: "confirm", + t: 0, + w: 0.8, + }); + const r = appendEvidence(dir, c.id, forgedAuthor); + assert.equal(r.ok, false, "appendEvidence refuses secret-bearing evidence at the boundary"); + + // (c) a secret in a tombstone reason is refused + const tomb = tombstone(dir, c.id, { + reason: fakeGithubPat(), + author: "alice", + t: 1, + }); + assert.equal(tomb.ok, false, "tombstone with a secret reason is refused"); + + // The on-disk logs contain NO secret material. + const evPath = join(dir, "evidence", `${c.id}.log`); + const tsPath = join(dir, "tombstones", `${c.id}.log`); + assert.ok(!existsSync(evPath), "no evidence log was written for the refused records"); + assert.ok(!existsSync(tsPath), "no tombstone log was written for the refused reason"); + // And a full verify() finds no secret-like content anywhere. + assert.ok(!verify(dir).issues.some((i) => /secret/.test(i)), "verify sees no secret on disk"); +}); + +// --------------------------------------------------------------------------- +// ME-07 — quarantine dedups by a TRUSTED hash (not the rejected record's own `h`), +// captures malformed no-`h` lines, and never stores raw secret material. +// --------------------------------------------------------------------------- + +test("mergeDirs: two forged records sharing a fake `h` BOTH quarantine; a no-`h` line is captured; secrets redacted", () => { + const dst = tmp(); + const src = tmp(); + const c = fact("collide", "trusted quarantine identity"); + putClaim(dst, c); + putClaim(src, c); + + // Source evidence log, written raw: two DISTINCT forged records that share one + // attacker-chosen `h`, a malformed line with no `h` at all, and a line whose author + // carries a secret (must be redacted, never persisted raw). + mkdirSync(join(src, "evidence"), { recursive: true }); + writeFileSync( + join(src, "evidence", `${c.id}.log`), + `${[ + '{"author":"attacker-a","h":"deadbeef","oracle":"test.run","ref":"ref-a","result":"confirm","t":0,"w":0.8}', + '{"author":"attacker-b","h":"deadbeef","oracle":"test.run","ref":"ref-b","result":"confirm","t":0,"w":0.8}', + "this line is not json and has no hash", + `{"author":"${fakeAnthropic()}","h":"deadbeef","oracle":"test.run","ref":"ref-c","result":"confirm","t":0,"w":0.8}`, + ].join("\n")}\n`, + ); + + const r = mergeDirs(dst, src); + assert.equal(r.quarantined, 4, "both same-`h` forgeries, the no-`h` line, and the secret line"); + + const qPath = join(dst, "quarantine", `${c.id}.log`); + const qLog = readFileSync(qPath, "utf8"); + // Both distinct forgeries survive (their refs are different — trusted identity, not `h`). + assert.match(qLog, /ref-a/, "first forged record quarantined"); + assert.match(qLog, /ref-b/, "second forged record quarantined (NOT collapsed by shared `h`)"); + // The malformed no-`h` line is captured, not silently dropped. + assert.match(qLog, /this line is not json/, "malformed no-`h` line captured in quarantine"); + // The secret is redacted: the raw credential never reaches disk. + assert.ok(!qLog.includes(fakeAnthropic()), "raw secret is NOT stored in quarantine"); + assert.match(qLog, /REDACTED/, "the secret-bearing record is stored redacted"); + + // Re-merge is idempotent under the trusted identity: nothing new, no duplicate lines. + const again = mergeDirs(dst, src); + assert.equal(again.quarantined, 0, "re-merge quarantines nothing new"); + assert.equal(readFileSync(qPath, "utf8"), qLog, "no duplicate quarantine lines"); +}); + // --------------------------------------------------------------------------- // P0-10 — resolvable typed evidence refs: a git: ref must resolve in THIS repo. // --------------------------------------------------------------------------- diff --git a/test/mcp.test.js b/test/mcp.test.js index c830bba..fc9d7ce 100644 --- a/test/mcp.test.js +++ b/test/mcp.test.js @@ -1,8 +1,9 @@ import assert from "node:assert/strict"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; +import { emitMcp } from "../src/emit/mcp.js"; import { addIntegration, removeIntegration } from "../src/integrations.js"; import { sync } from "../src/sync.js"; @@ -71,8 +72,10 @@ test("integrations add context7 writes it (opt-in) after not being present by de assert.ok(after.mcpServers["forge-cortex"], "existing forge server preserved"); }); -test("managed-entry update: a drifted forge server is refreshed, user servers untouched", () => { +test("ME-09: a divergent user entry with a registry name is preserved + reported, not overwritten", () => { const root = fixture(); + // A JSON entry named like the registry server but diverging from forge's spec is the + // USER's — registry names carry no implicit ownership, so it must survive byte-identical. writeFileSync( join(root, ".mcp.json"), JSON.stringify({ @@ -82,14 +85,20 @@ test("managed-entry update: a drifted forge server is refreshed, user servers un }, }), ); - sync({ targetRoot: root }); + const r = sync({ targetRoot: root }); const j = JSON.parse(readFileSync(join(root, ".mcp.json"), "utf8")); assert.deepEqual( j.mcpServers["forge-cortex"].args, - ["cortex-mcp"], - "stale forge entry refreshed", + ["OLD-STALE-ARG"], + "divergent registry-name entry preserved (not clobbered without adoption)", ); assert.ok(j.mcpServers.mine, "user server preserved"); + const row = r.report.find((x) => x.target === ".mcp.json"); + assert.match(row.note, /user-owned/); + assert.match(row.note, /--adopt/); + // Absent targets still receive the registry server untouched. + const cursor = JSON.parse(readFileSync(join(root, ".cursor", "mcp.json"), "utf8")); + assert.ok(cursor.mcpServers["forge-cortex"], "registry server written where absent"); }); test("sync emits Continue rules (Continue does not read AGENTS.md)", () => { @@ -157,7 +166,10 @@ test("RA-03: sync → integrations add → sync keeps BOTH servers in every targ assertServersEverywhere(root, ["forge-cortex", "context7"], "after second sync"); const cfg = JSON.parse(readFileSync(join(root, ".forge", "forge.config.json"), "utf8")); assert.deepEqual(cfg.mcp.integrations, ["context7"], "install recorded in the managed set"); - assert.ok(cfg.mcp.adopted.includes("context7"), "fresh create is forge-owned (auto-adopted)"); + assert.ok( + cfg.mcp.adopted.some((a) => a.server === "context7" && a.target === ".mcp.json"), + "fresh create is forge-owned per target (auto-adopted)", + ); }); test("Codex: a stale forge-marked block is refreshed on the next emit", () => { @@ -287,7 +299,9 @@ test("integrations remove never deletes a user-owned same-name entry", () => { const root = fixture(); writeFileSync( join(root, ".mcp.json"), - JSON.stringify({ mcpServers: { context7: { command: "my-custom-context7" } } }), + JSON.stringify({ + mcpServers: { context7: { command: "my-custom-context7" } }, + }), ); addIntegration("context7", { targetRoot: root }); // not adopted — user entry preserved const res = removeIntegration("context7", { targetRoot: root }); @@ -317,3 +331,146 @@ test("corrupt forge.config.json: sync emits registry-only + warns; add refuses; "corrupt config bytes never overwritten", ); }); + +// --------------------------------------------------------------------------- +// ME-08..ME-11 — per-target ownership, atomic add/remove, name/definition validation. +// --------------------------------------------------------------------------- + +const writeForgeConfigFile = (root, mcp) => { + mkdirSync(join(root, ".forge"), { recursive: true }); + writeFileSync(join(root, ".forge", "forge.config.json"), `${JSON.stringify({ mcp }, null, 2)}\n`); +}; + +test("ME-08: per-target adoption does not leak to other targets; legacy bare name honored", () => { + const root = fixture(); + // A divergent same-name context7 in TWO targets. + writeFileSync( + join(root, ".mcp.json"), + JSON.stringify({ mcpServers: { context7: { command: "custom-a" } } }), + ); + mkdirSync(join(root, ".cursor"), { recursive: true }); + writeFileSync( + join(root, ".cursor", "mcp.json"), + JSON.stringify({ mcpServers: { context7: { command: "custom-b" } } }), + ); + // Adopt context7 for .mcp.json ONLY (per-target record). + writeForgeConfigFile(root, { + integrations: ["context7"], + adopted: [{ server: "context7", target: ".mcp.json" }], + }); + sync({ targetRoot: root }); + const a = JSON.parse(readFileSync(join(root, ".mcp.json"), "utf8")); + const b = JSON.parse(readFileSync(join(root, ".cursor", "mcp.json"), "utf8")); + assert.equal(a.mcpServers.context7.command, "npx", "adopted target refreshed to catalog spec"); + assert.equal(b.mcpServers.context7.command, "custom-b", "non-adopted target NOT overwritten"); + + // Legacy bare name is honored as a wildcard: both targets refresh. + writeForgeConfigFile(root, { integrations: ["context7"], adopted: ["context7"] }); + writeFileSync( + join(root, ".cursor", "mcp.json"), + JSON.stringify({ mcpServers: { context7: { command: "custom-b" } } }), + ); + sync({ targetRoot: root }); + const b2 = JSON.parse(readFileSync(join(root, ".cursor", "mcp.json"), "utf8")); + assert.equal(b2.mcpServers.context7.command, "npx", "legacy bare adopted name honored for all"); +}); + +test("ME-08: add over a divergent entry in one target does not adopt it in others", () => { + const root = fixture(); + // context7 diverges only in .mcp.json; absent elsewhere. + writeFileSync( + join(root, ".mcp.json"), + JSON.stringify({ mcpServers: { context7: { command: "mine" } } }), + ); + const res = addIntegration("context7", { targetRoot: root }); + assert.ok(res.ok); + assert.equal(res.adopted, false, "pre-existing divergent entry not auto-adopted"); + const cfg = JSON.parse(readFileSync(join(root, ".forge", "forge.config.json"), "utf8")); + const owns = (t) => cfg.mcp.adopted.some((a) => a.server === "context7" && a.target === t); + assert.equal(owns(".mcp.json"), false, "divergent target NOT adopted"); + assert.equal(owns(".cursor/mcp.json"), true, "fresh target auto-adopted"); + // The divergent entry survives byte-identical. + const a = JSON.parse(readFileSync(join(root, ".mcp.json"), "utf8")); + assert.equal(a.mcpServers.context7.command, "mine", "user entry preserved"); +}); + +test("ME-10 add: a failing target does not record a fully-installed integration", () => { + const root = fixture(); + // Make .cursor a FILE so mkdirSync(.cursor) throws when emitting .cursor/mcp.json. + writeFileSync(join(root, ".cursor"), "not a directory"); + const res = addIntegration("context7", { targetRoot: root }); + assert.equal(res.ok, false, "partial emit is not a success"); + assert.ok(res.incomplete, "reported as incomplete"); + assert.ok( + res.rows.some((r) => r.action === "error"), + "a target reported an error", + ); + // The install was NOT recorded — a later add can finish. + const cfgPath = join(root, ".forge", "forge.config.json"); + if (existsSync(cfgPath)) { + const cfg = JSON.parse(readFileSync(cfgPath, "utf8")); + assert.ok( + !(cfg.mcp?.integrations || []).includes("context7"), + "context7 not recorded as installed after partial emit", + ); + } +}); + +test("ME-10 remove: a failing target cleanup keeps the integration recorded (recoverable)", () => { + const root = fixture(); + sync({ targetRoot: root }); + addIntegration("context7", { targetRoot: root }); + // Sabotage the Continue per-server file: replace it with a directory so cleanup throws. + const contFile = join(root, ".continue", "mcpServers", "forge-context7.yaml"); + rmSync(contFile, { force: true }); + mkdirSync(contFile, { recursive: true }); + const res = removeIntegration("context7", { targetRoot: root }); + assert.equal(res.removed, false, "not reported as fully removed"); + assert.ok( + res.rows.some((r) => r.action === "error"), + "a target cleanup errored", + ); + const cfg = JSON.parse(readFileSync(join(root, ".forge", "forge.config.json"), "utf8")); + assert.ok( + cfg.mcp.integrations.includes("context7"), + "integration kept in the managed set so a later remove can finish", + ); +}); + +test("ME-11: invalid server names are rejected before any write", () => { + const root = fixture(); + for (const bad of ["../x", "foo bar", "foo\nbar", "foo.bar", "foo]bar", "Foo", "-foo"]) { + assert.throws( + () => emitMcp({ targetRoot: root, servers: { [bad]: { command: "x" } } }), + /invalid MCP server name/, + `rejected: ${JSON.stringify(bad)}`, + ); + } + // Nothing was written for the rejected set. + assert.ok(!existsSync(join(root, ".mcp.json")), "no file written on validation failure"); +}); + +test("ME-11: foo vs forge-foo Continue filename collision is rejected", () => { + const root = fixture(); + assert.throws( + () => + emitMcp({ + targetRoot: root, + servers: { foo: { command: "x" }, "forge-foo": { command: "y" } }, + }), + /collide on Continue file/, + ); +}); + +test("ME-11: a YAML-special command is serialized safely (quoted)", () => { + const root = fixture(); + emitMcp({ + targetRoot: root, + servers: { srv: { command: "cmd: --flag #danger", args: ["a: b"] } }, + }); + const yaml = readFileSync(join(root, ".continue", "mcpServers", "forge-srv.yaml"), "utf8"); + assert.match(yaml, /command: "cmd: --flag #danger"/, "command quoted as a YAML scalar"); + // And the same command lands safely (JSON-quoted) in the Codex TOML. + const toml = readFileSync(join(root, ".codex", "config.toml"), "utf8"); + assert.match(toml, /command = "cmd: --flag #danger"/, "command quoted as a TOML string"); +}); diff --git a/test/stack.test.js b/test/stack.test.js index ea2e5e5..eb47c7a 100644 --- a/test/stack.test.js +++ b/test/stack.test.js @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { mkdtempSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; @@ -126,6 +126,64 @@ test("testRunners: structured descriptors alongside UNCHANGED testCommands strin ]); }); +test("ME-03: npm workspaces + nested packages → workspaces + packageRoots surfaced", () => { + const root = tmp(); + writeFileSync( + join(root, "package.json"), + JSON.stringify({ + name: "monorepo", + private: true, + workspaces: ["packages/*", "apps/*"], + }), + ); + mkdirSync(join(root, "packages", "core"), { recursive: true }); + writeFileSync(join(root, "packages", "core", "package.json"), JSON.stringify({ name: "core" })); + mkdirSync(join(root, "apps", "web"), { recursive: true }); + writeFileSync(join(root, "apps", "web", "package.json"), JSON.stringify({ name: "web" })); + // A nested Python package under the same tree must also be surfaced. + mkdirSync(join(root, "services", "api"), { recursive: true }); + writeFileSync(join(root, "services", "api", "pyproject.toml"), "[project]\nname='api'\n"); + + const s = detectStack(root); + assert.deepEqual(s.workspaces, ["apps/*", "packages/*"], JSON.stringify(s.workspaces)); + assert.ok(s.packageRoots.includes("packages/core"), JSON.stringify(s.packageRoots)); + assert.ok(s.packageRoots.includes("apps/web")); + assert.ok( + s.packageRoots.includes("services/api"), + "nested non-root suites (even outside the globs) must not be silently ignored", + ); +}); + +test("ME-03: pnpm-workspace.yaml globs are parsed (zero-dep) and negations ignored", () => { + const root = tmp(); + writeFileSync(join(root, "package.json"), JSON.stringify({ name: "pnpm-mono" })); + writeFileSync( + join(root, "pnpm-workspace.yaml"), + 'packages:\n - "packages/*"\n - "!**/__tests__/**"\n', + ); + mkdirSync(join(root, "packages", "lib"), { recursive: true }); + writeFileSync(join(root, "packages", "lib", "package.json"), JSON.stringify({ name: "lib" })); + const s = detectStack(root); + assert.deepEqual(s.workspaces, ["packages/*"], "negation entry dropped"); + assert.ok(s.packageRoots.includes("packages/lib")); +}); + +test("ME-03: a plain single-root repo has empty workspaces/packageRoots (shape unchanged)", () => { + const root = tmp(); + writeFileSync( + join(root, "package.json"), + JSON.stringify({ + dependencies: { next: "16" }, + scripts: { test: "vitest" }, + }), + ); + const s = detectStack(root); + assert.deepEqual(s.workspaces, [], "no workspace config → no globs"); + assert.deepEqual(s.packageRoots, [], "no nested manifests → no extra roots"); + // Pre-existing surface is untouched. + assert.ok(s.frameworks.includes("Next.js")); +}); + test("CLI: forge stack --json emits the detected stack", () => { const root = tmp(); writeFileSync(join(root, "go.mod"), "module x\n\ngo 1.22\n"); diff --git a/test/sync.test.js b/test/sync.test.js index 090434f..4614286 100644 --- a/test/sync.test.js +++ b/test/sync.test.js @@ -152,6 +152,63 @@ test("config disableSections drops a named section; config.rules appends (P1-03) assert.match(md, /## Workflow/, "other sections preserved"); }); +test("ME-20: invalid .forge/rules.json is fail-safe — sync does not throw, falls back, bytes preserved", () => { + const root = fixture(); + mkdirSync(join(root, ".forge"), { recursive: true }); + const rulesPath = join(root, ".forge/rules.json"); + const bad = '{ "sections": [ this is not json '; + writeFileSync(rulesPath, bad); + let res; + assert.doesNotThrow(() => { + res = sync({ targetRoot: root }); + }, "corrupt legacy rules.json must never abort sync"); + // Default rules still generated (fail-open). + assert.match( + readFileSync(join(root, "AGENTS.md"), "utf8"), + /## Workflow/, + "default rules emitted despite the corrupt override", + ); + // Surfaced as a warning, not swallowed. + assert.ok( + res.warnings.some((w) => w.includes("rules.json") && w.includes("not valid JSON")), + `warnings must mention the corrupt rules.json: ${JSON.stringify(res.warnings)}`, + ); + // Never overwritten — the user's bytes are left intact to fix. + assert.equal(readFileSync(rulesPath, "utf8"), bad, "corrupt rules.json bytes preserved"); +}); + +test("ME-20: valid .forge/rules.json with a non-array sections field is ignored, not thrown", () => { + const root = fixture(); + mkdirSync(join(root, ".forge"), { recursive: true }); + writeFileSync(join(root, ".forge/rules.json"), JSON.stringify({ sections: "oops" })); + assert.doesNotThrow(() => sync({ targetRoot: root })); + assert.match(readFileSync(join(root, "AGENTS.md"), "utf8"), /## Workflow/); +}); + +test("ME-19: a failing target write yields PARTIAL status, not unconditional success", () => { + const root = fixture(); + // Make one emit target (CLAUDE.md) un-writable by planting a DIRECTORY at its path — the + // claude emitter's read/write then throws, sync catches it as an action:"error" row. + mkdirSync(join(root, "CLAUDE.md"), { recursive: true }); + let res; + assert.doesNotThrow(() => { + res = sync({ targetRoot: root }); + }, "one failing target must not abort the whole sync"); + const errors = res.report.filter((r) => r.action === "error"); + assert.ok(errors.length >= 1, "the failed target is recorded as an error row"); + assert.equal(res.partial, true, "any error row makes the aggregate PARTIAL"); + assert.equal(res.status, "PARTIAL"); + // Other targets still succeeded — AGENTS.md was written before the failure. + assert.ok(existsSync(join(root, "AGENTS.md")), "earlier targets are still emitted"); +}); + +test("ME-19: an all-clean sync reports status OK / partial:false", () => { + const root = fixture(); + const res = sync({ targetRoot: root }); + assert.equal(res.partial, false); + assert.equal(res.status, "OK"); +}); + test("RA-16: a hand-edited AGENTS.md body with an INTACT marker still counts as drift", () => { const root = fixture(); sync({ targetRoot: root }); diff --git a/test/tools.test.js b/test/tools.test.js index 9e98f19..d184a0b 100644 --- a/test/tools.test.js +++ b/test/tools.test.js @@ -1,6 +1,13 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; @@ -262,3 +269,108 @@ test("KNOWN_TOOLS covers the documented set", () => { for (const t of ["claude", "cursor", "gemini", "codex", "zed", "vscode"]) assert.ok(KNOWN_TOOLS.includes(t)); }); + +// ME-12: valid-but-non-object top-level JSON (null / [] / "x" / 42) must be treated as +// CORRUPT, not coerced to {} — otherwise a later write silently replaces valid bytes. +for (const body of ["null", "[]", '"oops"', "42"]) { + test(`ME-12: non-object forge.config.json (${body}) is corrupt, not silently discarded`, () => { + const root = tmp(); + mkdirSync(join(root, ".forge"), { recursive: true }); + const file = join(root, ".forge/forge.config.json"); + writeFileSync(file, body); + + // read: reported corrupt, never thrown, never coerced to usable data + const cfg = readForgeConfig(root); + assert.equal(cfg.corrupt, true, "corruption reported"); + assert.equal(cfg.path, file); + assert.deepEqual(readRepoConfig(root), {}, "no bogus primary tool surfaces"); + + // write: refused, bytes preserved for the human to fix + const res = writeForgeConfig(root, (c) => { + c.profile = "minimal"; + }); + assert.equal(res.ok, false, "write refuses"); + assert.equal(readFileSync(file, "utf8"), body, "original bytes preserved"); + }); +} + +// ME-13: writes go through a temp file + atomic rename, and an existing valid config is +// backed up (timestamped) before it is overwritten — a crash can never truncate the config. +test("ME-13: writeForgeConfig backs up the existing config and writes atomically", () => { + const root = tmp(); + mkdirSync(join(root, ".forge"), { recursive: true }); + const file = join(root, ".forge/forge.config.json"); + const original = `${JSON.stringify({ profile: "minimal", keep: "me" }, null, 2)}\n`; + writeFileSync(file, original); + + const res = writeForgeConfig(root, (c) => { + c.primaryTool = "claude"; + }); + assert.equal(res.ok, true); + + // new content is intact and complete (no truncation), unknown keys round-tripped + const written = JSON.parse(readFileSync(file, "utf8")); + assert.deepEqual(written, { + profile: "minimal", + keep: "me", + primaryTool: "claude", + }); + + // a timestamped backup of the ORIGINAL bytes appears next to the config + const backups = readdirSync(join(root, ".forge")).filter((f) => + f.startsWith("forge.config.json.forge-bak-"), + ); + assert.equal(backups.length, 1, "one backup written"); + assert.equal( + readFileSync(join(root, ".forge", backups[0]), "utf8"), + original, + "backup holds the pre-overwrite bytes", + ); + // no temp file is left behind after the rename + assert.ok( + !readdirSync(join(root, ".forge")).some((f) => f.includes(".forge-tmp-")), + "temp file renamed away", + ); +}); + +test("ME-13: first write (no existing config) needs no backup", () => { + const root = tmp(); + const res = writeForgeConfig(root, (c) => { + c.primaryTool = "claude"; + }); + assert.equal(res.ok, true); + const backups = readdirSync(join(root, ".forge")).filter((f) => f.includes(".forge-bak-")); + assert.deepEqual(backups, [], "no backup for a fresh config"); +}); + +// ME-14: KNOWN_TOOLS and primary-tool auto-detection are reconciled with the emit targets — +// Roo is selectable and every tool with an on-disk marker is detected. +test("ME-14: KNOWN_TOOLS includes roo/aider/continue/windsurf and detection recognizes them", () => { + for (const t of ["aider", "continue", "windsurf", "roo"]) + assert.ok(KNOWN_TOOLS.includes(t), `${t} is selectable via \`forge tools\``); + + // Each tool's marker auto-detects that tool. + const aiderRoot = tmp(); + writeFileSync(join(aiderRoot, ".aider.conf.yml"), "read: AGENTS.md\n"); + assert.equal(detectPrimaryTool(aiderRoot), "aider"); + + const contRoot = tmp(); + mkdirSync(join(contRoot, ".continue")); + assert.equal(detectPrimaryTool(contRoot), "continue"); + + const windRoot = tmp(); + mkdirSync(join(windRoot, ".windsurf")); + assert.equal(detectPrimaryTool(windRoot), "windsurf"); + + const rooRoot = tmp(); + mkdirSync(join(rooRoot, ".roo")); + assert.equal(detectPrimaryTool(rooRoot), "roo"); + assert.equal(rowToolKey("Roo Code MCP"), "roo", "emit label maps to the same key"); +}); + +test("ME-14: `forge tools roo` is accepted (roo is a known tool)", async () => { + const root = tmp(); + const r = await applyPrimaryTool(root, "roo", { syncFn: () => FAKE_REPORT }); + assert.equal(r.primaryTool, "roo"); + assert.equal(readRepoConfig(root).primaryTool, "roo"); +});