diff --git a/CHANGELOG.md b/CHANGELOG.md index 25a1f12..3506960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,52 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). comments no longer claim strict mode "blocks": PostToolUse fires after the tool ran, so exit 2 is surfaced stderr feedback, not enforcement — blocking belongs to the PreToolUse guard. A wrapper-level regression test drives the real `secret-redact.sh`. +- **Guard blocks shell writes to protected paths (HI-06).** The PreToolUse guard now blocks + writes/mutations to protected paths (`.env`, keys, `secrets/`, `.ssh/`) via redirections and + truncations (`> .env`, `: > .env`), `tee`, `sed -i`, `cp`/`mv`/`install`, and `dd of=`, not + just reads. Interpreter-driven writes (`python -c`, `node -e`) remain out of scope and are + documented as such — this is defence in depth, not a sandbox. +- **Hardened git secret-reader detection (HI-07).** Reader detection now sees through wrappers + and global options (`env`/`command`/`VAR=val` prefixes, `/usr/bin/git`, `-C`/`--no-pager`/`-c`/ + `--git-dir`/`--work-tree`) and covers `git blame`/`show-index`/`bundle`. +- **Broader hook tool coverage (HI-08).** The protect-paths PreToolUse matcher now includes + `NotebookEdit`, and the secret-redact PostToolUse matcher includes `WebFetch`, `NotebookEdit`, + and MCP tools (`mcp__.*`); the settings template and plugin manifest stay in sync. + +### Fixed + +- **`forge verify` runs every detected suite (HI-01).** A polyglot repo where a passing Node + suite hid a failing or unexecuted pytest suite no longer reports PASS — every detected + executable suite runs, and a non-executable or missing one makes the overall result + INCOMPLETE. Per-suite `executed`/`notExecuted` detail is recorded. +- **Verification is bound to the final code state (HI-02).** Provenance records a `codeState` + fingerprint (git HEAD + a hash of working/staged/untracked changes); the completion gate only + counts a `forge verify` PASS as evidence when that fingerprint still matches at Stop, so code + edited _after_ verification no longer passes on a stale stamp. +- **Completion gate detects edits to pre-dirty files (HI-03).** The session baseline stores a + content fingerprint per already-dirty file, so further edits to it during the session are seen + instead of hidden by its start-of-session dirty state. +- **A changed test file is no longer proof (HI-04).** A deleted or empty test file no longer + satisfies the code-change evidence requirement; the test leg needs a substantive (existing, + non-empty) test file or a code-state-bound verify PASS. +- **Honest spawn and metrics classification (ME-01, ME-02).** Spawn failures (missing binary, + permission/exec-format errors, signal kills, timeout) are reported INCOMPLETE rather than a + false test FAIL — only a real non-zero exit counts as FAIL; deep-verify metrics no longer + count NOT_CONFIGURED/INCOMPLETE runs as a pass. Provenance records `gitAvailable` so a failure + to detect changed files is never reported as a clean tree (ME-04). +- **Ownership-safe settings uninstall (HI-05).** The settings merge records an `_forgeOwned` + manifest of exactly what it added (permissions genuinely absent before, hook guard-identities, + statusLine, `$schema`); `forge init --remove-settings` reverses only those, never a user-owned + entry that predates Forge. Ownership is path-aware, so a user's custom hook that merely shares + a basename with a Forge hook is never claimed or removed; a legacy scan seeds the manifest for + pre-manifest installs. +- **Init aborts on profile-persistence failure (HI-09).** When `forge init --profile` can't + persist because `.forge/forge.config.json` is corrupt, init now aborts before sync, + `.gitattributes`, and the settings merge — zero side effects — instead of continuing and + reporting the error afterward. +- **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. ### Changed diff --git a/global/guards/protect-paths.sh b/global/guards/protect-paths.sh index 48f0be3..630bb6e 100755 --- a/global/guards/protect-paths.sh +++ b/global/guards/protect-paths.sh @@ -45,16 +45,43 @@ if [ -n "${cmd:-}" ]; then # message mentioning ".env") isn't a false positive, AND require a protected path token. # Best-effort defence in depth — a content scan like `rg TOKEN .` with no named path can't # be caught here; that's what secret-redact.sh is for. - # Git subcommands that can print file/history content (RA-05): show, log, diff, - # stash (show -p), cat-file, archive, grep. Subcommand may be followed by a space - # or end the command string. - reader='(^|[;&|])[[:space:]]*((cat|less|more|head|tail|nl|xxd|od|strings|base64|rg|grep|ag)[[:space:]]|git[[:space:]]+(show|log|diff|stash|cat-file|archive|grep)([[:space:]]|$))' + # + # SCOPE: this is a POSIX-ERE regex guard, not a sandbox. Regex cannot parse shell, so + # interpreter-driven access/writes — `python -c 'open(".env","w")…'`, `node -e …`, `perl -e` + # — are DELIBERATELY out of scope here. This layer sits behind the permission system and + # secret-redact.sh; treat every match/miss as best-effort hardening, never a boundary. + # + # Git subcommands that can print file/history content (RA-05, HI-07): show, log, diff, + # stash (show -p), cat-file, archive, grep, blame, show-index, bundle. Subcommand may be + # followed by a space or end the command string. + # HI-07 (best-effort hardening): tolerate an optional `env `/`command ` or `VAR=val ` + # prefix and an optional absolute/relative path before `git` (e.g. `/usr/bin/git`), then + # skip git's own global options (`-C `, `--no-pager`, `-c k=v`, `--git-dir=…`, + # `--work-tree=…`) between `git` and the subcommand. A wrapper we don't model can still slip. + gitpfx='([[:alnum:]_]+=[^[:space:]]+[[:space:]]+|(env|command)[[:space:]]+)*([^[:space:]]*/)?git[[:space:]]+' + gitopt='(-C[[:space:]]+[^[:space:]]+[[:space:]]+|--no-pager[[:space:]]+|-c[[:space:]]+[^[:space:]]+[[:space:]]+|--git-dir=[^[:space:]]+[[:space:]]+|--work-tree=[^[:space:]]+[[:space:]]+)*' + gitsub='(show|log|diff|stash|cat-file|archive|grep|blame|show-index|bundle)([[:space:]]|$)' + reader="(^|[;&|])[[:space:]]*((cat|less|more|head|tail|nl|xxd|od|strings|base64|rg|grep|ag)[[:space:]]|${gitpfx}${gitopt}${gitsub})" # \b anchors the extensions so `.key` matches a real key file but NOT `Object.keys`, # and `.env` matches `.env`/`.env.prod` but NOT `.environment`. secret='(\.env(\.[A-Za-z0-9_-]+)?\b|id_rsa\b|id_ed25519\b|\.pem\b|\.key\b|/secrets/|/\.ssh/)' if printf '%s' "$cmd" | grep -qE "$reader" && printf '%s' "$cmd" | grep -qE "$secret"; then deny "reading a protected secret path via Bash is blocked. Read it yourself if intended." fi + # Close the Bash secret-WRITE bypass (HI-06): the Edit/Write guard covers tool writes, but a + # shell `echo x > .env`, `printf … >> .env`, `tee .env`, `sed -i … .env`, `cp/mv/install X + # .env`, `dd of=.env`, or a truncation (`> .env`, `: > .env`) mutates a protected file past + # it. Detect a protected-path token appearing (a) as a redirection target (`>`/`>>` then the + # path) or (b) as an argument to a known mutating command. Interpreter writes are out of + # scope (see SCOPE note above). Each alternative embeds "$secret", so a bare `echo hi > + # out.txt` (no protected token) is never blocked. + w_redir=">>?[[:space:]]*[\"']?[^[:space:]<>|;&]*${secret}" + w_cmd="(^|[;&|])[[:space:]]*(${gitpfx})?(tee([[:space:]]+-a)?|cp|mv|install)[[:space:]]+[^;&|]*${secret}" + w_sed="(^|[;&|])[[:space:]]*sed[[:space:]]+[^;&|]*-i[^;&|]*${secret}" + w_dd="(^|[;&|])[[:space:]]*dd[[:space:]]+([^;&|]*[[:space:]])?of=[^[:space:]]*${secret}" + if printf '%s' "$cmd" | grep -qE "${w_redir}|${w_cmd}|${w_sed}|${w_dd}"; then + deny "writing to a protected secret path via Bash is blocked. Edit it yourself if intended." + fi # Pipe-to-shell (e.g. curl … | sh). Boundary-aware so legit `… | shellcheck` is not caught. if [[ "$cmd" =~ \|[[:space:]]*(sh|bash|zsh)([[:space:]]|$) ]]; then deny "piping content to a shell is blocked." diff --git a/global/settings.template.json b/global/settings.template.json index 504b4e2..1deeb8c 100644 --- a/global/settings.template.json +++ b/global/settings.template.json @@ -102,7 +102,7 @@ ], "PreToolUse": [ { - "matcher": "Edit|Write|MultiEdit|Bash", + "matcher": "Edit|Write|MultiEdit|NotebookEdit|Bash", "hooks": [ { "type": "command", @@ -143,7 +143,7 @@ ] }, { - "matcher": "Bash|Read|Grep", + "matcher": "Bash|Read|Grep|WebFetch|NotebookEdit|mcp__.*", "hooks": [ { "type": "command", diff --git a/hooks/hooks.json b/hooks/hooks.json index 09a5ef7..fe5c973 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -31,7 +31,7 @@ ], "PreToolUse": [ { - "matcher": "Edit|Write|MultiEdit|Bash", + "matcher": "Edit|Write|MultiEdit|NotebookEdit|Bash", "hooks": [ { "type": "command", @@ -72,7 +72,7 @@ ] }, { - "matcher": "Bash|Read|Grep", + "matcher": "Bash|Read|Grep|WebFetch|NotebookEdit|mcp__.*", "hooks": [ { "type": "command", diff --git a/install.sh b/install.sh index b329f99..ba46bbe 100644 --- a/install.sh +++ b/install.sh @@ -35,11 +35,19 @@ say() { printf ' %s\n' "$*"; } # Run argv directly (no eval — S-02). In dry-run, print the argv instead of executing. act() { if [ "$DRY" = 1 ]; then say "[dry-run] $*"; else "$@"; fi; } -# link SRC DEST — back up an existing real file/dir, then symlink. +# link SRC DEST — back up an existing real file/dir OR a foreign symlink, then symlink. +# A symlink already pointing into this repo (Forge's own) is refreshed silently; any other +# existing symlink is a USER symlink and is backed up before being replaced, so `ln -sfn` +# never silently destroys it (HI-10). link() { local src="$1" dest="$2" [ -e "$src" ] || { say "skip (missing in bundle): $src"; return; } - if [ -e "$dest" ] && [ ! -L "$dest" ]; then + if [ -L "$dest" ]; then + case "$(readlink "$dest")" in + "$REPO"/*) : ;; # our own symlink — safe to refresh in place + *) act mv "$dest" "$dest.forge-bak-$STAMP"; say "backed up existing symlink $dest" ;; + esac + elif [ -e "$dest" ]; then act mv "$dest" "$dest.forge-bak-$STAMP"; say "backed up existing $dest" fi act mkdir -p "$(dirname "$dest")" @@ -136,7 +144,14 @@ uninstall_forge() { printf '%s\n' "$REMOVE_OUT" | sed 's/^/ /' else [ -n "$REMOVE_OUT" ] && printf '%s\n' "$REMOVE_OUT" | sed 's/^/ /' - say "note: automatic settings cleanup failed — remove the Forge hook/permission/statusline entries from $CLAUDE_DIR/settings.json by hand, or fix the file and run: forge init --remove-settings" + # Removing the asset symlinks now would leave GLOBAL settings hooks firing scripts that + # no longer exist — the dangerous case (HI-10). Stop with the assets intact instead of + # printing a false "Done." + cat >&2 < `${s.label}=${s.status}`).join(", ")}`, + ); + if (t.notExecuted?.length) + console.log(` suites skipped: ${t.notExecuted.join(", ")} (no built-in executor)`); const detail = t.output || (t.detected ?? []).join(", "); const verdictLine = r.status === "PASS" @@ -1150,6 +1156,14 @@ async function run(argv) { ? `— INCOMPLETE: ${t.output || (t.detected ?? []).join(", ") || "test run did not complete"}` : "— NOT CONFIGURED (no test runner detected)"; console.log(` tests: ${testsLine}`); + // Per-suite honesty (HI-01): a polyglot repo runs every executable suite; name which + // ones ran with what verdict, and which were detected but never executed. + if (t.executed?.length) + console.log( + ` suites ran: ${t.executed.map((s) => `${s.label}=${s.status}`).join(", ")}`, + ); + if (t.notExecuted?.length) + console.log(` suites skipped: ${t.notExecuted.join(", ")} (no built-in executor)`); console.log(` symbols checked: ${r.provenance.symbolsChecked}`); if (r.unknown.length) console.log( diff --git a/src/consensus.js b/src/consensus.js index c043329..d5508f4 100644 --- a/src/consensus.js +++ b/src/consensus.js @@ -349,9 +349,16 @@ export function verifyDeep({ } catch {} recordMetric(targetRoot, { stage: "verify", - // `outcome` keeps its historical block|pass vocabulary; the four-state verdict - // rides along additively as `status`. - outcome: verdict.block ? "block" : "pass", + // `outcome` is derived from the FINAL four-state status, not `verdict.block` alone + // (ME-01): an unverified run (NOT_CONFIGURED / INCOMPLETE) must never be counted as a + // "pass". PASS→pass, FAIL/block→block, and the unverified states carry their own + // lower-cased label. The four-state verdict still rides along additively as `status`. + outcome: + status === "PASS" + ? "pass" + : status === "FAIL" || verdict.block + ? "block" + : status.toLowerCase(), status, mode: "deep", lenses: deep.lenses.filter((l) => l.ran).length, diff --git a/src/gate.js b/src/gate.js index bc74abc..cc7f281 100644 --- a/src/gate.js +++ b/src/gate.js @@ -22,9 +22,10 @@ import { BRAND } from "./brand.js"; import { readSession, sessionPath } from "./cortex_hook.js"; import { decisionsPath } from "./decide.js"; import { statePath } from "./handoff.js"; -import { readBaseline, readDirtySnapshot } from "./session.js"; +import { fingerprintFile, readBaseline, readDirtySnapshot } from "./session.js"; import { isTestFile } from "./substrate.js"; import { IGNORE_DIRS } from "./util.js"; +import { computeCodeState } from "./verify.js"; // gitRaw keeps the exact bytes — porcelain's first column is a SPACE for unstaged // entries, and a trim() would eat it and shift the path slice by one. @@ -111,12 +112,19 @@ const IGNORED_PREFIX = (p) => IGNORE_DIRS.has(String(p).split("/")[0]); * by the block-once marker. * @param {string} root * @param {string|null} [baseHead] - * @param {{sinceMs?: number, preDirty?: Set}} [opts] + * @param {{sinceMs?: number, preDirty?: Map}} [opts] */ export function changedSet(root, baseHead, { sinceMs, preDirty } = {}) { const out = new Set(committedSince(root, baseHead, sinceMs)); for (const p of statusEntriesZ(root)) { - if (preDirty?.has(p)) continue; // already dirty before the session began + // A path that was already dirty at session start is hidden ONLY while its content is + // unchanged since then: its baseline fingerprint must be known AND still match. An + // unknown baseline (legacy snapshot) or a moved fingerprint means the agent edited a + // pre-dirty file THIS session — let it flow through the normal classification (HI-03). + if (preDirty?.has(p)) { + const baseFp = preDirty.get(p); + if (baseFp != null && fingerprintFile(root, p) === baseFp) continue; + } out.add(p); } return [...out].filter((p) => !IGNORED_PREFIX(p)).sort(); @@ -131,7 +139,12 @@ export function changedSet(root, baseHead, { sinceMs, preDirty } = {}) { * a config-only change keeps that lighter bar. * @param {{stopHookActive?: boolean, isRepo?: boolean, markerExists?: boolean, * killSwitch?: boolean, changed?: string[], stateTouched?: boolean, - * verifyEvidence?: {fresh: boolean, status: string} | null}} [input] + * verifyEvidence?: {fresh: boolean, status: string, codeStateMatches?: boolean} | null, + * substantiveTests?: string[] | null}} [input] + * verifyEvidence.codeStateMatches — the stamp's stored code-state fingerprint still + * equals the code as it stands now (HI-02); a fresh PASS only counts when this is true. + * substantiveTests — the FS-filtered subset of changed test files that still exist and + * are non-empty (HI-04); null (pure-table callers) falls back to raw classification. */ export function gateDecision({ stopHookActive = false, @@ -141,6 +154,7 @@ export function gateDecision({ changed = [], stateTouched = false, verifyEvidence = null, + substantiveTests = null, } = {}) { if (stopHookActive) return { allow: true, row: "stop-hook-active" }; if (!isRepo) return { allow: true, row: "not-a-repo" }; @@ -150,9 +164,19 @@ export function gateDecision({ for (const f of changed) classes[classifyPath(f)].push(f); const external = changed.length - classes.internal.length; if (!external && !stateTouched) return { allow: true, row: "no-changes", classes }; - const testEvidence = - classes.test.length > 0 || - (verifyEvidence?.fresh === true && verifyEvidence?.status === "PASS"); + // Test evidence has two legs. STRONG (HI-02): a fresh `verify` PASS whose stored code + // state still matches the tree NOW — proof the tests ran against the FINAL code, not a + // since-mutated one. WEAKER (HI-04): a substantive test file moved with the change + // (added/modified and non-empty — a deleted or emptied test is an obligation signal, + // never proof). `substantiveTests` is the caller's FS-filtered set; absent it (pure + // callers) we trust the raw classification. + const strongVerify = + verifyEvidence?.fresh === true && + verifyEvidence?.status === "PASS" && + verifyEvidence?.codeStateMatches === true; + const hasTestFile = + substantiveTests == null ? classes.test.length > 0 : substantiveTests.length > 0; + const testEvidence = strongVerify || hasTestFile; const docEvidence = classes.docs.length > 0 || stateTouched; if (classes.code.length) { if (!testEvidence) return { allow: false, row: "code-without-test-evidence", classes }; @@ -224,9 +248,9 @@ export function repairReason( let headline; const steps = []; if (row === "code-without-test-evidence") { - headline = `END-TO-END COMPLETENESS: code changed this session with NO test evidence — no test file moved with it and no fresh passing \`${BRAND.cli} verify\` run backs the change.`; + headline = `END-TO-END COMPLETENESS: code changed this session with NO test evidence — no substantive test file (added or modified, non-empty; a deleted or empty test does not count) moved with it, and no fresh passing \`${BRAND.cli} verify\` run bound to the CURRENT code state backs the change (a verify that ran BEFORE your last edit is stale — re-run it after the final change).`; steps.push( - `\`${BRAND.cli} verify\` — run the project's own tests against this change (or add/adjust a test that exercises the new behaviour).`, + `\`${BRAND.cli} verify\` — run the project's own tests against this change AFTER your final edit (a verify from before the last change no longer matches the code), or add/adjust a real test that exercises the new behaviour.`, docsSyncStep, handoffStep(), decideStep, @@ -309,14 +333,49 @@ export function stopGate(root, sid, hook = {}) { try { const provPath = join(root, ".forge", "provenance.json"); const mtime = statSync(provPath).mtimeMs; - const status = JSON.parse(readFileSync(provPath, "utf8"))?.tests?.status; - if (typeof status === "string") + const prov = JSON.parse(readFileSync(provPath, "utf8")); + const status = prov?.tests?.status; + if (typeof status === "string") { + // HI-02: the PASS only counts if the code has NOT moved since verification — the + // stamp's stored codeState.dirtyHash must still equal the code state recomputed + // NOW. Any doubt (git unavailable, null hash on either side, or a throw) → the + // stamp is NON-authoritative and does not count (fail toward a test-file change). + let codeStateMatches = false; + try { + const stored = prov?.codeState; + const now = computeCodeState(root); + codeStateMatches = + !!stored && + stored.gitAvailable === true && + now.gitAvailable === true && + typeof stored.dirtyHash === "string" && + typeof now.dirtyHash === "string" && + stored.dirtyHash === now.dirtyHash; + } catch {} verifyEvidence = { fresh: startedAt != null && mtime > startedAt, status, + codeStateMatches, }; + } } catch {} - const decision = gateDecision({ changed, stateTouched, verifyEvidence }); + // HI-04: a changed test file is an OBLIGATION signal, not proof. Only test files that + // still EXIST and are NON-EMPTY (added or modified, not deleted/emptied) may count + // toward the weaker evidence leg; the strong leg is the code-state-bound verify PASS. + const substantiveTests = changed.filter((p) => { + if (classifyPath(p) !== "test") return false; + try { + return statSync(join(root, p)).size > 0; + } catch { + return false; + } + }); + const decision = gateDecision({ + changed, + stateTouched, + verifyEvidence, + substantiveTests, + }); if (decision.allow) return decision; // Marker FIRST: if it can't be persisted, the block-once promise can't be kept — // on a read-only checkout that would mean an unsatisfiable block every turn, so diff --git a/src/init.js b/src/init.js index 0636cc6..c921c02 100644 --- a/src/init.js +++ b/src/init.js @@ -39,6 +39,28 @@ export function ensureLedgerGitattributes(targetRoot = process.cwd()) { // --------------------------------------------------------------------------- const FORGE_SETTINGS_MARKER = "forge-managed"; +// Sibling of the `_forge` marker: the ownership manifest (HI-05). It records the EXACT +// entries this (and any earlier) install ADDED — permission strings genuinely absent +// before, hook guard-identities Forge inserted, and whether Forge set the statusLine / +// $schema — so uninstall reverses only Forge's own additions and never a user-owned entry +// that happened to match the template. Absent on pre-HI-05 installs → template-match fallback. +const FORGE_OWNED_KEY = "_forgeOwned"; + +// The installed-assets root every Forge hook/statusline command resolves under. Used to +// tell a Forge-owned command (some spelling of a path under here, or the plugin root) apart +// from a user's OWN hook that merely shares a basename+args — those must never collide for +// ownership/removal purposes (HI-05). +const FORGE_GLOBAL = join(BRAND.root, "global"); + +/** True iff `command` is a Forge-managed hook/statusline command — i.e. it resolves to a + * path under the installed assets root (`~/.forge/…`, the resolved `/global/…`, quoted + * or not) or the plugin root (`${CLAUDE_PLUGIN_ROOT}/global/…`). A user's same-basename hook + * at a different absolute path is NOT Forge-owned, so it is never blocked on merge or removed + * on uninstall. Path-aware on purpose: `guardKey` (basename+args) is for dedup only. */ +function isForgeCommand(command) { + const c = normalizeCommand(command); + return c.includes(`${FORGE_GLOBAL}/`) || c.includes("${CLAUDE_PLUGIN_ROOT}/global/"); +} /** Single-quote a path for safe embedding in a shell command (RA-12): an install * prefix containing spaces (or any shell metacharacter) must not split into words. @@ -128,9 +150,16 @@ export function guardKey(command) { /** Merge Forge hook entries into existing hook arrays, matching by guard identity to avoid * duplicates. An existing entry that is the SAME resolved command as the template's — just - * spelled without quotes (a pre-RA-12 install) — is upgraded in place to the quoted form. */ + * spelled without quotes (a pre-RA-12 install) — is upgraded in place to the quoted form. + * A template hook counts as "already installed" only when an existing FORGE-owned entry (some + * path spelling of it) shares its guard key: a user's same-basename hook at a DIFFERENT path + * must NOT block Forge's own install (HI-05). Returns the merged tree plus the guard identities + * actually added this merge (`added: [{event, key, command}]`) so the ownership manifest can + * reverse exactly them later. */ function mergeHooks(existing = {}, template = {}) { const merged = { ...existing }; + /** @type {{event:string, key:string, command:string}[]} */ + const added = []; for (const [event, entries] of Object.entries(template)) { const existingEntries = merged[event] || []; // guardKey → template command, to heal old unquoted spellings of the same command. @@ -149,22 +178,78 @@ function mergeHooks(existing = {}, template = {}) { } } } - const existingKeys = new Set( + const presentForgeKeys = new Set( existingEntries .flatMap((e) => (e.hooks || []).map((h) => h.command)) - .filter(Boolean) + .filter((c) => typeof c === "string" && isForgeCommand(c)) .map(guardKey), ); const newEntries = []; for (const entry of entries) { - const hooks = (entry.hooks || []).filter((h) => !existingKeys.has(guardKey(h.command))); + const hooks = (entry.hooks || []).filter((h) => !presentForgeKeys.has(guardKey(h.command))); + for (const h of hooks) added.push({ event, key: guardKey(h.command), command: h.command }); if (hooks.length) { newEntries.push({ ...entry, hooks }); } } merged[event] = [...existingEntries, ...newEntries]; } - return merged; + return { merged, added }; +} + +/** Best-effort reconstruction of Forge's footprint from a settings file that predates the + * ownership manifest but still carries the `_forge` marker (i.e. Forge installed into it with + * older code). Used to SEED the manifest on the first re-merge with new code, so a later + * uninstall still removes exactly what old Forge added rather than leaving orphaned hooks. + * Template-match based, hardened with `isForgeCommand` for hooks so a user's different-path + * hook is never adopted. Conservative: only entries that both match the template AND (for + * hooks) resolve to a Forge path are claimed. */ +function legacyOwnedScan(settings, template) { + const owned = { + permissions: { allow: [], ask: [], deny: [] }, + /** @type {{event:string, key:string, command:string}[]} */ + hooks: [], + statusLine: false, + schema: false, + }; + if (settings.permissions && template.permissions) { + for (const level of ["allow", "ask", "deny"]) { + const tpl = template.permissions[level]; + const cur = settings.permissions[level]; + if (!Array.isArray(tpl) || !Array.isArray(cur)) continue; + const tplSet = new Set(tpl); + for (const s of cur) if (tplSet.has(s)) owned.permissions[level].push(s); + } + } + const templateKeys = new Set(); + for (const entries of Object.values(template.hooks || {})) + for (const entry of entries) + for (const h of entry.hooks || []) if (h.command) templateKeys.add(guardKey(h.command)); + if (settings.hooks && typeof settings.hooks === "object") { + for (const [event, entries] of Object.entries(settings.hooks)) { + if (!Array.isArray(entries)) continue; + for (const entry of entries) + for (const h of entry?.hooks || []) + if ( + typeof h?.command === "string" && + isForgeCommand(h.command) && + templateKeys.has(guardKey(h.command)) + ) + owned.hooks.push({ + event, + key: guardKey(h.command), + command: h.command, + }); + } + } + if ( + settings.statusLine?.command && + template.statusLine?.command && + normalizeCommand(settings.statusLine.command) === normalizeCommand(template.statusLine.command) + ) + owned.statusLine = true; + if (settings.$schema && settings.$schema === template.$schema) owned.schema = true; + return owned; } /** @@ -191,11 +276,36 @@ export function mergeSettings({ settingsPath, noSettings } = {}) { const before = JSON.stringify(existing); const report = { added: [], unchanged: [], path: target }; + // Ownership manifest (HI-05): accumulate across re-merges so a re-install never forgets what + // an earlier install added. Seed from the prior manifest if present; else, if the legacy + // `_forge` marker is present (old install, no manifest), reconstruct old Forge's footprint by + // template-match so a later uninstall still reverses it precisely. + const prior = + existing[FORGE_OWNED_KEY] && + typeof existing[FORGE_OWNED_KEY] === "object" && + existing[FORGE_OWNED_KEY].added + ? existing[FORGE_OWNED_KEY].added + : existing._forge === FORGE_SETTINGS_MARKER + ? legacyOwnedScan(existing, template) + : null; + const ownedPerms = { + allow: [...(prior?.permissions?.allow || [])], + ask: [...(prior?.permissions?.ask || [])], + deny: [...(prior?.permissions?.deny || [])], + }; + const ownedHooks = [...(prior?.hooks || [])]; + let ownedStatusLine = Boolean(prior?.statusLine); + let ownedSchema = Boolean(prior?.schema); + // Hooks if (template.hooks) { - const before = JSON.stringify(existing.hooks || {}); - existing.hooks = mergeHooks(existing.hooks, template.hooks); - if (JSON.stringify(existing.hooks) !== before) report.added.push("hooks"); + const beforeHooks = JSON.stringify(existing.hooks || {}); + const { merged, added } = mergeHooks(existing.hooks, template.hooks); + existing.hooks = merged; + for (const a of added) + if (!ownedHooks.some((o) => o.event === a.event && o.command === a.command)) + ownedHooks.push(a); + if (JSON.stringify(existing.hooks) !== beforeHooks) report.added.push("hooks"); else report.unchanged.push("hooks"); } @@ -204,29 +314,51 @@ export function mergeSettings({ settingsPath, noSettings } = {}) { const ep = existing.permissions || {}; for (const level of ["allow", "ask", "deny"]) { if (template.permissions[level]) { - const before = (ep[level] || []).length; - ep[level] = unionStrings(ep[level], template.permissions[level]); - if (ep[level].length > before) report.added.push(`permissions.${level}`); - else report.unchanged.push(`permissions.${level}`); + const cur = ep[level] || []; + const curSet = new Set(cur); + const newlyAdded = template.permissions[level].filter((s) => !curSet.has(s)); + ep[level] = unionStrings(cur, template.permissions[level]); + if (newlyAdded.length) { + report.added.push(`permissions.${level}`); + for (const s of newlyAdded) if (!ownedPerms[level].includes(s)) ownedPerms[level].push(s); + } else report.unchanged.push(`permissions.${level}`); } } if (!ep.defaultMode) ep.defaultMode = template.permissions.defaultMode || "default"; existing.permissions = ep; } - // Statusline — set only if not already configured + // Statusline — set only if not already configured (a user's own statusLine is left alone + // AND not claimed, so uninstall never removes it). if (template.statusLine && !existing.statusLine) { existing.statusLine = template.statusLine; report.added.push("statusLine"); + ownedStatusLine = true; } else if (template.statusLine) { report.unchanged.push("statusLine"); } - // Schema - if (template.$schema && !existing.$schema) existing.$schema = template.$schema; + // Schema — set only when absent; record whether WE set it. + if (template.$schema && !existing.$schema) { + existing.$schema = template.$schema; + ownedSchema = true; + } - // Mark as forge-managed (metadata, won't affect Claude Code) + // Mark as forge-managed (metadata, won't affect Claude Code) + persist the ownership manifest. existing._forge = FORGE_SETTINGS_MARKER; + existing[FORGE_OWNED_KEY] = { + version: BRAND.version, + added: { + permissions: { + allow: ownedPerms.allow, + ask: ownedPerms.ask, + deny: ownedPerms.deny, + }, + hooks: ownedHooks, + statusLine: ownedStatusLine, + schema: ownedSchema, + }, + }; // Nothing to do — don't rewrite (or back up) an already-current file. if (status !== "missing" && JSON.stringify(existing) === before) { @@ -263,15 +395,17 @@ export function mergeSettings({ settingsPath, noSettings } = {}) { export { LEGACY_PROFILES, PROFILES, validateProfile } from "./repo_config.js"; /** - * Remove every Forge-managed entry that `mergeSettings` added from the user's - * ~/.claude/settings.json, using the settings template as the authoritative shape - * (RA-17): hook entries whose guardKey matches a template guard (quote-normalized), - * permission strings appearing verbatim in the template's allow/ask/deny (removed only - * from the same list they are in), the statusLine iff its command matches the template's - * resolved command (quote-normalized), the `$schema` iff it is the template's, and the - * `_forge` marker. Everything user-owned is preserved unchanged; empty containers left - * behind are pruned. Timestamped backup + atomic tmp-file+rename write, same as - * `mergeSettings`. Corrupt file → refuses; missing file → noop. + * Remove every entry Forge added from the user's ~/.claude/settings.json (RA-17, HI-05). + * Authoritative source is the `_forgeOwned` ownership manifest written by `mergeSettings`: + * only the permission strings, hook commands, statusLine, and `$schema` Forge genuinely + * ADDED are reversed — a user's own `Bash(git status:*)`, identical statusLine/`$schema`, + * or same-basename hook at a DIFFERENT path is preserved byte-for-byte because it was never + * recorded as Forge-added. When the manifest is absent (a pre-HI-05 install, marked only by + * `_forge`) it falls back to a conservative template match: permission strings verbatim, hook + * commands whose guardKey matches a template guard AND resolve to a Forge path (so a user's + * different-path hook stays), statusLine/`$schema` only if they equal the template's. Empty + * containers are pruned; the `_forge` marker + `_forgeOwned` manifest are removed. Timestamped + * backup + atomic tmp-file+rename write. Corrupt file → refuses; missing file → noop. * @param {{settingsPath?: string}} [opts] * @returns {{action:"removed", path:string, removed:string[], backup:string} * | {action:"noop", path:string, reason:string} @@ -295,16 +429,43 @@ export function removeForgeSettings({ settingsPath } = {}) { /** @type {string[]} */ const removed = []; - // Hooks: drop every hook whose guard identity is one the template installs. - const templateKeys = new Set(); - for (const entries of Object.values(template.hooks || {})) { - for (const entry of entries) { - for (const h of entry.hooks || []) if (h.command) templateKeys.add(guardKey(h.command)); - } + // What to reverse. With an ownership manifest (HI-05) it is EXACTLY what Forge added; without + // one (a pre-HI-05 install) fall back to the template shape, hardened so a user's different-path + // hook is never removed and preferring to leave an entry when unsure. + const manifest = + settings[FORGE_OWNED_KEY] && + typeof settings[FORGE_OWNED_KEY] === "object" && + settings[FORGE_OWNED_KEY].added + ? settings[FORGE_OWNED_KEY].added + : null; + const owned = manifest || legacyOwnedScan(settings, template); + const ownedPerms = { + allow: new Set(owned.permissions?.allow || []), + ask: new Set(owned.permissions?.ask || []), + deny: new Set(owned.permissions?.deny || []), + }; + /** @type {Map>} event → normalized commands Forge owns */ + const ownedHookCmds = new Map(); + for (const h of owned.hooks || []) { + if (!ownedHookCmds.has(h.event)) ownedHookCmds.set(h.event, new Set()); + ownedHookCmds.get(h.event).add(normalizeCommand(h.command)); } + // Drop the statusLine/$schema only if Forge set it AND it is still the template's value — + // if the user replaced it after install, it is theirs now (leave it). + const dropStatusLine = + Boolean(owned.statusLine) && + Boolean(settings.statusLine?.command) && + Boolean(template.statusLine?.command) && + normalizeCommand(settings.statusLine.command) === normalizeCommand(template.statusLine.command); + const dropSchema = + Boolean(owned.schema) && Boolean(settings.$schema) && settings.$schema === template.$schema; + + // Hooks: drop only the exact commands Forge added (matched quote-normalized within the event). if (settings.hooks && typeof settings.hooks === "object") { for (const [event, entries] of Object.entries(settings.hooks)) { if (!Array.isArray(entries)) continue; + const ownedCmds = ownedHookCmds.get(event); + if (!ownedCmds || !ownedCmds.size) continue; let changed = false; const kept = []; for (const entry of entries) { @@ -313,7 +474,7 @@ export function removeForgeSettings({ settingsPath } = {}) { continue; } const hooks = entry.hooks.filter( - (h) => !(typeof h?.command === "string" && templateKeys.has(guardKey(h.command))), + (h) => !(typeof h?.command === "string" && ownedCmds.has(normalizeCommand(h.command))), ); if (hooks.length !== entry.hooks.length) changed = true; if (hooks.length) kept.push({ ...entry, hooks }); @@ -327,14 +488,13 @@ export function removeForgeSettings({ settingsPath } = {}) { if (Object.keys(settings.hooks).length === 0) delete settings.hooks; } - // Permissions: remove template strings verbatim, only from the SAME list they sit in. - if (settings.permissions && template.permissions) { + // Permissions: remove only the exact strings Forge added, from the SAME list they sit in. + if (settings.permissions) { for (const level of ["allow", "ask", "deny"]) { - const tpl = template.permissions[level]; + const drop = ownedPerms[level]; const cur = settings.permissions[level]; - if (!Array.isArray(tpl) || !Array.isArray(cur)) continue; - const tplSet = new Set(tpl); - const kept = cur.filter((s) => !tplSet.has(s)); + if (!drop.size || !Array.isArray(cur)) continue; + const kept = cur.filter((s) => !drop.has(s)); if (kept.length !== cur.length) { removed.push(`permissions.${level}`); if (kept.length) settings.permissions[level] = kept; @@ -346,7 +506,7 @@ export function removeForgeSettings({ settingsPath } = {}) { if ( settings.permissions && Object.keys(settings.permissions).length === 1 && - settings.permissions.defaultMode === (template.permissions.defaultMode || "default") + settings.permissions.defaultMode === (template.permissions?.defaultMode || "default") ) { delete settings.permissions; } else if (settings.permissions && Object.keys(settings.permissions).length === 0) { @@ -354,19 +514,13 @@ export function removeForgeSettings({ settingsPath } = {}) { } } - // Statusline: only if it IS the template's (quote-normalized) — a user's own stays. - if ( - settings.statusLine?.command && - template.statusLine?.command && - normalizeCommand(settings.statusLine.command) === normalizeCommand(template.statusLine.command) - ) { + if (dropStatusLine) { delete settings.statusLine; removed.push("statusLine"); } + if (dropSchema) delete settings.$schema; - // Schema: mergeSettings sets it only when absent — remove only the template's own value. - if (settings.$schema && settings.$schema === template.$schema) delete settings.$schema; - + if (FORGE_OWNED_KEY in settings) delete settings[FORGE_OWNED_KEY]; if ("_forge" in settings) { delete settings._forge; removed.push("_forge"); @@ -432,6 +586,11 @@ export function init({ // `=== false` (not `!valid.ok`): tsc only narrows the discriminated union this way here. if (valid.ok === false) return { profile: { error: valid.error }, aborted: true }; const profileResult = writeProfile(targetRoot, profile); + // HI-09: the profile name is valid, but persistence can still FAIL at write time when + // `.forge/forge.config.json` is corrupt (writeForgeConfig refuses). Abort BEFORE any further + // side effect — no sync/AGENTS.md, no .gitattributes append, no settings merge — exactly like + // the invalid-name path, so a corrupt config never leaves a half-initialized repo. + if (profileResult?.error) return { profile: profileResult, aborted: true }; const r = sync({ targetRoot }); ensureLedgerGitattributes(targetRoot); const settings = mergeSettings({ noSettings, settingsPath }); diff --git a/src/session.js b/src/session.js index 030cedb..a1c87ed 100644 --- a/src/session.js +++ b/src/session.js @@ -4,6 +4,7 @@ // completion gate diffs against it; the rehydration block tells a fresh session what // recently happened instead of letting it assume. import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { existsSync, mkdirSync, @@ -52,17 +53,35 @@ function statusPathsZ(root) { } } +/** A content fingerprint (sha256 hex of the bytes) of a working-tree file, or null when + * it can't be read (missing/deleted/binary-unreadable). The session baseline stores this + * per pre-dirty path so the gate can tell an untouched pre-existing edit (fingerprint + * still matches → keep hiding it) from one the agent edited FURTHER this session + * (fingerprint moved → it's a real session change, HI-03). */ +export function fingerprintFile(root, rel) { + try { + return createHash("sha256") + .update(readFileSync(join(root, rel))) + .digest("hex"); + } catch { + return null; + } +} + /** Record HEAD as this session's baseline — once. An existing file wins (a --resume * re-fires SessionStart and must NOT move the anchor mid-session). Also snapshots the * files ALREADY dirty at session start (even on an unborn HEAD), so the completion - * gate never attributes pre-existing dirt to this session. */ + * gate never attributes pre-existing dirt to this session. Each pre-dirty path is stored + * WITH a content fingerprint (`\t`) so a further edit to an already-dirty + * file is still seen as a session change (HI-03). */ export function recordBaseline(root, sid) { const head = git(root, ["rev-parse", "HEAD"]); const dirtyPath = sessionPath(root, sid, "dirty"); try { if (git(root, ["rev-parse", "--is-inside-work-tree"]) === "true" && !existsSync(dirtyPath)) { mkdirSync(join(root, ".forge", "sessions"), { recursive: true }); - writeFileSync(dirtyPath, `${statusPathsZ(root).join("\n")}\n`); + const lines = statusPathsZ(root).map((p) => `${fingerprintFile(root, p) ?? ""}\t${p}`); + writeFileSync(dirtyPath, `${lines.join("\n")}\n`); } } catch {} if (!head) return { recorded: false, head: null }; // unborn HEAD → dirty snapshot only @@ -77,13 +96,27 @@ export function recordBaseline(root, sid) { } } -/** The set of paths that were already dirty when the session started (empty when the - * snapshot is missing — degraded mode, gate errs toward its other guards). */ +/** A Map of the files already dirty when the session started. + * `null` fingerprint = unknown (an OLD snapshot that stored paths only, or a file + * unreadable at snapshot time) → the gate must NOT silently hide it (degrades to + * "potentially changed"). `.has(path)` keeps working for callers that only need + * membership. Null when the snapshot is missing (degraded mode). */ export function readDirtySnapshot(root, sid) { try { const p = sessionPath(root, sid, "dirty"); if (!existsSync(p)) return null; - return new Set(readFileSync(p, "utf8").split("\n").filter(Boolean)); + const map = new Map(); + for (const line of readFileSync(p, "utf8").split("\n")) { + if (!line) continue; + const tab = line.indexOf("\t"); + // New format `\t`: split on the FIRST tab (a path may contain tabs). + if (tab > 0 && /^[0-9a-f]{64}$/.test(line.slice(0, tab))) + map.set(line.slice(tab + 1), line.slice(0, tab)); + else if (tab === 0) + map.set(line.slice(1), null); // empty fingerprint written → unknown + else map.set(line, null); // legacy path-only line → unknown fingerprint + } + return map; } catch { return null; } diff --git a/src/verify.js b/src/verify.js index 7e82a30..adee2f4 100644 --- a/src/verify.js +++ b/src/verify.js @@ -4,6 +4,7 @@ // (a cheap, zero-LLM hallucination signal). It emits a provenance stamp so a // reviewer reads WHAT was checked, not the authoring transcript. import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { build as buildAtlas, has, isStale, load as loadAtlas } from "./atlas.js"; @@ -29,6 +30,43 @@ function git(args, cwd) { } } +/** + * A content fingerprint of the FULL working-tree change relative to HEAD — the unstaged + * diff, the staged diff, and every untracked (non-ignored) file's bytes, sorted, sha256'd. + * Two checkouts with the same `dirtyHash` have byte-identical pending changes, so a + * `verify` stamp can be BOUND to the exact code state it validated (HI-02): at Stop the + * gate recomputes this and only trusts the PASS when the hash still matches. Never throws; + * `gitAvailable:false` / `dirtyHash:null` is the honest "cannot bind" signal (the gate then + * refuses to count the stamp). Pure w.r.t. the tree — reads git + files, writes nothing. + * @param {string} [cwd] + * @returns {{head: string|null, dirtyHash: string|null, gitAvailable: boolean}} + */ +export function computeCodeState(cwd = process.cwd()) { + try { + if (git(["rev-parse", "--is-inside-work-tree"], cwd).trim() !== "true") + return { head: null, dirtyHash: null, gitAvailable: false }; + const head = git(["rev-parse", "HEAD"], cwd).trim() || null; + // Exclude forge's OWN state dir: writing provenance.json / session files must never + // perturb the fingerprint the stamp is bound to (self-reference), and it's ignored in + // real repos anyway — this keeps the hash stable even if a user forgot to gitignore it. + const untracked = git(["ls-files", "--others", "--exclude-standard", "-z"], cwd) + .split("\0") + .filter((f) => f && !f.startsWith(".forge/")) + .sort(); + const h = createHash("sha256"); + h.update(git(["diff", "HEAD"], cwd)); + h.update(git(["diff", "--cached"], cwd)); + for (const f of untracked) { + try { + h.update(readFileSync(join(cwd, f))); + } catch {} + } + return { head, dirtyHash: h.digest("hex"), gitAvailable: true }; + } catch { + return { head: null, dirtyHash: null, gitAvailable: false }; + } +} + // Run the project's OWN tests, driven off the stack detector (never a benchmark). The verdict // is an honest four-state `status`: // PASS — a real verifier ran and passed @@ -41,6 +79,17 @@ function git(args, cwd) { // a known bin only, never npx (it can download arbitrary packages). // `ran`/`passed` are kept for back-compat (consensus.js reads them). Bounded by a timeout // (FORGE_VERIFY_TIMEOUT_MS, default 10 min) so a hanging test can't hang the gate. +/** + * One executed (or attempted) suite's per-suite detail (HI-01/ME-02). + * @typedef {object} SuiteResult + * @property {string} label human-readable runner command + * @property {"PASS"|"FAIL"|"INCOMPLETE"} status + * @property {number|null} [exitCode] process exit code (0 pass, non-zero fail, null if it never ran) + * @property {string} [code] spawn error code (ENOENT/EACCES/ENOEXEC/…) when it did not execute + * @property {string} [signal] terminating signal, if any + * @property {boolean} [timedOut] true when the suite was killed for exceeding the timeout + * @property {string} [output] tail of the suite's own output (failures) + */ /** * @typedef {object} VerifyTests * @property {boolean} ran @@ -49,6 +98,8 @@ function git(args, cwd) { * @property {string} [runner] * @property {boolean} [timedOut] * @property {string[]} [detected] + * @property {SuiteResult[]} [executed] every suite forge actually spawned, with its per-suite verdict + * @property {string[]} [notExecuted] labels of detected suites forge has no built-in executor for * @property {string} [output] */ // Bins forge is willing to execute directly. Everything else stays report-only. @@ -65,14 +116,32 @@ function parseRunnerStrings(cmds) { return { label: cmd }; }); } +/** Is this descriptor one forge can execute directly (whitelisted bin, and a + * package.json present for the package-manager runners)? Pure. */ +function isExecutable(r, cwd) { + return !!( + r?.bin && + EXECUTORS.has(r.bin) && + (r.bin === "pytest" || existsSync(join(cwd, "package.json"))) + ); +} + /** + * Run EVERY detected executable suite (HI-01) — a polyglot repo where a passing + * Node suite hides a failing pytest suite must NOT report PASS. Aggregate to an + * honest four-state verdict: + * - all executed suites PASS and nothing was skipped → PASS + * - any executed suite FAILs (real non-zero exit) → FAIL + * - a detected suite is non-executable, or a spawn never completed (ENOENT / + * EACCES / ENOEXEC / signal / timeout, ME-02) → INCOMPLETE + * - no runners at all → NOT_CONFIGURED + * Only a real non-zero EXIT CODE from a suite that actually ran is a FAIL; a suite + * that never executed is INCOMPLETE, never a false FAIL. * @param {string} cwd * @returns {VerifyTests} */ function runTests(cwd) { const timeout = Number(process.env.FORGE_VERIFY_TIMEOUT_MS) || 600000; - const run = (cmd, args) => - execFileSync(cmd, args, { cwd, encoding: "utf8", stdio: "pipe", timeout }); // Detect the repo's real test runners (no test script → none → NOT_CONFIGURED, not a // forced npm-test failure). let stack = null; @@ -82,57 +151,105 @@ function runTests(cwd) { const detected = stack?.testCommands ?? []; if (!detected.length) return { ran: false, status: "NOT_CONFIGURED" }; const runners = stack?.testRunners?.length ? stack.testRunners : parseRunnerStrings(detected); - // First whitelisted descriptor wins. A package-manager runner is only real when a - // package.json exists (the guard the old npm-only path had). - const candidate = runners.find( - (r) => - r?.bin && - EXECUTORS.has(r.bin) && - (r.bin === "pytest" || existsSync(join(cwd, "package.json"))), - ); - if (!candidate) { - // Runners were detected but forge has no built-in executor for any of them — - // expected, didn't complete. Never guess a different runner. - const labels = runners.map((r) => r?.label).filter(Boolean); - return { - ran: false, - status: "INCOMPLETE", - detected, - output: `detected "${labels.join('", "')}" — no built-in executor; run it yourself and re-verify`, - }; - } - try { - run(candidate.bin, candidate.args ?? []); - return { ran: true, passed: true, status: "PASS", runner: candidate.label }; - } catch (e) { - if (e.code === "ENOENT") { - // The detected runner's binary isn't installed here — nothing ran, and silently - // substituting another package manager would verify the wrong thing. - return { - ran: false, - status: "INCOMPLETE", - detected, - output: `detected "${candidate.label}", executor unavailable (${candidate.bin} not on PATH)`, - }; + + /** @type {SuiteResult[]} */ + const executed = []; + /** @type {string[]} */ + const notExecuted = []; + for (const r of runners) { + const label = r?.label ?? String(r?.bin ?? "unknown"); + if (!isExecutable(r, cwd)) { + // No built-in executor (go/cargo/mvn/gradle/dotnet/rspec/phpunit/npx-runners) — + // report-only. Its absence means a PASS can't be claimed for the whole repo. + notExecuted.push(label); + continue; } - if (e.code === "ETIMEDOUT" || e.signal === "SIGTERM") { - return { - ran: true, - passed: false, - timedOut: true, - status: "INCOMPLETE", - runner: candidate.label, - output: `test run (${candidate.label}) exceeded ${timeout}ms`, - }; + try { + execFileSync(r.bin, r.args ?? [], { + cwd, + encoding: "utf8", + stdio: "pipe", + timeout, + }); + executed.push({ label, status: "PASS", exitCode: 0 }); + } catch (e) { + if (e.code === "ENOENT") { + // The detected runner's binary isn't installed here — nothing ran, and silently + // substituting another package manager would verify the wrong thing. + executed.push({ + label, + status: "INCOMPLETE", + exitCode: null, + code: "ENOENT", + output: `executor unavailable (${r.bin} not on PATH)`, + }); + } else if (e.code === "ETIMEDOUT" || e.signal === "SIGTERM") { + // Killed for running too long — it started but never reached a verdict. + executed.push({ + label, + status: "INCOMPLETE", + exitCode: null, + timedOut: true, + signal: e.signal, + output: `exceeded ${timeout}ms`, + }); + } else if (typeof e.status === "number") { + // A real, completed run that exited non-zero — the ONLY true FAIL. + executed.push({ + label, + status: "FAIL", + exitCode: e.status, + output: String(e.stdout || e.message || "").slice(-600), + }); + } else { + // EACCES / ENOEXEC / other spawn failure / signal termination: the suite did NOT + // execute, so this is INCOMPLETE, never FAIL (ME-02). + executed.push({ + label, + status: "INCOMPLETE", + exitCode: null, + code: e.code, + signal: e.signal, + output: `did not execute (${e.code || e.signal || "spawn error"})`, + }); + } } - return { - ran: true, - passed: false, - status: "FAIL", - runner: candidate.label, - output: String(e.stdout || e.message || "").slice(-600), - }; } + + // Aggregate. A PASS must mean every detected required suite ran and passed. + const anyFail = executed.some((s) => s.status === "FAIL"); + const anyIncomplete = executed.some((s) => s.status === "INCOMPLETE"); + const ranToVerdict = executed.some((s) => s.status === "PASS" || s.status === "FAIL"); + const timedOut = executed.some((s) => s.timedOut); + /** @type {"PASS"|"FAIL"|"INCOMPLETE"} */ + let status; + if (anyFail) status = "FAIL"; + else if (anyIncomplete || notExecuted.length) status = "INCOMPLETE"; + else status = "PASS"; // executed non-empty (NOT_CONFIGURED short-circuits above), all PASS + + // Honest human-readable summary, aggregated across suites. + const parts = []; + if (notExecuted.length) + parts.push( + `detected "${notExecuted.join('", "')}" — no built-in executor; run it yourself and re-verify`, + ); + for (const s of executed) { + if (s.status === "INCOMPLETE") parts.push(`"${s.label}" ${s.output ?? "did not execute"}`); + else if (s.status === "FAIL") parts.push(`"${s.label}" FAILED: ${s.output ?? ""}`); + } + const runnerLabels = executed.map((s) => s.label); + const runner = runnerLabels.join(", ") || runners.map((r) => r?.label).filter(Boolean)[0]; + return { + ran: ranToVerdict, + passed: status === "PASS", + status, + runner, + ...(timedOut ? { timedOut: true } : {}), + detected, + executed, + notExecuted, + ...(parts.length ? { output: parts.join("; ") } : {}), + }; } /** @@ -217,6 +334,9 @@ export function verify({ targetRoot = process.cwd(), base = "HEAD" } = {}) { changedFiles, untracked, tests, + // Bind the stamp to the exact code it was produced against (HI-02/ME-04): the Stop gate + // recomputes this and only counts the PASS as test-evidence when the hash still matches. + codeState: computeCodeState(targetRoot), symbolsChecked: symbols.length, unknownSymbols: unknown, }; diff --git a/test/cli_init.test.js b/test/cli_init.test.js index a94c3b9..8c6266a 100644 --- a/test/cli_init.test.js +++ b/test/cli_init.test.js @@ -113,3 +113,23 @@ test("install.sh propagates a failed settings merge as an INCOMPLETE install, an "merge exit status is no longer discarded by an || fallback on the pipeline", ); }); + +test("install.sh (HI-10): link() backs up foreign symlinks and uninstall stops when settings cleanup fails", () => { + const syntax = spawnSync("bash", ["-n", INSTALL_SH], { encoding: "utf8" }); + assert.equal(syntax.status, 0, `bash -n failed: ${syntax.stderr}`); + const sh = readFileSync(INSTALL_SH, "utf8"); + // link(): an existing symlink NOT pointing into the repo is backed up before replacement. + assert.match(sh, /backed up existing symlink/, "a foreign user symlink is backed up, not lost"); + assert.match(sh, /readlink "\$dest"/, "link() inspects the existing symlink target"); + // uninstall: a failed settings cleanup stops instead of removing assets the hooks point at. + assert.match( + sh, + /Uninstall INCOMPLETE — settings still reference Forge hooks/, + "uninstall refuses to strip assets while settings still reference them", + ); + // The INCOMPLETE branch exits non-zero and precedes asset removal. + const incompleteIdx = sh.indexOf("Uninstall INCOMPLETE"); + const unlinkIdx = sh.indexOf("unlink_ours", incompleteIdx); + assert.ok(incompleteIdx > 0 && unlinkIdx > incompleteIdx, "the stop precedes asset removal"); + assert.match(sh.slice(incompleteIdx, unlinkIdx), /exit 1/, "the incomplete path exits non-zero"); +}); diff --git a/test/consensus.test.js b/test/consensus.test.js index 5abf792..38784a9 100644 --- a/test/consensus.test.js +++ b/test/consensus.test.js @@ -288,6 +288,28 @@ test("verifyDeep: clean diff passes, persists provenance.deep + one verify metri } }); +test("verifyDeep: a NOT_CONFIGURED core is NOT recorded as an outcome 'pass' (ME-01)", () => { + const dir = mkdtempSync(join(tmpdir(), "forge-consensus-")); + try { + const r = verifyDeep({ + targetRoot: dir, + llm: false, + verifyImpl: () => fakeCore({ tests: { ran: false, status: "NOT_CONFIGURED" } }), + }); + assert.equal(r.status, "NOT_CONFIGURED"); + assert.equal(r.ok, false, "an unverified core is never ok"); + const metrics = readFileSync(join(dir, ".forge", "metrics.jsonl"), "utf8") + .trim() + .split("\n") + .map((l) => JSON.parse(l)); + assert.notEqual(metrics[0].outcome, "pass", "unverified must not count as a pass"); + assert.equal(metrics[0].outcome, "not_configured"); + assert.equal(metrics[0].status, "NOT_CONFIGURED"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("verifyDeep: a failing test suite blocks solo; findings + block land in provenance", () => { const dir = mkdtempSync(join(tmpdir(), "forge-consensus-")); try { diff --git a/test/gate.test.js b/test/gate.test.js index 6901330..95eaf6d 100644 --- a/test/gate.test.js +++ b/test/gate.test.js @@ -100,34 +100,67 @@ test("gate table: code + test evidence + docs (or state) → allow code-with-evi assert.equal(handoff.row, "code-with-evidence"); }); -test("gate table: a fresh passing verify run is test evidence; stale or FAIL is not", () => { +test("gate table: a fresh code-state-matching verify PASS is test evidence; stale/FAIL/moved-code is not", () => { const base = { changed: ["src/x.js", "README.md"] }; const fresh = gateDecision({ ...base, - verifyEvidence: { fresh: true, status: "PASS" }, + verifyEvidence: { fresh: true, status: "PASS", codeStateMatches: true }, }); assert.equal(fresh.allow, true); assert.equal(fresh.row, "code-with-evidence"); assert.equal( - gateDecision({ ...base, verifyEvidence: { fresh: false, status: "PASS" } }).row, + gateDecision({ + ...base, + verifyEvidence: { fresh: false, status: "PASS", codeStateMatches: true }, + }).row, "code-without-test-evidence", "a stale provenance stamp proves nothing about THIS session's change", ); assert.equal( - gateDecision({ ...base, verifyEvidence: { fresh: true, status: "FAIL" } }).row, + gateDecision({ + ...base, + verifyEvidence: { fresh: true, status: "FAIL", codeStateMatches: true }, + }).row, "code-without-test-evidence", "a fresh FAIL is not evidence of completion", ); + assert.equal( + gateDecision({ + ...base, + verifyEvidence: { fresh: true, status: "PASS", codeStateMatches: false }, + }).row, + "code-without-test-evidence", + "HI-02: a PASS whose code state moved since verification is stale and does not count", + ); assert.equal( gateDecision({ changed: ["src/x.js"], - verifyEvidence: { fresh: true, status: "PASS" }, + verifyEvidence: { fresh: true, status: "PASS", codeStateMatches: true }, }).row, "code-without-docs", "verify evidence covers the test leg only — docs are still owed", ); }); +test("gate table: HI-04 — a changed test file is not proof unless substantive", () => { + const changed = ["src/x.js", "test/x.test.js", "README.md"]; + assert.equal( + gateDecision({ changed, substantiveTests: [] }).row, + "code-without-test-evidence", + "a deleted/emptied test file does not satisfy the code-change test leg", + ); + assert.equal( + gateDecision({ changed, substantiveTests: ["test/x.test.js"] }).allow, + true, + "a real added/modified test file still counts as the weaker evidence leg", + ); + assert.equal( + gateDecision({ changed }).allow, + true, + "back-compat: no FS filter supplied → raw classification still decides", + ); +}); + test("gate table: test-only sessions pass (a regression test owes no prose)", () => { const r = gateDecision({ changed: ["test/gate.test.js"] }); assert.equal(r.allow, true); diff --git a/test/guards.test.js b/test/guards.test.js index 46de847..b35855a 100644 --- a/test/guards.test.js +++ b/test/guards.test.js @@ -131,6 +131,87 @@ test("protect-paths does not false-positive on benign git commands (RA-05)", () } }); +test("protect-paths blocks shell WRITES to a protected path (HI-06)", () => { + for (const command of [ + "echo X > .env", + "printf X >> .env", + "tee .env", + "tee -a .env", + "sed -i s/a/b/ .env", + "cp payload .env", + "mv payload .env", + "install payload .env", + "dd if=x of=.env", + ": > .env", + "> .env", + 'echo X > ".env"', + "cp key.pem /x/id_rsa", + ]) { + const r = runGuard("protect-paths.sh", { + tool_name: "Bash", + tool_input: { command }, + }); + assert.equal(r.code, 2, `must block: ${command}`); + assert.match( + r.err, + /writing to a protected secret path|in-place/, + `deny reason for: ${command}`, + ); + } +}); + +test("protect-paths does not false-positive on benign shell writes (HI-06)", () => { + for (const command of [ + "echo hi > out.txt", + "printf hi >> notes.md", + "tee build.log", + "cp src/a.js src/b.js", + "sed -i s/a/b/ src/x.js", + ]) { + const r = runGuard("protect-paths.sh", { + tool_name: "Bash", + tool_input: { command }, + }); + assert.equal(r.code, 0, `must not block: ${command}`); + } +}); + +test("protect-paths blocks git readers behind wrappers / global options (HI-07)", () => { + for (const command of [ + "git -C . diff -- .env", + "git --no-pager diff -- .env", + "/usr/bin/git diff -- .env", + "command git diff -- .env", + "env git diff -- .env", + "VAR=x git diff -- .env", + "git blame -- .env", + "git show-index .env", + "git -c core.pager=cat show HEAD:.env", + ]) { + const r = runGuard("protect-paths.sh", { + tool_name: "Bash", + tool_input: { command }, + }); + assert.equal(r.code, 2, `must block: ${command}`); + assert.match(r.err, /protected secret path/, `deny reason for: ${command}`); + } +}); + +test("protect-paths does not false-positive on benign git after HI-07 hardening", () => { + for (const command of [ + "git diff -- src/x.js", + "git --no-pager log --oneline", + "digit --version", + 'git commit -m "block cat .env and git show HEAD:.env reads"', + ]) { + const r = runGuard("protect-paths.sh", { + tool_name: "Bash", + tool_input: { command }, + }); + assert.equal(r.code, 0, `must not block: ${command}`); + } +}); + test("secret-redact redacts a token without jq (Node path)", () => { const r = runGuard("secret-redact.sh", { tool_name: "Bash", diff --git a/test/init.test.js b/test/init.test.js index 166f12b..508b6a0 100644 --- a/test/init.test.js +++ b/test/init.test.js @@ -396,3 +396,120 @@ test("removeForgeSettings refuses a corrupt file and noops on a missing or unman assert.equal(noop.action, "noop", "nothing forge-managed → noop"); assert.deepEqual(JSON.parse(readFileSync(clean, "utf8")), { model: "opus" }); }); + +// --------------------------------------------------------------------------- +// HI-05 — ownership manifest: uninstall reverses ONLY what Forge added +// --------------------------------------------------------------------------- + +const SETTINGS_SCHEMA = "https://json.schemastore.org/claude-code-settings.json"; + +test("HI-05 ownership round-trip: user-owned entries that collide with the template survive uninstall byte-identical", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-own-")); + const settingsPath = join(tmp, "settings.json"); + const base = join(BRAND.root, "global"); + // A statusLine byte-identical to the template's resolved command, a $schema identical to the + // template's, an allow string the template also ships, and a cortex.sh hook at a DIFFERENT + // absolute path than Forge's (same basename+args). None of these were added by Forge. + const userStatusLine = { + type: "command", + command: `bash ${shellQuote(join(base, "statusline.sh"))}`, + }; + const userCortex = { + type: "command", + command: "bash /home/user/custom/cortex.sh prompt", + }; + const original = { + $schema: SETTINGS_SCHEMA, + permissions: { allow: ["Bash(git status:*)"], defaultMode: "default" }, + statusLine: userStatusLine, + hooks: { UserPromptSubmit: [{ hooks: [userCortex] }] }, + }; + writeFileSync(settingsPath, JSON.stringify(original, null, 2)); + + const merged = mergeSettings({ settingsPath }); + assert.equal(merged.action, "merged"); + // Delta manifest: an already-present permission is NOT recorded as Forge-added. + const afterMerge = JSON.parse(readFileSync(settingsPath, "utf8")); + assert.ok(afterMerge._forgeOwned, "ownership manifest written"); + assert.ok( + !afterMerge._forgeOwned.added.permissions.allow.includes("Bash(git status:*)"), + "a permission the user already had is never claimed by Forge", + ); + assert.equal(afterMerge._forgeOwned.added.statusLine, false, "Forge did not set the statusLine"); + assert.equal(afterMerge._forgeOwned.added.schema, false, "Forge did not set $schema"); + // Forge DID add its own cortex.sh prompt hook even though the user has a same-name one. + const promptCmds = afterMerge.hooks.UserPromptSubmit.flatMap((e) => + (e.hooks || []).map((h) => h.command), + ).filter((c) => c.includes("cortex.sh") && c.endsWith("prompt")); + assert.equal(promptCmds.length, 2, "user's and Forge's cortex hooks coexist (different paths)"); + + const r = removeForgeSettings({ settingsPath }); + assert.equal(r.action, "removed"); + const after = JSON.parse(readFileSync(settingsPath, "utf8")); + assert.deepEqual(after, original, "every user-owned entry is restored byte-identical"); + // And Forge's own additions are gone. + assert.ok(!after.permissions.allow.includes("Bash(git diff:*)"), "Forge's permission removed"); +}); + +test("HI-05 delta manifest: a permission the user already had survives uninstall", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-delta-")); + const settingsPath = join(tmp, "settings.json"); + writeFileSync( + settingsPath, + JSON.stringify({ + permissions: { allow: ["Bash(git status:*)", "Bash(ls:*)"] }, + }), + ); + mergeSettings({ settingsPath }); + const manifest = JSON.parse(readFileSync(settingsPath, "utf8"))._forgeOwned.added; + assert.ok(!manifest.permissions.allow.includes("Bash(git status:*)"), "pre-owned not recorded"); + assert.ok(!manifest.permissions.allow.includes("Bash(ls:*)"), "pre-owned not recorded"); + assert.ok(manifest.permissions.allow.includes("Bash(git diff:*)"), "genuinely-added recorded"); + removeForgeSettings({ settingsPath }); + const after = JSON.parse(readFileSync(settingsPath, "utf8")); + assert.deepEqual( + after.permissions.allow.sort(), + ["Bash(git status:*)", "Bash(ls:*)"].sort(), + "the user's own permissions survive; only Forge's additions are removed", + ); +}); + +test("HI-05 re-merge is idempotent: the manifest and entries are stable, no duplicates", () => { + const tmp = mkdtempSync(join(tmpdir(), "forge-idem-")); + const settingsPath = join(tmp, "settings.json"); + mergeSettings({ settingsPath }); + const first = JSON.parse(readFileSync(settingsPath, "utf8")); + const second = mergeSettings({ settingsPath }); + assert.equal(second.action, "unchanged", "re-merge is a no-op"); + const afterSecond = JSON.parse(readFileSync(settingsPath, "utf8")); + assert.deepEqual(afterSecond._forgeOwned, first._forgeOwned, "manifest is byte-stable"); + const promptCmds = afterSecond.hooks.UserPromptSubmit.flatMap((e) => + (e.hooks || []).map((h) => h.command), + ); + assert.equal( + promptCmds.filter((c) => c.includes("cortex.sh") && c.endsWith("prompt")).length, + 1, + "no duplicate hook entries on re-merge", + ); +}); + +// --------------------------------------------------------------------------- +// HI-09 — profile persistence failure aborts init before any side effect +// --------------------------------------------------------------------------- + +test("HI-09 init aborts on a corrupt forge.config.json — zero side effects (no emit/gitattributes/settings)", () => { + const root = mkdtempSync(join(tmpdir(), "forge-hi09-")); + mkdirSync(join(root, ".forge"), { recursive: true }); + const cfg = join(root, ".forge/forge.config.json"); + const original = "{ corrupt"; + writeFileSync(cfg, original); + const settingsPath = join(root, "home-settings.json"); + const r = init({ targetRoot: root, profile: "minimal", settingsPath }); + assert.equal(r.aborted, true, "init reports the abort"); + assert.match(r.profile.error, /not valid JSON/); + assert.equal(readFileSync(cfg, "utf8"), original, "corrupt config bytes preserved"); + assert.ok(!existsSync(join(root, "AGENTS.md")), "no AGENTS.md emitted"); + assert.ok(!existsSync(join(root, "CLAUDE.md")), "no CLAUDE.md emitted"); + assert.ok(!existsSync(join(root, ".gitattributes")), "no .gitattributes appended"); + assert.ok(!existsSync(settingsPath), "settings never touched"); +}); diff --git a/test/settings_template.test.js b/test/settings_template.test.js index 2d2ebe8..d0a76de 100644 --- a/test/settings_template.test.js +++ b/test/settings_template.test.js @@ -58,3 +58,68 @@ test("the former inert curl-pipe deny rules are absent", () => { assert.ok(!deny.includes("Bash(curl:* | sh)"), "inert curl|sh rule must stay removed"); assert.ok(!deny.includes("Bash(curl:* | bash)"), "inert curl|bash rule must stay removed"); }); + +// --------------------------------------------------------------------------- +// HI-08 — hook matchers cover current write/output-capable tools, and the two +// manifests (settings.template.json + hooks/hooks.json) stay consistent. +// --------------------------------------------------------------------------- + +const pluginHooks = JSON.parse( + readFileSync(new URL("../hooks/hooks.json", import.meta.url), "utf8"), +); + +/** Matcher of the first hook group under `event` whose command mentions `guard`. */ +function matcherFor(manifest, event, guard) { + const groups = manifest.hooks?.[event] ?? []; + for (const group of groups) { + if ((group.hooks ?? []).some((h) => (h.command ?? "").includes(guard))) { + return group.matcher ?? ""; + } + } + return null; +} + +test("PreToolUse protect-paths matcher covers NotebookEdit (HI-08)", () => { + for (const [name, manifest] of [ + ["settings.template.json", template], + ["hooks.json", pluginHooks], + ]) { + const matcher = matcherFor(manifest, "PreToolUse", "protect-paths.sh"); + assert.ok(matcher, `${name}: protect-paths PreToolUse group present`); + for (const tool of ["Bash", "Edit", "Write", "MultiEdit", "NotebookEdit"]) { + assert.ok( + matcher.split("|").includes(tool), + `${name}: protect-paths matcher must include ${tool} (got: ${matcher})`, + ); + } + } +}); + +test("PostToolUse secret-redact matcher covers WebFetch, MCP, NotebookEdit (HI-08)", () => { + for (const [name, manifest] of [ + ["settings.template.json", template], + ["hooks.json", pluginHooks], + ]) { + const matcher = matcherFor(manifest, "PostToolUse", "secret-redact.sh"); + assert.ok(matcher, `${name}: secret-redact PostToolUse group present`); + for (const tool of ["Bash", "Read", "Grep", "WebFetch", "NotebookEdit", "mcp__.*"]) { + assert.ok( + matcher.split("|").includes(tool), + `${name}: secret-redact matcher must include ${tool} (got: ${matcher})`, + ); + } + } +}); + +test("protect-paths + secret-redact matchers agree across both manifests (HI-08)", () => { + for (const [event, guard] of [ + ["PreToolUse", "protect-paths.sh"], + ["PostToolUse", "secret-redact.sh"], + ]) { + assert.equal( + matcherFor(template, event, guard), + matcherFor(pluginHooks, event, guard), + `${guard} matcher must match between settings.template.json and hooks.json`, + ); + } +}); diff --git a/test/stop_gate.test.js b/test/stop_gate.test.js index 547b072..6f5a527 100644 --- a/test/stop_gate.test.js +++ b/test/stop_gate.test.js @@ -5,6 +5,7 @@ import { mkdirSync, mkdtempSync, readFileSync, + rmSync, utimesSync, writeFileSync, } from "node:fs"; @@ -12,6 +13,7 @@ import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { test } from "node:test"; import { fileURLToPath } from "node:url"; +import { computeCodeState } from "../src/verify.js"; const ENTRY = fileURLToPath(new URL("../src/cortex_hook_main.js", import.meta.url)); const GUARD = join( @@ -50,10 +52,15 @@ const stopGate = (root, sid, extra = {}, env = {}) => // A `verify` provenance stamp (the exact shape verify.js writes) with an explicit // mtime offset relative to now, so fresh-vs-stale is deterministic regardless of // filesystem timestamp granularity. Positive offsetMs = future = after session start. -function writeProvenance(root, status, { offsetMs = 5000 } = {}) { +// `codeState` is captured from the tree AS IT STANDS NOW (HI-02) — the caller writes +// this AFTER its edits, so it matches what the gate recomputes at Stop unless the code +// is changed again afterward. Pass codeState:false to omit it (a pre-HI-02 stamp). +function writeProvenance(root, status, { offsetMs = 5000, codeState = true } = {}) { mkdirSync(join(root, ".forge"), { recursive: true }); const p = join(root, ".forge", "provenance.json"); - writeFileSync(p, JSON.stringify({ tests: { status } })); + const stamp = { tests: { status } }; + if (codeState) stamp.codeState = computeCodeState(root); + writeFileSync(p, JSON.stringify(stamp)); const t = new Date(Date.now() + offsetMs); utimesSync(p, t, t); } @@ -226,6 +233,73 @@ test("pre-session dirt is never pinned on the session (review: false-block class assert.doesNotMatch(JSON.parse(blocked.stdout).reason, /a\.js/, "pre-existing dirt not cited"); }); +test("HI-03: a pre-dirty file edited AGAIN during the session is seen as a change", () => { + const { root } = gitFixture(); + writeFileSync(join(root, "a.js"), "export const one = 100;\n"); // dirty BEFORE the session + start(root, "hi3"); + // The agent further edits the already-dirty file — a path-only baseline would hide this. + writeFileSync(join(root, "a.js"), "export const one = 200;\n"); + const r = stopGate(root, "hi3"); + const out = JSON.parse(r.stdout); + assert.equal(out.decision, "block", "a further edit to a pre-dirty file is not hidden"); + assert.match(out.reason, /a\.js/, "the re-edited pre-dirty file is cited"); +}); + +test("HI-02: a verify PASS goes stale once code changes after it (dirtyHash mismatch → block)", () => { + const { root } = gitFixture(); + start(root, "hi2a"); + writeFileSync(join(root, "a.js"), "export const one = 21;\n"); + writeFileSync(join(root, ".forge", "state.md"), "# state\n"); // docs leg present + writeProvenance(root, "PASS"); // codeState captured with a.js = 21 + // ...then the agent edits code AGAIN — the stamp no longer describes the FINAL tree. + writeFileSync(join(root, "a.js"), "export const one = 22;\n"); + const r = stopGate(root, "hi2a"); + const out = JSON.parse(r.stdout); + assert.equal(out.decision, "block", "stale verify evidence (code moved after it) does not count"); + assert.match(out.reason, /test evidence/i); +}); + +test("HI-02: a verify PASS bound to the FINAL code state + handoff → allow", () => { + const { root } = gitFixture(); + start(root, "hi2b"); + writeFileSync(join(root, "a.js"), "export const one = 23;\n"); + writeFileSync(join(root, ".forge", "state.md"), "# state\n"); // docs leg + writeProvenance(root, "PASS"); // codeState matches the tree; nothing changes afterward + const r = stopGate(root, "hi2b"); + assert.equal(r.stdout.trim(), "", "a fresh verify matching the final code state passes"); +}); + +test("HI-04: a deleted test file is not positive evidence for a code change", () => { + const { root, git } = gitFixture(); + writeFileSync(join(root, "a.test.js"), "import './a.js';\n"); + git("add", "-A"); + git("-c", "commit.gpgsign=false", "commit", "-qm", "add test"); + start(root, "hi4a"); + writeFileSync(join(root, "a.js"), "export const one = 41;\n"); // code change + writeFileSync(join(root, "README.md"), "# app\n\ndocumented\n"); // docs leg + rmSync(join(root, "a.test.js")); // the only "test" signal is a DELETION + const r = stopGate(root, "hi4a"); + assert.equal( + JSON.parse(r.stdout).decision, + "block", + "a deleted test proves nothing about the code change", + ); +}); + +test("HI-04: an empty new test file is not positive evidence", () => { + const { root } = gitFixture(); + start(root, "hi4b"); + writeFileSync(join(root, "a.js"), "export const one = 42;\n"); + writeFileSync(join(root, "README.md"), "# app\n\ndocumented\n"); + writeFileSync(join(root, "a.test.js"), ""); // empty file — an obligation signal, not proof + const r = stopGate(root, "hi4b"); + assert.equal( + JSON.parse(r.stdout).decision, + "block", + "an empty test file does not satisfy the test leg", + ); +}); + test("a branch switch's old commits are not attributed to the session", () => { const { root, git } = gitFixture(); // A feature branch whose commit is an hour old (committer date aged explicitly). diff --git a/test/verify.test.js b/test/verify.test.js index 2e69f79..36a639f 100644 --- a/test/verify.test.js +++ b/test/verify.test.js @@ -1,10 +1,35 @@ import assert from "node:assert/strict"; import { execFileSync } from "node:child_process"; -import { mkdtempSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; -import { extractCalledSymbols, findUnknownSymbols, verify } from "../src/verify.js"; +import { + computeCodeState, + extractCalledSymbols, + findUnknownSymbols, + verify, +} from "../src/verify.js"; + +// A fake executable so a suite's exit code (and thus verify's verdict) is deterministic, +// regardless of what npm/pytest do on this machine. +const fakeBin = (dir, name, exitCode, { executable = true } = {}) => { + const p = join(dir, name); + writeFileSync(p, `#!/bin/sh\nexit ${exitCode}\n`); + if (executable) chmodSync(p, 0o755); + return p; +}; +// Run `verify` with a bin dir prepended to PATH (fake runners win; the real git behind +// verify still resolves from the rest of PATH). +const withBins = (binDir, fn) => { + const old = process.env.PATH; + process.env.PATH = `${binDir}:${old}`; + try { + return fn(); + } finally { + process.env.PATH = old; + } +}; const gitRepo = () => { const root = mkdtempSync(join(tmpdir(), "forge-verify-")); @@ -168,3 +193,136 @@ test("verify: an untracked source file appears in provenance (changedFiles + unt assert.ok(r.changedFiles.includes("brand_new.js"), "untracked file in changedFiles"); assert.ok(r.provenance.untracked.includes("brand_new.js"), "untracked file in provenance stamp"); }); + +// --------------------------------------------------------------------------- +// HI-01 — run EVERY detected executable suite; a passing suite must not hide a +// second, unexecuted/failing one. +// --------------------------------------------------------------------------- + +test("verify: polyglot — passing Node suite + non-executable go suite ⇒ INCOMPLETE, not PASS", () => { + const root = gitRepo(); + writeFileSync( + join(root, "package.json"), + JSON.stringify({ name: "t", scripts: { test: "true" } }), + ); + writeFileSync(join(root, "go.mod"), "module example.com/app\n\ngo 1.22\n"); + const bin = mkdtempSync(join(tmpdir(), "forge-bin-")); + fakeBin(bin, "npm", 0); // Node suite passes + const r = withBins(bin, () => verify({ targetRoot: root })); + assert.equal( + r.tests.status, + "INCOMPLETE", + "a non-executable suite means the repo isn't fully verified", + ); + assert.equal(r.ok, false, "INCOMPLETE is never ok:true"); + assert.ok( + r.tests.executed.some((s) => s.label.includes("npm") && s.status === "PASS"), + "the Node suite ran and passed", + ); + assert.ok( + r.tests.notExecuted.some((l) => l.includes("go test")), + "the go suite is recorded as not executed", + ); +}); + +test("verify: two executable suites both pass ⇒ PASS (every suite ran)", () => { + const root = gitRepo(); + writeFileSync( + join(root, "package.json"), + JSON.stringify({ name: "t", scripts: { test: "true" } }), + ); + writeFileSync(join(root, "requirements.txt"), "pytest\n"); + const bin = mkdtempSync(join(tmpdir(), "forge-bin-")); + fakeBin(bin, "npm", 0); + fakeBin(bin, "pytest", 0); + const r = withBins(bin, () => verify({ targetRoot: root })); + assert.equal(r.tests.status, "PASS"); + assert.equal(r.ok, true); + assert.equal(r.tests.executed.length, 2, "both suites ran"); + assert.ok(r.tests.executed.every((s) => s.status === "PASS")); + assert.deepEqual(r.tests.notExecuted, []); +}); + +test("verify: one of two executable suites fails ⇒ FAIL (failure is not hidden)", () => { + const root = gitRepo(); + writeFileSync( + join(root, "package.json"), + JSON.stringify({ name: "t", scripts: { test: "true" } }), + ); + writeFileSync(join(root, "requirements.txt"), "pytest\n"); + const bin = mkdtempSync(join(tmpdir(), "forge-bin-")); + fakeBin(bin, "npm", 0); // Node passes + fakeBin(bin, "pytest", 1); // pytest fails + const r = withBins(bin, () => verify({ targetRoot: root })); + assert.equal(r.tests.status, "FAIL"); + assert.equal(r.ok, false); + const failed = r.tests.executed.find((s) => s.label.includes("pytest")); + assert.equal(failed.status, "FAIL"); + assert.equal(failed.exitCode, 1, "the real non-zero exit code is recorded"); +}); + +// --------------------------------------------------------------------------- +// ME-02 — a suite that never executed (spawn failure) is INCOMPLETE, never a FAIL. +// --------------------------------------------------------------------------- + +test("verify: a suite killed by a signal ⇒ INCOMPLETE, not FAIL (ME-02)", () => { + const root = gitRepo(); + writeFileSync(join(root, "requirements.txt"), "pytest\n"); + const bin = mkdtempSync(join(tmpdir(), "forge-bin-")); + // Starts but is terminated by SIGKILL before reaching a real exit code — it did NOT + // complete, so this must be INCOMPLETE, never a false FAIL (a non-zero exit code). + const p = join(bin, "pytest"); + writeFileSync(p, "#!/bin/sh\nkill -9 $$\n"); + chmodSync(p, 0o755); + const r = withBins(bin, () => verify({ targetRoot: root })); + assert.equal(r.tests.status, "INCOMPLETE", "a signal-killed run is not a test failure"); + assert.equal(r.ok, false); + const s = r.tests.executed.find((x) => x.label.includes("pytest")); + assert.equal(s.status, "INCOMPLETE"); + assert.notEqual(s.status, "FAIL"); + assert.ok(s.signal || s.code, `spawn signal/code recorded (${s.signal ?? s.code})`); +}); + +// --------------------------------------------------------------------------- +// HI-02 / ME-04 — codeState fingerprint bound to the exact tree state. +// --------------------------------------------------------------------------- + +test("computeCodeState: stable for an unchanged tree; tracked and untracked edits change dirtyHash", () => { + const root = gitRepo(); + writeFileSync(join(root, "a.js"), "export const a = 1\n"); + execFileSync("git", ["add", "."], { cwd: root, stdio: "ignore" }); + execFileSync("git", ["commit", "-m", "init"], { cwd: root, stdio: "ignore" }); + + const s1 = computeCodeState(root); + assert.equal(s1.gitAvailable, true); + assert.equal(typeof s1.head, "string"); + assert.ok(s1.head.length >= 7, "HEAD sha captured"); + assert.equal(typeof s1.dirtyHash, "string"); + assert.equal(computeCodeState(root).dirtyHash, s1.dirtyHash, "unchanged tree → stable hash"); + + writeFileSync(join(root, "a.js"), "export const a = 2\n"); // tracked edit + const s2 = computeCodeState(root); + assert.notEqual(s2.dirtyHash, s1.dirtyHash, "a tracked edit changes dirtyHash"); + + writeFileSync(join(root, "b.js"), "export const b = 3\n"); // untracked file + const s3 = computeCodeState(root); + assert.notEqual(s3.dirtyHash, s2.dirtyHash, "an untracked file changes dirtyHash"); +}); + +test("computeCodeState: non-git dir → gitAvailable:false, dirtyHash null (ME-04)", () => { + const dir = mkdtempSync(join(tmpdir(), "forge-nogit-")); + const s = computeCodeState(dir); + assert.equal(s.gitAvailable, false); + assert.equal(s.dirtyHash, null); + assert.equal(s.head, null); +}); + +test("verify: provenance carries the codeState fingerprint (HI-02)", () => { + const root = gitRepo(); + writeFileSync(join(root, "a.js"), "export const a = 1\n"); + const r = verify({ targetRoot: root }); + assert.ok(r.provenance.codeState, "codeState present on provenance"); + assert.equal(r.provenance.codeState.gitAvailable, true); + assert.equal(typeof r.provenance.codeState.dirtyHash, "string"); + assert.ok("head" in r.provenance.codeState); +});