diff --git a/packages/gittensory-engine/src/governor/self-plagiarism.ts b/packages/gittensory-engine/src/governor/self-plagiarism.ts index 0110815a26..d3902b4591 100644 --- a/packages/gittensory-engine/src/governor/self-plagiarism.ts +++ b/packages/gittensory-engine/src/governor/self-plagiarism.ts @@ -81,6 +81,24 @@ export function fingerprintSimilarity(left: string, right: string): number { return intersection / union; } +/** + * Build a real `OwnSubmissionRecord.fingerprint` from the real set of file paths a submission actually + * changed (`CodingAgentDriverResult.changedFiles`/`HandoffPacket.changedFiles`, never a fabricated or + * partial list). Comma-joined so `fingerprintSimilarity`'s own `tokenSet` splitter treats each path as one + * token -- two submissions touching mostly the same files read as near-duplicates. Deduped and sorted so the + * same real change set always produces the identical fingerprint regardless of the order paths were reported + * in. Empty input (no changed files) is an honest empty string, never a fabricated placeholder token. + */ +export function fingerprintFromChangedFiles(paths: readonly string[]): string { + const unique = new Set( + paths + .filter((path): path is string => typeof path === "string") + .map((path) => path.trim()) + .filter((path) => path.length > 0), + ); + return [...unique].sort().join(","); +} + function submissionTimeMs(value: string | null | undefined): number | null { if (!value) return null; const parsed = Date.parse(value); diff --git a/packages/gittensory-engine/test/self-plagiarism.test.ts b/packages/gittensory-engine/test/self-plagiarism.test.ts index e4057b0b2e..14332ecaac 100644 --- a/packages/gittensory-engine/test/self-plagiarism.test.ts +++ b/packages/gittensory-engine/test/self-plagiarism.test.ts @@ -4,6 +4,7 @@ import { test } from "node:test"; import { buildSelfPlagiarismGovernorLedgerEvent, DEFAULT_SELF_PLAGIARISM_SIMILARITY_THRESHOLD, + fingerprintFromChangedFiles, fingerprintSimilarity, resolveSelfPlagiarismConfig, selfPlagiarismCheck, @@ -35,6 +36,7 @@ function prior(overrides: Partial = {}): OwnSubmissionRecor test("barrel: the public entrypoint re-exports the self-plagiarism governor API (#2345)", () => { assert.equal(typeof selfPlagiarismCheck, "function"); assert.equal(typeof fingerprintSimilarity, "function"); + assert.equal(typeof fingerprintFromChangedFiles, "function"); assert.equal(typeof buildSelfPlagiarismGovernorLedgerEvent, "function"); assert.equal(typeof resolveSelfPlagiarismConfig, "function"); assert.equal(DEFAULT_SELF_PLAGIARISM_SIMILARITY_THRESHOLD, 0.85); @@ -127,3 +129,31 @@ test("fingerprintSimilarity: returns Jaccard overlap for token sets", () => { assert.equal(fingerprintSimilarity("abc def", "ABC DEF"), 1); assert.equal(fingerprintSimilarity("aa bb", "bb cc"), 1 / 3); }); + +test("fingerprintFromChangedFiles: sorts and comma-joins a real changed-file set", () => { + assert.equal(fingerprintFromChangedFiles(["src/b.ts", "src/a.ts"]), "src/a.ts,src/b.ts"); +}); + +test("fingerprintFromChangedFiles: dedupes repeated paths", () => { + assert.equal(fingerprintFromChangedFiles(["src/a.ts", "src/a.ts"]), "src/a.ts"); +}); + +test("fingerprintFromChangedFiles: is order-independent -- the same real change set always fingerprints identically", () => { + const first = fingerprintFromChangedFiles(["src/b.ts", "src/a.ts", "docs/c.md"]); + const second = fingerprintFromChangedFiles(["docs/c.md", "src/a.ts", "src/b.ts"]); + assert.equal(first, second); +}); + +test("fingerprintFromChangedFiles: an empty change set produces an honest empty string, never a fabricated token", () => { + assert.equal(fingerprintFromChangedFiles([]), ""); +}); + +test("fingerprintFromChangedFiles: blank/whitespace-only entries are dropped, not turned into empty tokens", () => { + assert.equal(fingerprintFromChangedFiles(["src/a.ts", " ", ""]), "src/a.ts"); +}); + +test("fingerprintFromChangedFiles: feeds fingerprintSimilarity as a real Jaccard token set (comma-delimited paths)", () => { + const a = fingerprintFromChangedFiles(["src/a.ts", "src/b.ts"]); + const b = fingerprintFromChangedFiles(["src/a.ts", "src/c.ts"]); + assert.equal(fingerprintSimilarity(a, b), 1 / 3); // {a,b} vs {a,c}: intersection 1, union 3 +}); diff --git a/packages/gittensory-miner/lib/attempt-cli.d.ts b/packages/gittensory-miner/lib/attempt-cli.d.ts index 59f909bb76..662966ccfe 100644 --- a/packages/gittensory-miner/lib/attempt-cli.d.ts +++ b/packages/gittensory-miner/lib/attempt-cli.d.ts @@ -13,6 +13,7 @@ import type { resolveAmsPolicy } from "./ams-policy.js"; import type { checkMinerKillSwitch } from "./governor-kill-switch.js"; import type { resolveMinerGoalSpec } from "./miner-goal-spec.js"; import type { ClaimConflictResult, resolveClaimConflict } from "./claim-conflict-resolver.js"; +import type { recordOwnSubmission } from "./governor-state.js"; import type { getAttemptHistory } from "./portfolio-queue.js"; type CommonAttemptResultFields = { @@ -93,6 +94,7 @@ export type RunAttemptOptions = { resolveMinerGoalSpec?: typeof resolveMinerGoalSpec; runMinerAttempt?: typeof runMinerAttempt; resolveClaimConflict?: typeof resolveClaimConflict; + recordOwnSubmission?: typeof recordOwnSubmission; getAttemptHistory?: typeof getAttemptHistory; /** Invoked with the real structured result at every return point, in addition to (never instead of) the * plain exit-code return -- the loop orchestrator's real hook into what actually happened. */ diff --git a/packages/gittensory-miner/lib/attempt-cli.js b/packages/gittensory-miner/lib/attempt-cli.js index 9b91fa1f07..f2aad1f806 100644 --- a/packages/gittensory-miner/lib/attempt-cli.js +++ b/packages/gittensory-miner/lib/attempt-cli.js @@ -13,7 +13,7 @@ // own design treats that as "skip that stage entirely"). governor.convergenceInput is now a real per-issue // portfolio-queue.js read (#5654), not a placeholder. -import { resolveCodingAgentModeFromConfig, resolveFirstConfiguredCodingAgentDriverName } from "@loopover/engine"; +import { fingerprintFromChangedFiles, resolveCodingAgentModeFromConfig, resolveFirstConfiguredCodingAgentDriverName } from "@loopover/engine"; import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js"; import { constructProductionCodingAgentDriver } from "./coding-agent-construction.js"; import { runSlopAssessment } from "./slop-assessment.js"; @@ -35,6 +35,7 @@ import { resolveAmsPolicy } from "./ams-policy.js"; import { checkMinerKillSwitch } from "./governor-kill-switch.js"; import { buildAttemptGovernorContext, buildAttemptLoopInput } from "./attempt-input-builder.js"; import { getAttemptHistory } from "./portfolio-queue.js"; +import { recordOwnSubmission } from "./governor-state.js"; import { runMinerAttempt } from "./attempt-runner.js"; const ATTEMPT_USAGE = @@ -456,6 +457,31 @@ export async function runAttempt(args, options = {}) { { fetchLiveIssueSnapshot: deps.fetchLiveIssueSnapshot, executeLocalWrite: deps.executeLocalWrite }, ); } + + // Real own-submission history (#5655 follow-up): governor-state.js's recordOwnSubmission/ + // listRecentOwnSubmissions store (#5134) existed and was already READ by resolveOwnRejectionHistory + // (#5655), but nothing ever WROTE to it -- attempt-runner.js's own header names this exact gap + // ("real persistence primitives... but isn't auto-loaded here yet"). Left unfixed, that trigger is a + // silent no-op in every real deployment: an empty table always resolves "no prior submissions found." + // The fingerprint is the real changed-files set from the loop's own handoff packet (never fabricated) -- + // omitted (not recorded as an empty placeholder) when the packet reports no changed files at all. A + // logging failure must never fail an otherwise-successful attempt, matching the summary-event write below. + const changedFiles = result.loopResult.handoffPacket?.changedFiles?.map((file) => file.path) ?? []; + const fingerprint = fingerprintFromChangedFiles(changedFiles); + if (fingerprint) { + try { + const record = options.recordOwnSubmission ?? recordOwnSubmission; + record({ + repoFullName: parsed.repoFullName, + fingerprint, + submittedAt: new Date(nowMs).toISOString(), + pullRequestNumber: selfPrNumber, + issueNumber: parsed.issueNumber, + }); + } catch { + // Deliberately swallowed -- see comment above. + } + } } const finalResult = { diff --git a/test/unit/miner-attempt-cli.test.ts b/test/unit/miner-attempt-cli.test.ts index b743418393..394ee4a31d 100644 --- a/test/unit/miner-attempt-cli.test.ts +++ b/test/unit/miner-attempt-cli.test.ts @@ -14,6 +14,7 @@ import type { AttemptLog } from "../../packages/gittensory-miner/lib/attempt-log import { closeDefaultGovernorLedger, initGovernorLedger } from "../../packages/gittensory-miner/lib/governor-ledger.js"; import { closeDefaultWorktreeAllocator, openWorktreeAllocator } from "../../packages/gittensory-miner/lib/worktree-allocator.js"; import { closeDefaultPortfolioQueueStore } from "../../packages/gittensory-miner/lib/portfolio-queue.js"; +import { closeDefaultGovernorState } from "../../packages/gittensory-miner/lib/governor-state.js"; import { buildAttemptDeps, parseAttemptArgs, runAttempt } from "../../packages/gittensory-miner/lib/attempt-cli.js"; import type { PrepareAttemptWorktreeResult } from "../../packages/gittensory-miner/lib/attempt-worktree.js"; import { DEFAULT_AMS_POLICY_SPEC, DEFAULT_MINER_GOAL_SPEC, parseFocusManifest } from "../../packages/gittensory-engine/src/index"; @@ -82,6 +83,9 @@ function readyPipelineOptions(overrides: Record = {}) { // Never touches the real (filesystem-backed) default portfolio-queue store (#5654) -- a test that cares // about a real convergenceInput value overrides this explicitly. getAttemptHistory: () => ({ attempts: 0, consecutiveFailures: 0, reenqueues: 0, reachedDone: false }), + // Never touches the real (filesystem-backed) default governor-state store (#5655 follow-up) -- a test + // that cares whether recordOwnSubmission was actually called overrides this explicitly. + recordOwnSubmission: vi.fn(), ...overrides, }; } @@ -108,6 +112,7 @@ afterEach(() => { closeDefaultAttemptLog(); closeDefaultGovernorLedger(); closeDefaultPortfolioQueueStore(); + closeDefaultGovernorState(); vi.unstubAllEnvs(); vi.restoreAllMocks(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); @@ -418,6 +423,167 @@ describe("runAttempt (#5132)", () => { }); }); + it("REGRESSION (#5655 follow-up): a real submitted outcome with a real changed-files set records real own-submission history, closing the gap that left resolveOwnRejectionHistory silently a no-op", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const worktreeResult = fakeWorktreeResult(); + const recordOwnSubmissionSpy = vi.fn(); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: worktreeResult.worktreePath, timeoutMs: 1000 }, + execResult: { code: 0, stdout: "https://github.com/acme/widgets/pull/9\n" }, + loopResult: fakeLoopResult({ + handoffPacket: { changedFiles: [{ path: "src/b.ts" }, { path: "src/a.ts" }] }, + }), + }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + nowMs: Date.parse("2026-07-13T12:00:00.000Z"), + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ recordOwnSubmission: recordOwnSubmissionSpy, runMinerAttempt: runMinerAttemptSpy }), + }); + + expect(recordOwnSubmissionSpy).toHaveBeenCalledWith({ + repoFullName: "acme/widgets", + // Sorted, comma-joined, deduped -- the real fingerprintFromChangedFiles contract (#5653 follow-up sibling). + fingerprint: "src/a.ts,src/b.ts", + submittedAt: "2026-07-13T12:00:00.000Z", + pullRequestNumber: 9, + issueNumber: 7, + }); + }); + + it("REGRESSION (#5655 follow-up): when options.recordOwnSubmission is omitted, runAttempt falls back to the REAL governor-state.js default, not a fabricated no-op", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-attempt-cli-governor-state-")); + roots.push(root); + vi.stubEnv("GITTENSORY_MINER_GOVERNOR_STATE_DB", join(root, "governor-state.sqlite3")); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0, stdout: "https://github.com/acme/widgets/pull/9\n" }, + loopResult: fakeLoopResult({ handoffPacket: { changedFiles: [{ path: "src/a.ts" }] } }), + }); + const { recordOwnSubmission: _omitted, ...optionsWithoutRecordOwnSubmission } = readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...optionsWithoutRecordOwnSubmission, + }); + + // A real write against the isolated store proves the real default (not a DI stub) actually ran. + const { listRecentOwnSubmissions } = await import("../../packages/gittensory-miner/lib/governor-state.js"); + const submissions = listRecentOwnSubmissions({ repoFullName: "acme/widgets" }); + expect(submissions).toEqual([ + expect.objectContaining({ repoFullName: "acme/widgets", fingerprint: "src/a.ts", pullRequestNumber: 9, issueNumber: 7 }), + ]); + }); + + it("does not record own-submission history when the loop's handoff packet reports no changed files -- an honest absence, never a fabricated fingerprint", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const recordOwnSubmissionSpy = vi.fn(); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0, stdout: "https://github.com/acme/widgets/pull/9\n" }, + loopResult: fakeLoopResult({ handoffPacket: { changedFiles: [] } }), + }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ recordOwnSubmission: recordOwnSubmissionSpy, runMinerAttempt: runMinerAttemptSpy }), + }); + + expect(recordOwnSubmissionSpy).not.toHaveBeenCalled(); + }); + + it("records own-submission history with a null pullRequestNumber when the real PR number couldn't be parsed from execResult -- an honest gap, not a skipped record", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const recordOwnSubmissionSpy = vi.fn(); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0 }, // no stdout -- PR number genuinely unrecoverable + loopResult: fakeLoopResult({ handoffPacket: { changedFiles: [{ path: "src/a.ts" }] } }), + }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ recordOwnSubmission: recordOwnSubmissionSpy, runMinerAttempt: runMinerAttemptSpy }), + }); + + expect(recordOwnSubmissionSpy).toHaveBeenCalledWith(expect.objectContaining({ pullRequestNumber: null })); + }); + + it("REGRESSION: a recordOwnSubmission failure never fails an otherwise-successful attempt", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const recordOwnSubmissionSpy = vi.fn().mockImplementation(() => { + throw new Error("disk full"); + }); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0, stdout: "https://github.com/acme/widgets/pull/9\n" }, + loopResult: fakeLoopResult({ handoffPacket: { changedFiles: [{ path: "src/a.ts" }] } }), + }); + + const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ recordOwnSubmission: recordOwnSubmissionSpy, runMinerAttempt: runMinerAttemptSpy }), + }); + + expect(exitCode).toBe(0); + expect(recordOwnSubmissionSpy).toHaveBeenCalled(); + }); + + it("does not record own-submission history on a non-submitted outcome", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const recordOwnSubmissionSpy = vi.fn(); + const runMinerAttemptSpy = vi.fn().mockResolvedValue({ outcome: "abandon", loopResult: fakeLoopResult() }); + + await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ recordOwnSubmission: recordOwnSubmissionSpy, runMinerAttempt: runMinerAttemptSpy }), + }); + + expect(recordOwnSubmissionSpy).not.toHaveBeenCalled(); + }); + it("REGRESSION (#5654): the real portfolio-queue attempt history is read for THIS issue and threads into governor.convergenceInput, not the old hardcoded literal", async () => { const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); vi.spyOn(console, "log").mockImplementation(() => undefined); diff --git a/test/unit/self-plagiarism.test.ts b/test/unit/self-plagiarism.test.ts index 9e1ffd0874..4b9360df4b 100644 --- a/test/unit/self-plagiarism.test.ts +++ b/test/unit/self-plagiarism.test.ts @@ -3,6 +3,7 @@ import { buildSelfPlagiarismGovernorLedgerEvent, DEFAULT_SELF_PLAGIARISM_CONFIG, DEFAULT_SELF_PLAGIARISM_SIMILARITY_THRESHOLD, + fingerprintFromChangedFiles, fingerprintSimilarity, resolveSelfPlagiarismConfig, selfPlagiarismCheck, @@ -54,6 +55,36 @@ describe("fingerprintSimilarity", () => { }); }); +describe("fingerprintFromChangedFiles", () => { + it("sorts and comma-joins a real changed-file set", () => { + expect(fingerprintFromChangedFiles(["src/b.ts", "src/a.ts"])).toBe("src/a.ts,src/b.ts"); + }); + + it("dedupes repeated paths", () => { + expect(fingerprintFromChangedFiles(["src/a.ts", "src/a.ts"])).toBe("src/a.ts"); + }); + + it("is order-independent -- the same real change set always fingerprints identically", () => { + const first = fingerprintFromChangedFiles(["src/b.ts", "src/a.ts", "docs/c.md"]); + const second = fingerprintFromChangedFiles(["docs/c.md", "src/a.ts", "src/b.ts"]); + expect(first).toBe(second); + }); + + it("produces an honest empty string for an empty change set, never a fabricated token", () => { + expect(fingerprintFromChangedFiles([])).toBe(""); + }); + + it("drops blank/whitespace-only entries instead of turning them into empty tokens", () => { + expect(fingerprintFromChangedFiles(["src/a.ts", " ", ""])).toBe("src/a.ts"); + }); + + it("feeds fingerprintSimilarity as a real Jaccard token set (comma-delimited paths)", () => { + const a = fingerprintFromChangedFiles(["src/a.ts", "src/b.ts"]); + const b = fingerprintFromChangedFiles(["src/a.ts", "src/c.ts"]); + expect(fingerprintSimilarity(a, b)).toBeCloseTo(1 / 3); // {a,b} vs {a,c}: intersection 1, union 3 + }); +}); + describe("resolveSelfPlagiarismConfig", () => { it("returns defaults for nullish and invalid top-level shapes", () => { expect(resolveSelfPlagiarismConfig(undefined)).toEqual({