fix(upstream-bridge): suppress re-file of recently-closed rollup kinds

Addresses sf-mp4rp6xn-hpag5h: bridgeUpstreamFeedback's idempotency
check only looked at currently-OPEN upstream-rollup entries, so any
closure (human-clear or agent-fix) would let the bridge re-file the
same cluster on the next session_start. Observed live during 2026-05-13
dogfood: closed 3 upstream-rollup entries with human-clear, bridge
re-filed all 3 on the next run.

Change: extend the idempotency set to also exclude rollup kinds that
were RESOLVED within the last 30 days (matches the existing
THIRTY_DAYS_MS upstream-source cutoff — same window, same rationale).

Closures are treated as time-limited: after the window expires, a
re-cluster CAN re-file, because the original closure was made against
then-current state and later state may legitimately surface the same
kind again. This is the right balance — operators get respite from
re-files while the closure decision was fresh, without trapping the
ledger forever if conditions actually change.

7 new tests cover the regression (files new / skips open / skips
recently-closed / allows re-file after window / threshold guards /
non-forge-repo bail). Full SF extension suite (1545 tests) passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mikael Hugo 2026-05-14 04:37:10 +02:00
parent 62b19d7ba4
commit 6d27cba067
2 changed files with 238 additions and 8 deletions

View file

@ -0,0 +1,214 @@
/**
* upstream-bridge.test.mjs closure-aware idempotency.
*
* Validates that bridgeUpstreamFeedback respects:
* - currently-OPEN rollup kinds (existing behavior)
* - rollup kinds CLOSED within the last 30 days (new behavior addressing
* sf-mp4rp6xn-hpag5h: bridge re-filed every closed cluster on every
* session_start because it only checked open state)
* - allows re-file when the same kind was closed OUTSIDE the lookback
*/
import {
mkdirSync,
mkdtempSync,
rmSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, test } from "vitest";
import { bridgeUpstreamFeedback } from "../upstream-bridge.js";
import {
closeDatabase,
listSelfFeedbackEntries,
openDatabase,
} from "../sf-db.js";
import { markResolved, recordSelfFeedback } from "../self-feedback.js";
const tmpDirs = [];
const ORIGINAL_SF_HOME = process.env.SF_HOME;
beforeEach(() => {
delete process.env.SF_HOME;
});
afterEach(() => {
closeDatabase();
if (ORIGINAL_SF_HOME === undefined) {
delete process.env.SF_HOME;
} else {
process.env.SF_HOME = ORIGINAL_SF_HOME;
}
while (tmpDirs.length > 0) {
const dir = tmpDirs.pop();
if (dir) rmSync(dir, { recursive: true, force: true });
}
});
function makeForgeProject() {
const dir = mkdtempSync(join(tmpdir(), "sf-bridge-"));
tmpDirs.push(dir);
mkdirSync(join(dir, ".sf"), { recursive: true });
writeFileSync(
join(dir, "package.json"),
JSON.stringify({ name: "singularity-forge" }),
);
openDatabase(join(dir, ".sf", "sf.db"));
return dir;
}
function makeSfHome(upstreamEntries) {
const dir = mkdtempSync(join(tmpdir(), "sf-home-"));
tmpDirs.push(dir);
mkdirSync(join(dir, "agent"), { recursive: true });
const path = join(dir, "agent", "upstream-feedback.jsonl");
writeFileSync(
path,
upstreamEntries.map((e) => JSON.stringify(e)).join("\n") + "\n",
);
process.env.SF_HOME = dir;
return dir;
}
function makeUpstreamEntry(overrides = {}) {
return {
schemaVersion: 1,
id: `sf-upstream-${Math.random().toString(36).slice(2, 8)}`,
ts: new Date().toISOString(),
kind: "runaway-guard-hard-pause",
severity: "medium",
blocking: false,
summary: "synthetic upstream observation",
evidence: "tmpdir test",
repoIdentity: "external",
sfVersion: "test",
basePath: `/some/external/repo-${Math.random().toString(36).slice(2, 6)}`,
...overrides,
};
}
describe("bridgeUpstreamFeedback", () => {
test("files a new rollup when threshold met and no prior matching kind", () => {
const project = makeForgeProject();
// 3 entries of same kind, 2 distinct repos (meets THRESHOLD_COUNT=3, THRESHOLD_REPOS=2)
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/b" }),
]);
const filed = bridgeUpstreamFeedback(project);
expect(filed).toBe(1);
const entries = listSelfFeedbackEntries();
const rollup = entries.find((e) =>
e.kind?.startsWith("upstream-rollup:"),
);
expect(rollup).toBeTruthy();
expect(rollup.kind).toBe("upstream-rollup:runaway-guard-hard-pause");
});
test("skips when an OPEN rollup of the same kind already exists", () => {
const project = makeForgeProject();
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/b" }),
]);
expect(bridgeUpstreamFeedback(project)).toBe(1);
expect(bridgeUpstreamFeedback(project)).toBe(0);
});
test("skips when a same-kind rollup was RESOLVED within the lookback window (sf-mp4rp6xn-hpag5h)", async () => {
const project = makeForgeProject();
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/b" }),
]);
// File once, then close with human-clear (today)
expect(bridgeUpstreamFeedback(project)).toBe(1);
const filed = listSelfFeedbackEntries().find((e) =>
e.kind?.startsWith("upstream-rollup:"),
);
const ok = markResolved(
filed.id,
{
reason: "out of scope for inline-fix",
evidence: { kind: "human-clear" },
},
project,
);
expect(ok).toBe(true);
// Re-running the bridge MUST NOT re-file. This is the regression guard.
const refiled = bridgeUpstreamFeedback(project);
expect(refiled).toBe(0);
});
test("allows re-file when same-kind rollup was resolved BEFORE lookback (>30d)", async () => {
const project = makeForgeProject();
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/b" }),
]);
expect(bridgeUpstreamFeedback(project)).toBe(1);
const filed = listSelfFeedbackEntries().find((e) =>
e.kind?.startsWith("upstream-rollup:"),
);
markResolved(
filed.id,
{ reason: "ancient close", evidence: { kind: "human-clear" } },
project,
);
// Back-date the resolution to 60 days ago via direct DB update
const oldTs = new Date(
Date.now() - 60 * 24 * 60 * 60 * 1000,
).toISOString();
const { _getAdapter } = await import("../sf-db/sf-db-core.js");
_getAdapter()
.prepare("UPDATE self_feedback SET resolved_at = :ts WHERE id = :id")
.run({ ":ts": oldTs, ":id": filed.id });
// Bridge should re-file because the closure is now stale (>30 days)
const refiled = bridgeUpstreamFeedback(project);
expect(refiled).toBe(1);
});
test("doesn't fire below threshold (2 entries, 2 repos < 3-count threshold)", () => {
const project = makeForgeProject();
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/b" }),
]);
expect(bridgeUpstreamFeedback(project)).toBe(0);
});
test("doesn't fire below repo-distinct threshold (3 entries, 1 repo)", () => {
const project = makeForgeProject();
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
]);
expect(bridgeUpstreamFeedback(project)).toBe(0);
});
test("returns 0 when basePath isn't the forge repo", () => {
const dir = mkdtempSync(join(tmpdir(), "sf-non-forge-"));
tmpDirs.push(dir);
writeFileSync(
join(dir, "package.json"),
JSON.stringify({ name: "some-other-app" }),
);
makeSfHome([
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/a" }),
makeUpstreamEntry({ basePath: "/b" }),
]);
expect(bridgeUpstreamFeedback(dir)).toBe(0);
});
});

View file

@ -89,20 +89,36 @@ export function bridgeUpstreamFeedback(basePath = process.cwd()) {
list.push(e); list.push(e);
byKind.set(e.kind, list); byKind.set(e.kind, list);
} }
// Read existing forge-local entries once for idempotency checks // Read existing forge-local entries once for idempotency checks. We
// exclude both currently-OPEN rollup kinds AND rollup kinds that an
// operator (or the triage worker) closed within the last LOOKBACK_DAYS
// window. Without the closed-recently check, the bridge re-files the
// same cluster on the next session_start every time someone closes it
// — observed in dogfood 2026-05-13 (sf-mp4rp6xn-hpag5h). The closure
// is treated as time-limited: after the window expires, a re-cluster
// CAN re-file (the original closure was made against then-current
// state; later state may legitimately surface the same kind again).
const existing = readAllSelfFeedback(basePath); const existing = readAllSelfFeedback(basePath);
const openRollupKinds = new Set( const recentCutoff = Date.now() - THIRTY_DAYS_MS;
existing const suppressedRollupKinds = new Set();
.filter((e) => !e.resolvedAt && e.kind.startsWith("upstream-rollup:")) for (const e of existing) {
.map((e) => e.kind), if (!e.kind?.startsWith("upstream-rollup:")) continue;
); if (!e.resolvedAt) {
suppressedRollupKinds.add(e.kind);
continue;
}
const resolvedMs = new Date(e.resolvedAt).getTime();
if (Number.isFinite(resolvedMs) && resolvedMs >= recentCutoff) {
suppressedRollupKinds.add(e.kind);
}
}
let filed = 0; let filed = 0;
for (const [kind, entries] of byKind) { for (const [kind, entries] of byKind) {
if (entries.length < THRESHOLD_COUNT) continue; if (entries.length < THRESHOLD_COUNT) continue;
const distinctRepos = new Set(entries.map((e) => e.basePath)).size; const distinctRepos = new Set(entries.map((e) => e.basePath)).size;
if (distinctRepos < THRESHOLD_REPOS) continue; if (distinctRepos < THRESHOLD_REPOS) continue;
const rollupKind = `upstream-rollup:${kind}`; const rollupKind = `upstream-rollup:${kind}`;
if (openRollupKinds.has(rollupKind)) continue; if (suppressedRollupKinds.has(rollupKind)) continue;
// Derive severity, capped at medium // Derive severity, capped at medium
const severity = capSeverity(maxSeverity(entries)); const severity = capSeverity(maxSeverity(entries));
// Build evidence block: up to 5 samples + full id list // Build evidence block: up to 5 samples + full id list
@ -129,7 +145,7 @@ export function bridgeUpstreamFeedback(basePath = process.cwd()) {
); );
if (result) { if (result) {
filed++; filed++;
openRollupKinds.add(rollupKind); // prevent double-filing in same run suppressedRollupKinds.add(rollupKind); // prevent double-filing in same run
} }
} }
return filed; return filed;