fix(self-feedback): reject non-canonical evidence at the writer layer
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) <noreply@anthropic.com>
This commit is contained in:
parent
6d27cba067
commit
d477ce7039
2 changed files with 109 additions and 1 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue