diff --git a/src/resources/extensions/sf/self-feedback.js b/src/resources/extensions/sf/self-feedback.js index 524d359fe..310c1e2f6 100644 --- a/src/resources/extensions/sf/self-feedback.js +++ b/src/resources/extensions/sf/self-feedback.js @@ -436,6 +436,59 @@ function isAcceptedEvidence(evidence) { return typeof kind === "string" && ACCEPTED_EVIDENCE_KINDS.has(kind); } +/** + * Outcomes-verification AC1 (sf-mp4rxkwn-jmp039): when an agent-fix + * resolution cites a commit_sha, verify the commit actually exists in the + * repo. Without this check, an agent can stamp any string as a commit and + * the writer-layer constraint will accept it. + * + * Returns one of: + * "verified" — git exists and the commit is in the repo + * "missing" — git exists, commit lookup failed (e.g. NONEXISTENT_SHA) + * "ungrokable" — git unavailable or basePath isn't a git repo; we + * cannot verify and must not punish (don't make every + * non-git working tree start failing resolutions) + * + * Caller policy: reject on "missing"; accept on "verified" and + * "ungrokable". The "ungrokable" carve-out is the only practical one — an + * SF self-feedback ledger living outside a git repo is a real (rare) case + * and the agent-fix-unverified kind exists for explicit "I edited but + * can't cite a verified commit" workflows. + */ +function verifyCommitExists(commitSha, basePath) { + if (typeof commitSha !== "string" || !commitSha.trim()) return "ungrokable"; + const sha = commitSha.trim(); + let execFileSync; + try { + const cp = require("node:child_process"); + execFileSync = cp.execFileSync; + } catch { + return "ungrokable"; + } + try { + execFileSync("git", ["rev-parse", "--verify", `${sha}^{commit}`], { + cwd: basePath, + stdio: ["ignore", "pipe", "pipe"], + timeout: 5000, + }); + return "verified"; + } catch (err) { + // rev-parse exits non-zero when the commit doesn't exist OR when + // the directory isn't a git repo. Distinguish via a second probe: + // `git rev-parse --git-dir` exits 0 when inside a repo. + try { + execFileSync("git", ["rev-parse", "--git-dir"], { + cwd: basePath, + stdio: ["ignore", "pipe", "pipe"], + timeout: 5000, + }); + return "missing"; // in a repo but the SHA is unknown + } catch { + return "ungrokable"; // not in a repo or git unavailable + } + } +} + /** * Mark an entry as resolved. Updates SQLite for forge-local feedback when * available, otherwise rewrites the legacy JSONL fallback in place. Entries are @@ -465,6 +518,22 @@ export function markResolved(entryId, resolution, basePath = process.cwd()) { // caller can recover by re-submitting with a valid evidence kind. return false; } + // Outcomes-verification AC1 (sf-mp4rxkwn-jmp039): when an agent-fix + // resolution cites a commit_sha, verify the commit actually exists in + // the repo. Reject when git can confirm the commit is missing; accept + // when git is unavailable or basePath isn't a git repo (the + // agent-fix-unverified kind exists for that explicit case). + if ( + resolution.evidence.kind === "agent-fix" && + typeof resolution.evidence.commitSha === "string" && + resolution.evidence.commitSha.trim().length > 0 + ) { + const status = verifyCommitExists( + resolution.evidence.commitSha, + basePath, + ); + if (status === "missing") return false; + } if (isForgeRepo(basePath) && isDbAvailable()) { try { importLegacyJsonlToDb(basePath); diff --git a/src/resources/extensions/sf/tests/self-feedback-db.test.mjs b/src/resources/extensions/sf/tests/self-feedback-db.test.mjs index 85aa25f86..06cdc1f61 100644 --- a/src/resources/extensions/sf/tests/self-feedback-db.test.mjs +++ b/src/resources/extensions/sf/tests/self-feedback-db.test.mjs @@ -207,6 +207,94 @@ test("markResolved_accepts_each_canonical_evidence_kind", () => { } }); +test("markResolved_rejects_agent_fix_with_nonexistent_commit_sha", async () => { + const project = makeForgeProject(); + // Initialize a real git repo so the verifier can distinguish "commit + // missing" from "not a repo". Without a repo, the carve-out returns + // "ungrokable" and the resolution would be accepted. + const { execFileSync } = await import("node:child_process"); + execFileSync("git", ["init", "-q", "-b", "main"], { cwd: project }); + execFileSync("git", ["config", "user.email", "test@example.com"], { + cwd: project, + }); + execFileSync("git", ["config", "user.name", "test"], { cwd: project }); + execFileSync("git", ["commit", "--allow-empty", "-m", "init", "-q"], { + cwd: project, + }); + + const filed = recordSelfFeedback( + { kind: "commit-verif-test", severity: "high", summary: "x" }, + project, + ); + + // Bogus SHA — git rev-parse --verify exits non-zero + const okBogus = markResolved( + filed.entry.id, + { + reason: "phantom fix", + evidence: { + kind: "agent-fix", + commitSha: "0000000000000000000000000000000000000000", + }, + }, + project, + ); + assert.equal(okBogus, false); + + // Real SHA from the init commit — accepted + const headSha = execFileSync("git", ["rev-parse", "HEAD"], { + cwd: project, + encoding: "utf-8", + }).trim(); + const okReal = markResolved( + filed.entry.id, + { + reason: "verified fix", + evidence: { kind: "agent-fix", commitSha: headSha }, + }, + project, + ); + assert.equal(okReal, true); +}); + +test("markResolved_accepts_agent_fix_with_no_commit_sha_or_ungrokable_path", () => { + const project = makeForgeProject(); + // No git init → ungrokable → carve-out allows the resolution + const filed = recordSelfFeedback( + { kind: "no-git-test", severity: "low", summary: "x" }, + project, + ); + const ok = markResolved( + filed.entry.id, + { + reason: "verified some other way", + evidence: { kind: "agent-fix", commitSha: "anything-goes-no-repo" }, + }, + project, + ); + assert.equal(ok, true); + + // agent-fix without any commitSha is also accepted (commit not always + // the right artifact — summaryNarrative or testPath may be) + const filed2 = recordSelfFeedback( + { kind: "no-sha-test", severity: "low", summary: "x" }, + project, + ); + const ok2 = markResolved( + filed2.entry.id, + { + reason: "test-based verification", + evidence: { + kind: "agent-fix", + testPath: "src/foo.test.mjs", + summaryNarrative: "added a test that pins the behavior", + }, + }, + project, + ); + assert.equal(ok2, true); +}); + test("markResolved_appends_resolution_event_to_jsonl_audit_log", () => { const project = makeForgeProject(); const result = recordSelfFeedback(