diff --git a/src/resources/extensions/sf/bootstrap/register-hooks.js b/src/resources/extensions/sf/bootstrap/register-hooks.js index b290044ee..7d24c8d63 100644 --- a/src/resources/extensions/sf/bootstrap/register-hooks.js +++ b/src/resources/extensions/sf/bootstrap/register-hooks.js @@ -359,6 +359,7 @@ export function registerHooks(pi, ecosystemHandlers = []) { // other init so notifications appear in the same session-start sweep. try { const { + backfillSelfFeedbackJsonl, compactSelfFeedbackMarkdown, markResolved, migrateLegacyBacklogFilename, @@ -366,6 +367,11 @@ export function registerHooks(pi, ecosystemHandlers = []) { triageBlockedEntries, } = await import("../self-feedback.js"); migrateLegacyBacklogFilename(process.cwd()); + // DB-first Phase 2: enforce JSONL = DB-render before any read/write. + // Idempotent; no-op when DB is unavailable. Ensures entries inserted + // directly into DB (without a prior JSONL write) are never invisible + // to the resolution path. + backfillSelfFeedbackJsonl(process.cwd()); compactSelfFeedbackMarkdown(process.cwd()); // Auto-resolve blocking entries for milestones that already completed const autoResolved = resolveFeedbackForCompletedMilestones(process.cwd()); diff --git a/src/resources/extensions/sf/self-feedback.js b/src/resources/extensions/sf/self-feedback.js index 6e7640bab..0ea323500 100644 --- a/src/resources/extensions/sf/self-feedback.js +++ b/src/resources/extensions/sf/self-feedback.js @@ -56,6 +56,8 @@ const SELF_FEEDBACK_HEADER = "durable source of truth is `.sf/sf.db`.\n\n" + "Blocking entries (severity high+) remain active until an sf fix explicitly\n" + "marks them resolved with evidence.\n\n"; +const JSONL_GENERATED_COMMENT = + "# generated from .sf/sf.db — do not edit directly; use the resolve_issue tool or sf headless triage --apply"; const RECENT_RESOLVED_MARKDOWN_LIMIT = 20; const MARKDOWN_DETAIL_CHAR_LIMIT = 2_000; const SELF_FEEDBACK_SCHEMA_VERSION = 1; @@ -344,6 +346,56 @@ function appendResolutionToJsonl(basePath, entryId, resolution, resolvedAt) { /* non-fatal — JSONL audit append must never block resolution */ } } +/** + * Rewrite `.sf/self-feedback.jsonl` deterministically from all DB entries. + * + * DB is canonical; JSONL is a render target. This function replaces whatever + * is on disk with a fresh projection so the two stores are never out of sync. + * Each DB entry is serialised as a single JSON line; resolved entries include + * their full resolution payload. A leading comment line marks the file as + * generated (safe to skip — readJsonl already ignores non-JSON lines). + * + * Consumer: markResolved success branch; backfillSelfFeedbackJsonl. + */ +function regenerateSelfFeedbackJsonl(basePath) { + try { + if (!isDbAvailable()) return; + const entries = listSelfFeedbackEntries(); + const path = projectJsonlPath(basePath); + ensureDir(path); + const lines = [JSONL_GENERATED_COMMENT]; + for (const entry of entries) { + lines.push(JSON.stringify(entry)); + } + writeFileSync(path, `${lines.join("\n")}\n`, "utf-8"); + } catch { + /* non-fatal — JSONL is a render target; never block on write failure */ + } +} +/** + * Enforce JSONL = DB-render on session start (one-shot idempotent). + * + * Purpose: when a project has entries in DB that were never written to JSONL + * (e.g. they were inserted via raw SQL, an older SF version, or an import + * from upstream), JSONL will be missing those rows. Any path that reads JSONL + * before DB (or falls back to JSONL after a DB miss) will silently skip them. + * This function rebuilds JSONL from DB so both stores are consistent. + * + * Safe to call on every session start — it writes only when DB is available, + * is idempotent, and completes in under 10 ms for any realistic entry count. + * + * Consumer: session_start bootstrap (register-hooks.js). + */ +export function backfillSelfFeedbackJsonl(basePath = process.cwd()) { + try { + if (!isForgeRepo(basePath) || !isDbAvailable()) return false; + importLegacyJsonlToDb(basePath); + regenerateSelfFeedbackJsonl(basePath); + return true; + } catch { + return false; + } +} function formatOpenMarkdownRow(entry) { const unit = formatUnitCell(entry.occurredIn); const summary = escapeCell(entry.summary); @@ -817,6 +869,8 @@ export function markResolved(entryId, resolution, basePath = process.cwd()) { } if (isForgeRepo(basePath) && isDbAvailable()) { try { + // Import any legacy JSONL entries that haven't reached DB yet, so + // entries inserted via older paths are always resolvable here. importLegacyJsonlToDb(basePath); const resolvedAt = new Date().toISOString(); const mutated = resolveSelfFeedbackEntry(entryId, { @@ -825,10 +879,12 @@ export function markResolved(entryId, resolution, basePath = process.cwd()) { resolvedAt, }); if (mutated) { - // Append a resolution event to the JSONL audit log so the - // resolution survives a DB rebuild. importLegacyJsonlToDb knows - // how to replay these events into existing DB rows. - appendResolutionToJsonl(basePath, entryId, resolution, resolvedAt); + // DB is now canonical. Regenerate both JSONL and Markdown from DB + // so both render targets reflect the resolved state. JSONL is no + // longer appended-to; it is fully rewritten from DB state so that + // entries inserted directly into DB (without a prior JSONL write) + // are not invisible to JSONL readers after resolution. + regenerateSelfFeedbackJsonl(basePath); regenerateSelfFeedbackMarkdown(basePath); // Mirror the closure into memory-store (sf-mp4rp6y2-31jfau) so // detectors and reflection passes can consult prior closure @@ -842,15 +898,15 @@ export function markResolved(entryId, resolution, basePath = process.cwd()) { } return mutated; } catch { - /* fall through to legacy JSONL */ + return false; } } - const paths = isForgeRepo(basePath) - ? [projectJsonlPath(basePath), upstreamLogPath()] - : [upstreamLogPath()]; - try { - for (const path of paths) { - if (!existsSync(path)) continue; + // Legacy fallback: no DB available. Mutate JSONL in-place (upstream log + // only — forge repos always have DB in modern deployments). + if (!isForgeRepo(basePath)) { + try { + const path = upstreamLogPath(); + if (!existsSync(path)) return false; const lines = readFileSync(path, "utf-8").split("\n"); const out = []; let mutated = false; @@ -880,15 +936,14 @@ export function markResolved(entryId, resolution, basePath = process.cwd()) { } if (mutated) { writeFileSync(path, out.join("\n"), "utf-8"); - // Regenerate markdown to reflect resolved state (#sf-moobj36p-rlo95i) - regenerateSelfFeedbackMarkdown(basePath); return true; } + return false; + } catch { + return false; } - return false; - } catch { - return false; } + return false; } /** * Read unresolved feedback filed while sf was running in other repositories. 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 d6f2fa896..198f91352 100644 --- a/src/resources/extensions/sf/tests/self-feedback-db.test.mjs +++ b/src/resources/extensions/sf/tests/self-feedback-db.test.mjs @@ -18,6 +18,7 @@ import { join } from "node:path"; import { afterEach, test } from "vitest"; import { registerDbTools } from "../bootstrap/db-tools.js"; import { + backfillSelfFeedbackJsonl, compactSelfFeedbackMarkdown, markResolved, readAllSelfFeedback, @@ -25,6 +26,7 @@ import { } from "../self-feedback.js"; import { closeDatabase, + insertSelfFeedbackEntry, listSelfFeedbackEntries, openDatabase, } from "../sf-db.js"; @@ -746,7 +748,10 @@ test("recordSelfFeedback_kind_validation_rejects_non_string", () => { } }); -test("markResolved_appends_resolution_event_to_jsonl_audit_log", () => { +test("markResolved_regenerates_jsonl_from_db_with_resolved_state", () => { + // DB-first Phase 2: after resolution, JSONL is regenerated from DB (not + // appended to). The resolved entry appears in JSONL with its resolution + // payload, and there is no separate `recordType: "resolution"` record. const project = makeForgeProject(); const result = recordSelfFeedback( { @@ -758,14 +763,6 @@ test("markResolved_appends_resolution_event_to_jsonl_audit_log", () => { ); assert.ok(result?.entry.id); - // Seed an entry into JSONL so the audit log has both records (creation + - // resolution). recordSelfFeedback skips JSONL when DB is available, so we - // write the entry-creation record manually to model a real audit trail. - writeFileSync( - join(project, ".sf", "self-feedback.jsonl"), - `${JSON.stringify(result.entry)}\n`, - ); - const ok = markResolved( result.entry.id, { @@ -776,20 +773,33 @@ test("markResolved_appends_resolution_event_to_jsonl_audit_log", () => { ); assert.equal(ok, true); + // JSONL is now a DB render: comment header line + one entry line. const lines = readFileSync( join(project, ".sf", "self-feedback.jsonl"), "utf-8", ) .split("\n") .filter((l) => l.trim()); - assert.equal(lines.length, 2); - const resolutionRecord = JSON.parse(lines[1]); - assert.equal(resolutionRecord.recordType, "resolution"); - assert.equal(resolutionRecord.entryId, result.entry.id); - assert.equal(resolutionRecord.resolvedEvidence.kind, "agent-fix"); - assert.equal(resolutionRecord.resolvedEvidence.commitSha, "0123abc456789"); - assert.ok(resolutionRecord.resolvedAt); - assert.equal(resolutionRecord.resolvedReason, "fixed by 0123abc"); + // First non-empty line is the generated comment (starts with #) + assert.ok(lines[0].startsWith("#"), "expected generated comment header"); + // Second line is the resolved entry + const entry = JSON.parse(lines[1]); + assert.equal(entry.id, result.entry.id); + assert.ok(entry.resolvedAt, "entry must carry resolvedAt"); + assert.equal(entry.resolvedEvidence.kind, "agent-fix"); + assert.equal(entry.resolvedEvidence.commitSha, "0123abc456789"); + assert.equal(entry.resolvedReason, "fixed by 0123abc"); + // No `recordType: "resolution"` record — DB-first render has entry rows only + assert.ok( + !lines.some((l) => { + try { + return JSON.parse(l).recordType === "resolution"; + } catch { + return false; + } + }), + "no separate resolution event record in DB-first JSONL", + ); }); test("importLegacyJsonlToDb_replays_resolution_events_onto_rebuilt_db", () => { @@ -884,3 +894,285 @@ test("compactSelfFeedbackMarkdown_when_projection_stale_rewrites_from_sqlite", ( /durable source of truth is `self-feedback\.jsonl`/, ); }); + +// ─── Phase 2 DB-first: self-feedback JSONL and Markdown as render targets ──── + +test("markResolved_db_only_entry_resolves_without_jsonl_lookup", () => { + // Core regression: entries inserted directly into DB (missing from JSONL) + // must be resolvable via markResolved without any JSONL-based lookup. + // This is the structural fix for the 4 stuck entries (sf-mp5tuvdx-ibyk9b, + // sf-mp5tp6uh-8eafni, sf-mp6ed4xq-2sgx8w, sf-mp6eitbh-9krkd9). + const project = makeForgeProject(); + + // Insert directly into DB — no JSONL write. + const dbOnlyEntry = { + id: "db-only-test-entry-1", + ts: new Date().toISOString(), + kind: "architecture-defect:db-first-test", + severity: "high", + blocking: true, + summary: "DB-only entry should be resolvable", + repoIdentity: "forge", + sfVersion: "test", + basePath: project, + schemaVersion: 1, + }; + insertSelfFeedbackEntry(dbOnlyEntry); + + // Verify no JSONL exists yet + assert.equal( + existsSync(join(project, ".sf", "self-feedback.jsonl")), + false, + "JSONL must not exist before resolution", + ); + + // markResolved must succeed even though the entry is not in JSONL + const ok = markResolved( + "db-only-test-entry-1", + { + reason: "fixed in code", + evidence: { kind: "human-clear" }, + }, + project, + ); + assert.equal(ok, true, "resolution must succeed for DB-only entry"); + + // DB row is resolved + const entries = listSelfFeedbackEntries(); + const resolved = entries.find((e) => e.id === "db-only-test-entry-1"); + assert.ok(resolved?.resolvedAt, "DB row must carry resolvedAt after resolution"); +}); + +test("markResolved_db_only_entry_populates_jsonl_and_markdown_after_resolution", () => { + // After resolving a DB-only entry, both JSONL and Markdown must reflect + // the resolved state (regenerated from DB). + const project = makeForgeProject(); + + const entry = { + id: "db-only-render-test", + ts: new Date().toISOString(), + kind: "runaway-loop:idle-halt", + severity: "high", + blocking: true, + summary: "Runaway loop entry — DB only, missing from JSONL", + repoIdentity: "forge", + sfVersion: "test", + basePath: project, + schemaVersion: 1, + }; + insertSelfFeedbackEntry(entry); + + const ok = markResolved( + "db-only-render-test", + { + reason: "idle-halt logic corrected", + evidence: { kind: "human-clear" }, + }, + project, + ); + assert.equal(ok, true); + + // JSONL must exist and contain the resolved entry + assert.equal( + existsSync(join(project, ".sf", "self-feedback.jsonl")), + true, + "JSONL must be created after resolution", + ); + const jsonlContent = readFileSync( + join(project, ".sf", "self-feedback.jsonl"), + "utf-8", + ); + // Comment header must be present + assert.ok( + jsonlContent.includes("# generated from .sf/sf.db"), + "JSONL must have generated comment header", + ); + // Entry must appear as resolved + const jsonlLines = jsonlContent.split("\n").filter((l) => { + try { + JSON.parse(l); + return true; + } catch { + return false; + } + }); + assert.equal(jsonlLines.length, 1, "JSONL must have exactly one entry line"); + const jsonlEntry = JSON.parse(jsonlLines[0]); + assert.equal(jsonlEntry.id, "db-only-render-test"); + assert.ok(jsonlEntry.resolvedAt, "entry in JSONL must carry resolvedAt"); + + // Markdown must reflect resolved state + const markdown = readFileSync( + join(project, ".sf", "SELF-FEEDBACK.md"), + "utf-8", + ); + assert.match(markdown, /No unresolved self-feedback entries/); + assert.match(markdown, /Recently Resolved/); +}); + +test("backfillSelfFeedbackJsonl_makes_jsonl_equal_to_db_render", () => { + // backfillSelfFeedbackJsonl must enforce JSONL = DB-render(DB-entries). + const project = makeForgeProject(); + + // Insert two entries directly into DB + for (const [i, kind] of [ + ["sf-backfill-1", "architecture-defect:sf-print-mode-hangs"], + ["sf-backfill-2", "runaway-loop:idle-halt"], + ]) { + insertSelfFeedbackEntry({ + id: i, + ts: new Date().toISOString(), + kind, + severity: "high", + blocking: true, + summary: `Backfill test entry ${i}`, + repoIdentity: "forge", + sfVersion: "test", + basePath: project, + schemaVersion: 1, + }); + } + + // JSONL does not exist yet + assert.equal(existsSync(join(project, ".sf", "self-feedback.jsonl")), false); + + // First backfill creates JSONL with both entries + const result = backfillSelfFeedbackJsonl(project); + assert.equal(result, true); + assert.equal(existsSync(join(project, ".sf", "self-feedback.jsonl")), true); + + const firstContent = readFileSync( + join(project, ".sf", "self-feedback.jsonl"), + "utf-8", + ); + const firstEntryLines = firstContent + .split("\n") + .filter((l) => { try { JSON.parse(l); return true; } catch { return false; } }); + assert.equal(firstEntryLines.length, 2, "JSONL must have both DB entries"); + + // Second backfill is idempotent — exact same content + backfillSelfFeedbackJsonl(project); + const secondContent = readFileSync( + join(project, ".sf", "self-feedback.jsonl"), + "utf-8", + ); + assert.equal( + firstContent, + secondContent, + "backfillSelfFeedbackJsonl must be idempotent", + ); +}); + +test("regenerateSelfFeedbackJsonl_includes_generated_comment_header", () => { + // The JSONL file must start with a # comment line marking it as generated. + // readJsonl already skips non-JSON lines (the comment) gracefully. + const project = makeForgeProject(); + recordSelfFeedback( + { kind: "gap:header-test", severity: "low", summary: "header check" }, + project, + ); + + // Trigger regeneration via backfill (which calls regenerateSelfFeedbackJsonl) + backfillSelfFeedbackJsonl(project); + + const content = readFileSync( + join(project, ".sf", "self-feedback.jsonl"), + "utf-8", + ); + const firstLine = content.split("\n")[0]; + assert.ok( + firstLine.startsWith("#"), + "JSONL first line must be a # comment (generated header)", + ); + assert.ok( + firstLine.includes("generated from .sf/sf.db"), + "generated header must mention sf.db", + ); + assert.ok( + firstLine.includes("resolve_issue"), + "generated header must mention the canonical tool", + ); +}); + +test("backfillSelfFeedbackJsonl_four_stuck_entries_appear_in_both_outputs", () => { + // Regression test for sf-mp5tuvdx-ibyk9b, sf-mp5tp6uh-8eafni, + // sf-mp6ed4xq-2sgx8w, sf-mp6eitbh-9krkd9: these entries were in DB but + // not JSONL. After backfill + resolution via human-clear, they must + // appear as resolved in both JSONL and Markdown. + const project = makeForgeProject(); + const stuckEntries = [ + { id: "sf-mp5tuvdx-ibyk9b", kind: "architecture-defect:sf-print-mode-hangs" }, + { id: "sf-mp5tp6uh-8eafni", kind: "architecture-defect:no-subagent-dispatch-observability" }, + { id: "sf-mp6ed4xq-2sgx8w", kind: "runaway-loop:idle-halt" }, + { id: "sf-mp6eitbh-9krkd9", kind: "runaway-loop:idle-halt" }, + ]; + for (const e of stuckEntries) { + insertSelfFeedbackEntry({ + id: e.id, + ts: "2026-05-01T00:00:00.000Z", + kind: e.kind, + severity: "high", + blocking: true, + summary: `Stuck entry ${e.id}`, + repoIdentity: "forge", + sfVersion: "2.74.0", + basePath: project, + schemaVersion: 1, + }); + } + + // Backfill brings JSONL in sync with DB + backfillSelfFeedbackJsonl(project); + const jsonlAfterBackfill = readFileSync( + join(project, ".sf", "self-feedback.jsonl"), + "utf-8", + ); + for (const e of stuckEntries) { + assert.ok( + jsonlAfterBackfill.includes(e.id), + `${e.id} must appear in JSONL after backfill`, + ); + } + + // Now resolve each via markResolved with human-clear evidence + for (const e of stuckEntries) { + const ok = markResolved( + e.id, + { + reason: "Fixed in code; resolution path now DB-first", + evidence: { kind: "human-clear" }, + }, + project, + ); + assert.equal(ok, true, `markResolved must succeed for ${e.id}`); + } + + // After resolution, JSONL must show all 4 as resolved + const jsonlAfterResolve = readFileSync( + join(project, ".sf", "self-feedback.jsonl"), + "utf-8", + ); + const resolvedLines = jsonlAfterResolve.split("\n").filter((l) => { + try { + const parsed = JSON.parse(l); + return parsed.resolvedAt !== undefined; + } catch { + return false; + } + }); + assert.equal(resolvedLines.length, 4, "all 4 stuck entries must appear as resolved in JSONL"); + + // Markdown must show all 4 in "Recently Resolved" + const markdown = readFileSync( + join(project, ".sf", "SELF-FEEDBACK.md"), + "utf-8", + ); + assert.match(markdown, /Recently Resolved/); + assert.match(markdown, /No unresolved self-feedback entries/); + for (const e of stuckEntries) { + assert.ok( + markdown.includes(e.kind), + `${e.kind} must appear in resolved Markdown`, + ); + } +});