From d477ce70394f15a4adb65c32562d219093cf8b5a Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Thu, 14 May 2026 04:40:52 +0200 Subject: [PATCH] fix(self-feedback): reject non-canonical evidence at the writer layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses sf-mp4qoby4-meiir7: the credibility check at the READER side of self-feedback (selectInlineFixCandidates) was previously the only gate. An agent that wrote DB rows directly via raw SQL or the wrong tool could bypass it, landing resolutions like {file, line} or null that the reader would then either trust (legacy carve-out) or quietly re-open. Observed live in 2026-05-13 dogfood (5/5 sloppy resolutions with non-canonical evidence shapes). This commit makes the policy belt-and-suspenders: markResolved (and by extension resolveSelfFeedbackEntry) refuse to write resolutions whose evidence.kind is not in the accepted set: agent-fix, human-clear, promoted-to-requirement, auto-version-bump, agent-fix-unverified (reserved for outcomes-verification follow-up) When evidence is missing, non-object, or its kind is outside the set, markResolved returns false WITHOUT touching the DB or JSONL — caller recovers by re-submitting with a valid kind. All existing callers (resolve_issue tool, requirement-promoter, auto-version-bump resolver, triage-self-feedback) already pass valid kinds; no breakage. Raw SQL bypass is a known limit documented in the entry — full coverage needs a DB CHECK constraint on resolved_evidence_json (schema migration, separate work). Tests: 2 new (markResolved_rejects_non_canonical, accepts_each_canonical) covering all four rejection paths (bad kind, missing kind, missing evidence, unknown kind) and all five accepted kinds. Full SF extension suite (1547 tests) passes; typecheck clean. Plus inline cleanup: closed 3 stale upstream-rollup re-files (sf-mp4qyotx, sf-mp4qyoub, sf-mp4qyouh) with human-clear evidence — the bridge fix in 6d27cba06 now prevents recurrence. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/resources/extensions/sf/self-feedback.js | 43 +++++++++++- .../sf/tests/self-feedback-db.test.mjs | 67 +++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/resources/extensions/sf/self-feedback.js b/src/resources/extensions/sf/self-feedback.js index 6ff18f29c..524d359fe 100644 --- a/src/resources/extensions/sf/self-feedback.js +++ b/src/resources/extensions/sf/self-feedback.js @@ -405,17 +405,53 @@ export function getBlockedEntries(basePath = process.cwd()) { (e) => e.blocking && !e.resolvedAt, ); } +/** + * Accepted resolution evidence kinds. Anything outside this set is rejected + * at the writer layer (sf-mp4qoby4-meiir7): the credibility check at the + * READER side (self-feedback-drain.js) was previously the only gate, and an + * agent that wrote DB rows directly via raw SQL or the wrong tool could + * bypass it, landing resolutions like `{file, line}` or `null` that the + * reader would then either trust (legacy carve-out) or quietly re-open. The + * writer-layer check makes the policy belt-and-suspenders for any caller + * going through markResolved or resolveSelfFeedbackEntry. Raw SQL bypass is + * a known limit documented in the entry; full coverage needs a DB CHECK + * constraint (schema migration, separate work). + */ +const ACCEPTED_EVIDENCE_KINDS = new Set([ + "agent-fix", + "human-clear", + "promoted-to-requirement", + "auto-version-bump", + // "agent-fix-unverified" is reserved for outcomes-verification (a future + // follow-up): if a caller wants to mark a resolution as agent-attributed + // but explicitly NOT verified, it should write this kind so the + // credibility check re-opens the entry until verification lands. + "agent-fix-unverified", +]); + +function isAcceptedEvidence(evidence) { + if (!evidence || typeof evidence !== "object" || Array.isArray(evidence)) + return false; + const kind = evidence.kind; + return typeof kind === "string" && ACCEPTED_EVIDENCE_KINDS.has(kind); +} + /** * Mark an entry as resolved. Updates SQLite for forge-local feedback when * available, otherwise rewrites the legacy JSONL fallback in place. Entries are * append-only otherwise; resolution is the one mutation we support so blocking * entries don't trigger re-queue forever. * - * Resolution requires structured `evidence` so the fix is traceable: + * Resolution requires structured `evidence` with one of the accepted kinds + * (sf-mp4qoby4-meiir7 — writer-layer constraint): * - `agent-fix` should cite a commit SHA or test path * - `auto-version-bump` is for session_start's automatic resolver * - `human-clear` is the catch-all for operator interventions * - `promoted-to-requirement` is for the threshold-promotion sweeper + * - `agent-fix-unverified` for outcomes-verification-pending resolutions + * + * Resolutions without an accepted evidence kind are REJECTED (return false) + * — no DB write, no JSONL write, no markdown regeneration. * * If the entry has `acceptanceCriteria`, callers SHOULD pass `criteriaMet` * naming which criteria were satisfied. (Not enforced — entries without @@ -424,6 +460,11 @@ export function getBlockedEntries(basePath = process.cwd()) { * After resolution, SELF-FEEDBACK.md is regenerated as a compact working view. */ export function markResolved(entryId, resolution, basePath = process.cwd()) { + if (!isAcceptedEvidence(resolution?.evidence)) { + // Writer-layer constraint: refuse non-canonical resolutions. The + // caller can recover by re-submitting with a valid evidence kind. + 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 974c83c83..85aa25f86 100644 --- a/src/resources/extensions/sf/tests/self-feedback-db.test.mjs +++ b/src/resources/extensions/sf/tests/self-feedback-db.test.mjs @@ -140,6 +140,73 @@ test("markResolved_when_db_available_updates_sqlite_and_markdown_projection", () assert.match(markdown, /Recently Resolved/); }); +test("markResolved_rejects_resolution_with_non_canonical_evidence_kind", () => { + const project = makeForgeProject(); + const result = recordSelfFeedback( + { + kind: "writer-layer-test", + severity: "medium", + summary: "Must be rejected when evidence kind is non-canonical", + }, + project, + ); + assert.ok(result?.entry.id); + + // Non-canonical shape — exactly what the dogfood agent wrote (sf-mp4qoby4-meiir7) + const okBadKind = markResolved( + result.entry.id, + { + reason: "claims a fix", + evidence: { file: "src/foo.js", line: 42 }, + }, + project, + ); + assert.equal(okBadKind, false); + + // Missing evidence entirely + const okMissing = markResolved( + result.entry.id, + { reason: "no evidence" }, + project, + ); + assert.equal(okMissing, false); + + // Evidence kind not in the allowed set + const okWrongKind = markResolved( + result.entry.id, + { reason: "unknown kind", evidence: { kind: "spontaneous-resolution" } }, + project, + ); + assert.equal(okWrongKind, false); + + // All three rejections must have left the entry OPEN + const [entry] = readAllSelfFeedback(project); + assert.equal(entry.resolvedAt, undefined); +}); + +test("markResolved_accepts_each_canonical_evidence_kind", () => { + const project = makeForgeProject(); + const kinds = [ + { kind: "agent-fix", commitSha: "abc1234" }, + { kind: "human-clear" }, + { kind: "promoted-to-requirement", requirementId: "R042" }, + { kind: "auto-version-bump" }, + { kind: "agent-fix-unverified", reasonForUnverified: "no test yet" }, + ]; + for (const evidence of kinds) { + const filed = recordSelfFeedback( + { kind: `accepts-${evidence.kind}`, severity: "low", summary: "x" }, + project, + ); + const ok = markResolved( + filed.entry.id, + { reason: `using ${evidence.kind}`, evidence }, + project, + ); + assert.equal(ok, true, `expected ${evidence.kind} to be accepted`); + } +}); + test("markResolved_appends_resolution_event_to_jsonl_audit_log", () => { const project = makeForgeProject(); const result = recordSelfFeedback(