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:
parent
62b19d7ba4
commit
6d27cba067
2 changed files with 238 additions and 8 deletions
214
src/resources/extensions/sf/tests/upstream-bridge.test.mjs
Normal file
214
src/resources/extensions/sf/tests/upstream-bridge.test.mjs
Normal 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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -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;
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue