From 007aae3634795acf7f6ce4e53e76e4df0d2ffa1b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 15:41:19 -0700 Subject: [PATCH] feat(miner): wire claim-conflict resolution end-to-end Closes #4848 claim-adjudication.js's isDuplicateClusterWinnerByClaim wrapper was correct and well-tested in isolation but had no caller that assembled a real competing-claims set from GitHub -- a genuine duplicate claim had no automated resolution path. checkSubmissionFreshness already catches the common pre-submission case (aborting before open_pr if another author's PR already references the issue), but that check can only see what's public at the moment it runs. Two miners racing closely enough that both pass their own freshness check before either's PR exists yet is a genuine TOCTOU window freshness cannot close. claim-conflict-resolver.js is the post-submission reconciliation for exactly that window: once this miner's PR is real and public, it fetches the live issue snapshot, assembles the real competing-claims set (every other OPEN PR referencing the issue, excluding this miner's own), and adjudicates via the existing claim-adjudication.js. When this miner's claim loses, it closes its own just-opened PR (never anyone else's) via a new close_pr local-write spec, citing the winner. Fails open (never closes anything) when the live snapshot can't be fetched. Wired into attempt-cli.js right after a real "submitted" outcome, using the miner's own real claim-ledger claimedAt for its side of the election and each competing PR's real GitHub createdAt as the best publicly-observable proxy for a third-party PR (documented asymmetry -- the maintainer gate's own "first observed" semantics need a continuous server-side observation history this stateless client-side tool doesn't have). Extracted the shared PR-number-from-exec-result parser (previously private to loop-cli.js) into pr-number-parse.js so both call sites agree on exactly one implementation. --- packages/gittensory-engine/src/index.ts | 1 + .../src/miner/local-write-tools.ts | 17 ++ .../gittensory-miner/lib/attempt-cli.d.ts | 3 + packages/gittensory-miner/lib/attempt-cli.js | 41 ++++- .../lib/claim-conflict-resolver.d.ts | 29 ++++ .../lib/claim-conflict-resolver.js | 97 +++++++++++ .../lib/live-issue-snapshot.js | 10 +- packages/gittensory-miner/lib/loop-cli.js | 15 +- .../gittensory-miner/lib/pr-number-parse.d.ts | 4 + .../gittensory-miner/lib/pr-number-parse.js | 23 +++ .../lib/submission-freshness-check.d.ts | 2 +- packages/gittensory-miner/package.json | 2 +- src/mcp/local-write-tools.ts | 1 + test/unit/local-write-tools.test.ts | 15 ++ test/unit/miner-attempt-cli.test.ts | 149 +++++++++++++++++ .../miner-claim-conflict-resolver.test.ts | 152 ++++++++++++++++++ test/unit/miner-live-issue-snapshot.test.ts | 8 +- test/unit/miner-pr-number-parse.test.ts | 40 +++++ .../miner-submission-freshness-check.test.ts | 14 +- 19 files changed, 591 insertions(+), 32 deletions(-) create mode 100644 packages/gittensory-miner/lib/claim-conflict-resolver.d.ts create mode 100644 packages/gittensory-miner/lib/claim-conflict-resolver.js create mode 100644 packages/gittensory-miner/lib/pr-number-parse.d.ts create mode 100644 packages/gittensory-miner/lib/pr-number-parse.js create mode 100644 test/unit/miner-claim-conflict-resolver.test.ts create mode 100644 test/unit/miner-pr-number-parse.test.ts diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index 2b722f2947..8bcca5b1de 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -349,6 +349,7 @@ export { export { LOCAL_WRITE_BOUNDARY, buildApplyLabelsSpec, + buildClosePrSpec, buildCreateBranchSpec, buildDeleteBranchSpec, buildFileIssueSpec, diff --git a/packages/gittensory-engine/src/miner/local-write-tools.ts b/packages/gittensory-engine/src/miner/local-write-tools.ts index 7ace795919..490e2d5293 100644 --- a/packages/gittensory-engine/src/miner/local-write-tools.ts +++ b/packages/gittensory-engine/src/miner/local-write-tools.ts @@ -44,6 +44,23 @@ export function buildOpenPrSpec(input: { repoFullName: string; base: string; hea return spec("open_pr", "Open a pull request from your local branch.", { repoFullName: input.repoFullName, base: input.base, head: input.head, title: input.title, body: input.body, draft }, command); } +/** Close a pull request the miner itself opened (e.g. it lost a claim-conflict adjudication to an earlier + * claimant, #4848) -- never used against a PR the miner does not own. `comment`, when supplied, is posted + * before the close via a separate `gh pr comment` so the reason survives on the PR even though `gh pr close` + * itself has no comment-body flag. */ +export function buildClosePrSpec(input: { repoFullName: string; number: number; comment?: string | undefined }): LocalWriteActionSpec { + const closeCommand = `gh pr close ${input.number} --repo ${sq(input.repoFullName)}`; + const command = input.comment + ? `gh pr comment ${input.number} --repo ${sq(input.repoFullName)} --body ${sq(input.comment)} && ${closeCommand}` + : closeCommand; + return spec( + "close_pr", + "Close a pull request you opened.", + { repoFullName: input.repoFullName, number: input.number, ...(input.comment ? { comment: input.comment } : {}) }, + command, + ); +} + /** File an issue (e.g. an issue-discovery proposal). */ export function buildFileIssueSpec(input: { repoFullName: string; title: string; body: string; labels?: string[] | undefined }): LocalWriteActionSpec { const labels = input.labels ?? []; diff --git a/packages/gittensory-miner/lib/attempt-cli.d.ts b/packages/gittensory-miner/lib/attempt-cli.d.ts index e348813ed8..3242fd7c43 100644 --- a/packages/gittensory-miner/lib/attempt-cli.d.ts +++ b/packages/gittensory-miner/lib/attempt-cli.d.ts @@ -12,6 +12,7 @@ import type { buildCodingTaskSpec } from "./coding-task-spec.js"; 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"; type CommonAttemptResultFields = { repoFullName: string; @@ -45,6 +46,7 @@ export type AttemptCliResult = decision?: unknown; spec?: LocalWriteActionSpec; execResult?: unknown; + claimConflict?: ClaimConflictResult; }); export type ParsedAttemptArgs = @@ -79,6 +81,7 @@ export type RunAttemptOptions = { checkMinerKillSwitch?: typeof checkMinerKillSwitch; resolveMinerGoalSpec?: typeof resolveMinerGoalSpec; runMinerAttempt?: typeof runMinerAttempt; + resolveClaimConflict?: typeof resolveClaimConflict; /** 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. */ onResult?: (result: AttemptCliResult) => void; diff --git a/packages/gittensory-miner/lib/attempt-cli.js b/packages/gittensory-miner/lib/attempt-cli.js index 1a77f6240a..986705074e 100644 --- a/packages/gittensory-miner/lib/attempt-cli.js +++ b/packages/gittensory-miner/lib/attempt-cli.js @@ -3,8 +3,10 @@ // (worktree-allocator.js + attempt-worktree.js), the four ledgers (claim/event/attempt-log/governor), the // real coding-agent driver (#5131) and slop assessor (#5133), a live SelfReviewContext fetch (#5145), a real // coding-task spec (#5239), the operator's AmsPolicySpec execution policy (#5249), rejectionSignaled (#5241), -// and finally a real runMinerAttempt call -- the first point in this epic where a real coding agent actually -// runs, not just checks-and-reports-blocked. +// a real runMinerAttempt call -- the first point in this epic where a real coding agent actually runs, not +// just checks-and-reports-blocked -- and, only on a real "submitted" outcome, a real post-submission +// claim-conflict resolution (#4848, claim-conflict-resolver.js) for the narrow race window +// checkSubmissionFreshness cannot see (two miners submitting almost simultaneously). // // KNOWN, DOCUMENTED GAPS (not fabricated -- see attempt-input-builder.js's own header for the full list): // governor.convergenceInput is an honest first-attempt-shaped literal, not a real per-issue attempt-history @@ -17,6 +19,8 @@ import { fetchLiveIssueSnapshot } from "./live-issue-snapshot.js"; import { executeLocalWrite } from "./execute-local-write.js"; import { openClaimLedger } from "./claim-ledger.js"; import { resolveMinerGoalSpec } from "./miner-goal-spec.js"; +import { resolveClaimConflict } from "./claim-conflict-resolver.js"; +import { parsePrNumberFromExecResult } from "./pr-number-parse.js"; import { initEventLedger } from "./event-ledger.js"; import { initAttemptLog } from "./attempt-log.js"; import { initGovernorLedger } from "./governor-ledger.js"; @@ -362,8 +366,9 @@ export async function runAttempt(args, options = {}) { // Real soft-claim (#5393): recorded once we've committed to a real attempt (past feasibility), so a // sibling miner process on this machine sees it via claimLedger.listClaims/listActiveClaims while this // attempt is in flight. Released in `finally` on every terminal outcome -- mirrors the worktree - // allocation slot's own acquire-then-always-release pattern below. - claimLedger.claimIssue(parsed.repoFullName, parsed.issueNumber, `attempt:${attemptId}`); + // allocation slot's own acquire-then-always-release pattern below. The real claimedAt this returns is + // ALSO this miner's own claim-time for the post-submission conflict check further down (#4848). + const claimRecord = claimLedger.claimIssue(parsed.repoFullName, parsed.issueNumber, `attempt:${attemptId}`); claimedIssue = true; const runAttemptPipeline = options.runMinerAttempt ?? runMinerAttempt; @@ -382,6 +387,30 @@ export async function runAttempt(args, options = {}) { ); worktreeResult.attemptOk = result.outcome === "submitted"; + + // Real claim-conflict resolution (#4848): only meaningful once a real PR exists, so this only ever runs + // on a real "submitted" outcome. checkSubmissionFreshness (inside runMinerAttempt) already caught the + // common pre-submission case; this closes the narrower TOCTOU window where two miners raced past that + // check almost simultaneously -- see claim-conflict-resolver.js's own header for why the adjudicator + // can only run POST-submission (it needs a real PR number on both sides of the election). + let claimConflict; + if (result.outcome === "submitted") { + const selfPrNumber = parsePrNumberFromExecResult(result.execResult, parsed.repoFullName); + if (selfPrNumber !== null) { + const resolveConflict = options.resolveClaimConflict ?? resolveClaimConflict; + claimConflict = await resolveConflict( + { + repoFullName: parsed.repoFullName, + issueNumber: parsed.issueNumber, + selfPrNumber, + selfClaimedAt: claimRecord.claimedAt, + minerLogin: parsed.minerLogin, + }, + { fetchLiveIssueSnapshot: deps.fetchLiveIssueSnapshot, executeLocalWrite: deps.executeLocalWrite }, + ); + } + } + const finalResult = { outcome: `attempt_${result.outcome}`, repoFullName: parsed.repoFullName, @@ -404,6 +433,10 @@ export async function runAttempt(args, options = {}) { ...("decision" in result ? { decision: result.decision } : {}), ...("spec" in result ? { spec: result.spec } : {}), ...("execResult" in result ? { execResult: result.execResult } : {}), + // Present only on a real "submitted" outcome whose PR number was recoverable from execResult -- omitted + // (not fabricated as "checked: false") on every other outcome, and on a submitted outcome where the new + // PR's number genuinely couldn't be parsed (an honest gap, not silently swallowed). + ...(claimConflict !== undefined ? { claimConflict } : {}), }; if (parsed.json) { diff --git a/packages/gittensory-miner/lib/claim-conflict-resolver.d.ts b/packages/gittensory-miner/lib/claim-conflict-resolver.d.ts new file mode 100644 index 0000000000..5530cede2f --- /dev/null +++ b/packages/gittensory-miner/lib/claim-conflict-resolver.d.ts @@ -0,0 +1,29 @@ +import type { LiveIssueSnapshot } from "./submission-freshness-check.js"; +import type { ObservedClaim } from "./claim-adjudication.js"; +import type { LocalWriteActionSpec } from "@jsonbored/gittensory-engine"; + +export function assembleCompetingClaims( + snapshot: LiveIssueSnapshot | null | undefined, + selfPrNumber: number, + minerLogin: string, +): ObservedClaim[]; + +export type ClaimConflictInput = { + repoFullName: string; + issueNumber: number; + selfPrNumber: number; + selfClaimedAt: string | null; + minerLogin: string; +}; + +export type ClaimConflictDeps = { + fetchLiveIssueSnapshot: (repoFullName: string, issueNumber: number) => Promise; + executeLocalWrite: (spec: LocalWriteActionSpec) => Promise; +}; + +export type ClaimConflictResult = + | { checked: false; reason: "live_state_unavailable" } + | { checked: true; isWinner: true; winnerNumber: number | null; competingCount: number } + | { checked: true; isWinner: false; winnerNumber: number | null; competingCount: number; closeResult: unknown }; + +export function resolveClaimConflict(input: ClaimConflictInput, deps: ClaimConflictDeps): Promise; diff --git a/packages/gittensory-miner/lib/claim-conflict-resolver.js b/packages/gittensory-miner/lib/claim-conflict-resolver.js new file mode 100644 index 0000000000..21d0d5a50c --- /dev/null +++ b/packages/gittensory-miner/lib/claim-conflict-resolver.js @@ -0,0 +1,97 @@ +// Real claim-conflict resolution (#4848): the missing piece over claim-adjudication.js's own adjudicator, +// which is correct and well-tested in isolation but has no caller that assembles a REAL competing-claims set. +// checkSubmissionFreshness (submission-freshness-check.js) already catches the common case pre-submission -- +// aborting before open_pr if another author's PR already references the issue -- but that check can only see +// what's PUBLIC at the moment it runs. Two miners racing closely enough that BOTH pass their own freshness +// check before either's PR exists yet is a genuine TOCTOU window freshness cannot close. This module is the +// POST-submission reconciliation for exactly that window: once THIS miner's PR is real and public, check +// whether ANOTHER open PR also claims the same issue and, if this miner's claim loses the election, close its +// own just-opened PR (never anyone else's) -- the write action the contributor-vs-maintainer safety framework +// keeps maintainer-only (#4833's own scope note), since it means the autonomous loop acts on a race-resolution +// decision with no human review. +// +// CLAIM-TIME ASYMMETRY (documented, not accidental): `self`'s claimedAt is the miner's OWN real local +// claim-ledger timestamp (claim-ledger.js, recorded before work even started). A competing PR's claimedAt uses +// its real GitHub `createdAt` instead -- the maintainer gate's own duplicate-winner election uses gittensory +// server's "first observed this PR's linked-issue set" timestamp, but that requires a continuous, persistent +// observation history this stateless client-side tool does not have for a PR it doesn't own. `createdAt` is +// the best real, publicly-observable proxy available for someone else's PR -- live-issue-snapshot.js's own +// comment on `createdAt` explains this in more detail. +// +// EVENTUAL CONSISTENCY: this checks GitHub's live state immediately after submission. A competing PR that +// exists but hasn't yet propagated through GitHub's own search/GraphQL indexing in that instant would be +// invisible to this one-shot check -- there is no retry/backoff here, which would be its own separate scope. + +import { adjudicateSoftClaim } from "./claim-adjudication.js"; +import { buildClosePrSpec } from "@jsonbored/gittensory-engine"; + +/** + * Assemble the real competing-claims set from a fetched LiveIssueSnapshot: every OTHER open PR referencing + * the issue, excluding `selfPrNumber` and any PR authored by `minerLogin` itself (case-insensitive, mirrors + * checkSubmissionFreshness's own author comparison -- a login can be echoed back with different casing). + * Pure given its inputs. + * + * @param {import("./submission-freshness-check.js").LiveIssueSnapshot | null | undefined} snapshot + * @param {number} selfPrNumber + * @param {string} minerLogin + * @returns {import("./claim-adjudication.js").ObservedClaim[]} + */ +export function assembleCompetingClaims(snapshot, selfPrNumber, minerLogin) { + const minerLoginKey = minerLogin.trim().toLowerCase(); + const referencingPrs = Array.isArray(snapshot?.referencingPrs) ? snapshot.referencingPrs : []; + return referencingPrs + .filter((pr) => pr.state === "open" && pr.number !== selfPrNumber) + .filter((pr) => typeof pr.authorLogin !== "string" || pr.authorLogin.trim().toLowerCase() !== minerLoginKey) + .map((pr) => ({ number: pr.number, claimedAt: pr.createdAt ?? null })); +} + +/** + * Resolve a real claim conflict for an already-submitted PR. Fails OPEN (never closes anything) when the live + * snapshot can't be fetched -- an unavailable check is not evidence of a lost claim. + * + * @param {{ repoFullName: string, issueNumber: number, selfPrNumber: number, selfClaimedAt: string | null, minerLogin: string }} input + * @param {{ + * fetchLiveIssueSnapshot: (repoFullName: string, issueNumber: number) => Promise, + * executeLocalWrite: (spec: import("@jsonbored/gittensory-engine").LocalWriteActionSpec) => Promise, + * }} deps + * @returns {Promise<{ + * checked: boolean, + * reason?: "live_state_unavailable", + * isWinner?: boolean, + * winnerNumber?: number | null, + * competingCount?: number, + * closeResult?: unknown, + * }>} + */ +export async function resolveClaimConflict(input, deps) { + let snapshot; + try { + snapshot = await deps.fetchLiveIssueSnapshot(input.repoFullName, input.issueNumber); + } catch { + snapshot = null; + } + if (!snapshot || typeof snapshot !== "object") { + return { checked: false, reason: "live_state_unavailable" }; + } + + const competing = assembleCompetingClaims(snapshot, input.selfPrNumber, input.minerLogin); + const adjudication = adjudicateSoftClaim({ number: input.selfPrNumber, claimedAt: input.selfClaimedAt }, competing); + + if (adjudication.isWinner) { + return { checked: true, isWinner: true, winnerNumber: adjudication.winnerNumber, competingCount: competing.length }; + } + + const comment = adjudication.winnerNumber + ? `Closing this PR: pull request #${adjudication.winnerNumber} claimed this issue first. This is an automated soft-claim conflict resolution -- no action needed from you.` + : `Closing this PR: another open pull request already claims this issue. This is an automated soft-claim conflict resolution -- no action needed from you.`; + const spec = buildClosePrSpec({ repoFullName: input.repoFullName, number: input.selfPrNumber, comment }); + const closeResult = await deps.executeLocalWrite(spec); + + return { + checked: true, + isWinner: false, + winnerNumber: adjudication.winnerNumber, + competingCount: competing.length, + closeResult, + }; +} diff --git a/packages/gittensory-miner/lib/live-issue-snapshot.js b/packages/gittensory-miner/lib/live-issue-snapshot.js index c3d38a7f73..1ecdc626dc 100644 --- a/packages/gittensory-miner/lib/live-issue-snapshot.js +++ b/packages/gittensory-miner/lib/live-issue-snapshot.js @@ -21,6 +21,7 @@ const LIVE_ISSUE_SNAPSHOT_QUERY = ` number state author { login } + createdAt } } } @@ -50,7 +51,14 @@ function normalizeReferencingPr(node) { const state = normalizeIssueOrPrState(node.state); if (state !== "open" && state !== "closed" && state !== "merged") return null; const authorLogin = typeof node.author?.login === "string" ? node.author.login : ""; - return { number: node.number, state, authorLogin }; + // GitHub's real PR creation timestamp (ISO 8601), when present -- null otherwise (never fabricated). Not + // an ordering signal for the maintainer gate's own duplicate-cluster election (duplicate-winner.ts's own + // doc explains why: a PR can be backdated by editing an old placeholder to add the linked issue later), but + // it's the only real, publicly-observable claim-time proxy claim-conflict-resolver.js's own client-side + // caller has for a THIRD-PARTY PR -- unlike gittensory's own server, the miner has no continuous observation + // history to derive a true "first linked" timestamp from. + const createdAt = typeof node.createdAt === "string" ? node.createdAt : null; + return { number: node.number, state, authorLogin, createdAt }; } function parseRepoFullName(repoFullName) { diff --git a/packages/gittensory-miner/lib/loop-cli.js b/packages/gittensory-miner/lib/loop-cli.js index 8d9fdcc418..8830b17358 100644 --- a/packages/gittensory-miner/lib/loop-cli.js +++ b/packages/gittensory-miner/lib/loop-cli.js @@ -40,6 +40,7 @@ import { pollCheckRuns } from "./ci-poller.js"; import { recordPrOutcomeSnapshot } from "./pr-outcome.js"; import { buildLoopClosureSummary } from "./loop-closure.js"; import { attemptLoopReentry } from "./loop-reentry.js"; +import { parsePrNumberFromExecResult } from "./pr-number-parse.js"; import { DEFAULT_AMS_POLICY_SPEC } from "@jsonbored/gittensory-engine"; const LOOP_USAGE = @@ -150,20 +151,6 @@ function parseIssueNumberFromIdentifier(identifier) { return match ? Number(match[1]) : null; } -/** `gh pr create` (local-write-tools.ts's `buildOpenPrSpec` -- no `--json` flag) prints the created PR's own - * URL to stdout on success; this is `gh`'s real, documented, stable CLI behavior, not an invented contract. - * Scoped to the exact target repo so an unrelated URL elsewhere in stdout/stderr noise can never match. */ -function parsePrNumberFromExecResult(execResult, repoFullName) { - if (!execResult || execResult.timedOut || execResult.code !== 0 || typeof execResult.stdout !== "string") { - return null; - } - const escapedRepo = repoFullName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const match = execResult.stdout.match(new RegExp(`github\\.com/${escapedRepo}/pull/(\\d+)`)); - if (!match) return null; - const prNumber = Number(match[1]); - return Number.isInteger(prNumber) && prNumber > 0 ? prNumber : null; -} - function convergenceKey(repoFullName, identifier) { return `${repoFullName}:${identifier}`; } diff --git a/packages/gittensory-miner/lib/pr-number-parse.d.ts b/packages/gittensory-miner/lib/pr-number-parse.d.ts new file mode 100644 index 0000000000..c8bada8f68 --- /dev/null +++ b/packages/gittensory-miner/lib/pr-number-parse.d.ts @@ -0,0 +1,4 @@ +export function parsePrNumberFromExecResult( + execResult: { stdout?: string | undefined; code?: number | null | undefined; timedOut?: boolean | undefined } | null | undefined, + repoFullName: string, +): number | null; diff --git a/packages/gittensory-miner/lib/pr-number-parse.js b/packages/gittensory-miner/lib/pr-number-parse.js new file mode 100644 index 0000000000..bf6884b4c7 --- /dev/null +++ b/packages/gittensory-miner/lib/pr-number-parse.js @@ -0,0 +1,23 @@ +// Shared PR-number extraction from a real `gh pr create` executeLocalWrite result (#4848). `gh pr create` +// prints the new PR's URL to stdout on success -- this is the one place that URL is authoritatively parsed, +// so loop-cli.js's CI/gate-status polling and attempt-cli.js's post-submission claim-conflict check agree on +// exactly how a PR number is recovered from a real command's raw output. + +/** `gh pr create` (local-write-tools.ts's `buildOpenPrSpec` -- no `--json` flag) prints the created PR's own + * URL to stdout on success; this is `gh`'s real, documented, stable CLI behavior, not an invented contract. + * Scoped to the exact target repo so an unrelated URL elsewhere in stdout/stderr noise can never match. + * + * @param {{ stdout?: string, code?: number | null, timedOut?: boolean } | null | undefined} execResult + * @param {string} repoFullName + * @returns {number | null} + */ +export function parsePrNumberFromExecResult(execResult, repoFullName) { + if (!execResult || execResult.timedOut || execResult.code !== 0 || typeof execResult.stdout !== "string") { + return null; + } + const escapedRepo = repoFullName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const match = execResult.stdout.match(new RegExp(`github\\.com/${escapedRepo}/pull/(\\d+)`)); + if (!match) return null; + const prNumber = Number(match[1]); + return Number.isInteger(prNumber) && prNumber > 0 ? prNumber : null; +} diff --git a/packages/gittensory-miner/lib/submission-freshness-check.d.ts b/packages/gittensory-miner/lib/submission-freshness-check.d.ts index 7256602944..e215c0c149 100644 --- a/packages/gittensory-miner/lib/submission-freshness-check.d.ts +++ b/packages/gittensory-miner/lib/submission-freshness-check.d.ts @@ -10,7 +10,7 @@ export type SubmissionFreshnessCandidate = { export type LiveIssueSnapshot = { state: "open" | "closed"; - referencingPrs: Array<{ number: number; state: "open" | "closed" | "merged"; authorLogin: string }>; + referencingPrs: Array<{ number: number; state: "open" | "closed" | "merged"; authorLogin: string; createdAt: string | null }>; }; export type SubmissionFreshnessClaimLedger = { diff --git a/packages/gittensory-miner/package.json b/packages/gittensory-miner/package.json index c22d5b2c49..8664dcea97 100644 --- a/packages/gittensory-miner/package.json +++ b/packages/gittensory-miner/package.json @@ -33,7 +33,7 @@ "expected-engine.version" ], "scripts": { - "build": "node --check bin/gittensory-miner.js && node --check bin/gittensory-miner-mcp.js && node --check lib/ams-policy.js && node --check lib/attempt-cli.js && node --check lib/attempt-input-builder.js && node --check lib/attempt-log.js && node --check lib/attempt-runner.js && node --check lib/attempt-worktree.js && node --check lib/calibration-run.js && node --check lib/calibration-types.js && node --check lib/calibration.js && node --check lib/ci-poller.js && node --check lib/claim-adjudication.js && node --check lib/claim-ledger-cli.js && node --check lib/claim-ledger-expiry.js && node --check lib/claim-ledger.js && node --check lib/cli.js && node --check lib/coding-agent-construction.js && node --check lib/coding-agent-house-rules.js && node --check lib/coding-task-spec.js && node --check lib/deny-check.js && node --check lib/deny-hook-synthesis.js && node --check lib/deny-hooks.js && node --check lib/deployment-docs-audit.js && node --check lib/discover-cli.js && node --check lib/event-ledger-cli.js && node --check lib/event-ledger.js && node --check lib/execute-local-write.js && node --check lib/feasibility-cli.js && node --check lib/gate-verdict-poller.js && node --check lib/governor-action-mode.js && node --check lib/governor-chokepoint-persisted.js && node --check lib/governor-chokepoint.js && node --check lib/governor-kill-switch.js && node --check lib/governor-ledger-cli.js && node --check lib/governor-ledger.js && node --check lib/governor-open-pr.js && node --check lib/governor-run-halt.js && node --check lib/governor-state.js && node --check lib/governor-write-rate-limit.js && node --check lib/harness-submission-trigger.js && node --check lib/laptop-init.js && node --check lib/live-issue-snapshot.js && node --check lib/local-store.js && node --check lib/loop-cli.js && node --check lib/loop-closure.js && node --check lib/loop-reentry.js && node --check lib/manage-poll.js && node --check lib/manage-status.js && node --check lib/metrics-cli.js && node --check lib/miner-goal-spec.js && node --check lib/opportunity-fanout.js && node --check lib/opportunity-ranker.js && node --check lib/orb-export.js && node --check lib/plan-store-cli.js && node --check lib/plan-store.js && node --check lib/portfolio-dashboard.js && node --check lib/portfolio-discovery.js && node --check lib/portfolio-queue-cli.js && node --check lib/portfolio-queue-manager.js && node --check lib/portfolio-queue.js && node --check lib/portfolio-queue-expiry.js && node --check lib/pr-disposition-poller.js && node --check lib/pr-outcome.js && node --check lib/prediction-ledger.js && node --check lib/pretooluse-hook.js && node --check lib/rejection-signal.js && node --check lib/rejection-state-machine.js && node --check lib/rejection-templates.js && node --check lib/replay-objective-anchor.js && node --check lib/replay-snapshot.js && node --check lib/replay-task-generation.js && node --check lib/repo-clone.js && node --check lib/run-state-cli.js && node --check lib/run-state.js && node --check lib/self-review-context.js && node --check lib/slop-assessment.js && node --check lib/stack-detection.js && node --check lib/status.js && node --check lib/submission-freshness-check.js && node --check lib/update-check.js && node --check lib/version.js && node --check lib/worktree-allocator.js" + "build": "node --check bin/gittensory-miner.js && node --check bin/gittensory-miner-mcp.js && node --check lib/ams-policy.js && node --check lib/attempt-cli.js && node --check lib/attempt-input-builder.js && node --check lib/attempt-log.js && node --check lib/attempt-runner.js && node --check lib/attempt-worktree.js && node --check lib/calibration-run.js && node --check lib/calibration-types.js && node --check lib/calibration.js && node --check lib/ci-poller.js && node --check lib/claim-adjudication.js && node --check lib/claim-conflict-resolver.js && node --check lib/claim-ledger-cli.js && node --check lib/claim-ledger-expiry.js && node --check lib/claim-ledger.js && node --check lib/cli.js && node --check lib/coding-agent-construction.js && node --check lib/coding-agent-house-rules.js && node --check lib/coding-task-spec.js && node --check lib/deny-check.js && node --check lib/deny-hook-synthesis.js && node --check lib/deny-hooks.js && node --check lib/deployment-docs-audit.js && node --check lib/discover-cli.js && node --check lib/event-ledger-cli.js && node --check lib/event-ledger.js && node --check lib/execute-local-write.js && node --check lib/feasibility-cli.js && node --check lib/gate-verdict-poller.js && node --check lib/governor-action-mode.js && node --check lib/governor-chokepoint-persisted.js && node --check lib/governor-chokepoint.js && node --check lib/governor-kill-switch.js && node --check lib/governor-ledger-cli.js && node --check lib/governor-ledger.js && node --check lib/governor-open-pr.js && node --check lib/governor-run-halt.js && node --check lib/governor-state.js && node --check lib/governor-write-rate-limit.js && node --check lib/harness-submission-trigger.js && node --check lib/laptop-init.js && node --check lib/live-issue-snapshot.js && node --check lib/local-store.js && node --check lib/loop-cli.js && node --check lib/loop-closure.js && node --check lib/loop-reentry.js && node --check lib/manage-poll.js && node --check lib/manage-status.js && node --check lib/metrics-cli.js && node --check lib/miner-goal-spec.js && node --check lib/opportunity-fanout.js && node --check lib/opportunity-ranker.js && node --check lib/orb-export.js && node --check lib/plan-store-cli.js && node --check lib/plan-store.js && node --check lib/portfolio-dashboard.js && node --check lib/portfolio-discovery.js && node --check lib/portfolio-queue-cli.js && node --check lib/portfolio-queue-manager.js && node --check lib/portfolio-queue.js && node --check lib/portfolio-queue-expiry.js && node --check lib/pr-disposition-poller.js && node --check lib/pr-number-parse.js && node --check lib/pr-outcome.js && node --check lib/prediction-ledger.js && node --check lib/pretooluse-hook.js && node --check lib/rejection-signal.js && node --check lib/rejection-state-machine.js && node --check lib/rejection-templates.js && node --check lib/replay-objective-anchor.js && node --check lib/replay-snapshot.js && node --check lib/replay-task-generation.js && node --check lib/repo-clone.js && node --check lib/run-state-cli.js && node --check lib/run-state.js && node --check lib/self-review-context.js && node --check lib/slop-assessment.js && node --check lib/stack-detection.js && node --check lib/status.js && node --check lib/submission-freshness-check.js && node --check lib/update-check.js && node --check lib/version.js && node --check lib/worktree-allocator.js" }, "dependencies": { "@jsonbored/gittensory-engine": "*", diff --git a/src/mcp/local-write-tools.ts b/src/mcp/local-write-tools.ts index e1c77dda01..bfd02f215d 100644 --- a/src/mcp/local-write-tools.ts +++ b/src/mcp/local-write-tools.ts @@ -8,6 +8,7 @@ export { LOCAL_WRITE_BOUNDARY, buildApplyLabelsSpec, + buildClosePrSpec, buildCreateBranchSpec, buildDeleteBranchSpec, buildFileIssueSpec, diff --git a/test/unit/local-write-tools.test.ts b/test/unit/local-write-tools.test.ts index 5f4677b747..1766e44c0b 100644 --- a/test/unit/local-write-tools.test.ts +++ b/test/unit/local-write-tools.test.ts @@ -12,6 +12,7 @@ vi.mock("@jsonbored/gittensory-engine", async () => { import { LOCAL_WRITE_BOUNDARY, buildApplyLabelsSpec, + buildClosePrSpec, buildCreateBranchSpec, buildDeleteBranchSpec, buildFileIssueSpec, @@ -36,6 +37,20 @@ describe("local write-tool specs (#780)", () => { expect(s.command.endsWith("--draft")).toBe(true); }); + it("close_pr closes with a preceding comment when one is supplied", () => { + const s = buildClosePrSpec({ repoFullName: "o/r", number: 7, comment: "Closing: lost the claim to #5" }); + expect(s.action).toBe("close_pr"); + expect(s.command).toBe("gh pr comment 7 --repo 'o/r' --body 'Closing: lost the claim to #5' && gh pr close 7 --repo 'o/r'"); + expect(s.boundary).toBe(LOCAL_WRITE_BOUNDARY); + expect(s.inputs).toEqual({ repoFullName: "o/r", number: 7, comment: "Closing: lost the claim to #5" }); + }); + + it("close_pr omits the comment step entirely when no comment is supplied", () => { + const s = buildClosePrSpec({ repoFullName: "o/r", number: 7 }); + expect(s.command).toBe("gh pr close 7 --repo 'o/r'"); + expect(s.inputs).toEqual({ repoFullName: "o/r", number: 7 }); + }); + it("file_issue includes each label as a --label arg, and omits them when none", () => { expect(buildFileIssueSpec({ repoFullName: "o/r", title: "T", body: "B", labels: ["bug", "good first issue"] }).command).toBe( "gh issue create --repo 'o/r' --title 'T' --body 'B' --label 'bug' --label 'good first issue'", diff --git a/test/unit/miner-attempt-cli.test.ts b/test/unit/miner-attempt-cli.test.ts index 25cc8fbd74..08f74617d5 100644 --- a/test/unit/miner-attempt-cli.test.ts +++ b/test/unit/miner-attempt-cli.test.ts @@ -307,6 +307,155 @@ describe("runAttempt (#5132)", () => { expect(typeof deps.driver.run).toBe("function"); }); + it("REGRESSION (#4848): a real submitted outcome with a recoverable PR number runs the real claim-conflict check and surfaces its result", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + 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/123\n", stderr: "", timedOut: false }, + loopResult: { outcome: "handoff", totalTurnsUsed: 3, totalCostUsd: 0.42, iterationsUsed: 2 }, + }); + const resolveClaimConflictSpy = vi.fn().mockResolvedValue({ checked: true, isWinner: true, winnerNumber: 123, competingCount: 0 }); + + const exitCode = await runAttempt(["acme/widgets", "7", "--miner-login", "alice", "--json"], { + env: { MINER_CODING_AGENT_PROVIDER: "noop" }, + attemptId: "conflict-attempt", + openWorktreeAllocator: () => allocator, + openClaimLedger: () => claimLedger, + initEventLedger: () => eventLedger, + initAttemptLog: () => attemptLog, + initGovernorLedger: () => governorLedger, + ...readyPipelineOptions({ runMinerAttempt: runMinerAttemptSpy }), + resolveClaimConflict: resolveClaimConflictSpy, + }); + + expect(exitCode).toBe(0); + expect(resolveClaimConflictSpy).toHaveBeenCalledTimes(1); + const [input, deps] = resolveClaimConflictSpy.mock.calls[0]!; + expect(input).toMatchObject({ + repoFullName: "acme/widgets", + issueNumber: 7, + selfPrNumber: 123, + minerLogin: "alice", + }); + expect(typeof input.selfClaimedAt).toBe("string"); // the real claim-ledger record's own claimedAt + expect(typeof deps.fetchLiveIssueSnapshot).toBe("function"); + expect(typeof deps.executeLocalWrite).toBe("function"); + }); + + it("REGRESSION: uses the REAL default resolveClaimConflict (not just an injected override) when options.resolveClaimConflict is omitted", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const fetchLiveIssueSnapshot = vi.fn().mockResolvedValue({ state: "open" as const, referencingPrs: [] }); + const executeLocalWrite = vi.fn(); + const buildAttemptDepsSpy = vi.fn((env: Record, ledgers: unknown) => ({ + ...buildAttemptDeps(env, ledgers as never), + fetchLiveIssueSnapshot, + executeLocalWrite, + })); + + 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({ + buildAttemptDeps: buildAttemptDepsSpy, + runMinerAttempt: async () => ({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0, stdout: "https://github.com/acme/widgets/pull/9\n" }, + loopResult: {}, + }), + }), + // resolveClaimConflict deliberately omitted -- exercises the real module-level default. + }); + + expect(fetchLiveIssueSnapshot).toHaveBeenCalledWith("acme/widgets", 7); + expect(executeLocalWrite).not.toHaveBeenCalled(); // no competing claims -> trivial win, no close_pr write + expect(JSON.parse(String(log.mock.calls[0]?.[0])).claimConflict).toEqual({ + checked: true, + isWinner: true, + winnerNumber: 9, + competingCount: 0, + }); + }); + + it("does not run the claim-conflict check on a non-submitted outcome", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + vi.spyOn(console, "log").mockImplementation(() => undefined); + const resolveClaimConflictSpy = vi.fn(); + + 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({ runMinerAttempt: async () => ({ outcome: "abandon", loopResult: {} }) }), + resolveClaimConflict: resolveClaimConflictSpy, + }); + + expect(resolveClaimConflictSpy).not.toHaveBeenCalled(); + }); + + it("does not run the claim-conflict check on a submitted outcome whose PR number can't be recovered from execResult", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const resolveClaimConflictSpy = vi.fn(); + + 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({ + runMinerAttempt: async () => ({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0 }, + loopResult: {}, + }), + }), + resolveClaimConflict: resolveClaimConflictSpy, + }); + + expect(resolveClaimConflictSpy).not.toHaveBeenCalled(); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).not.toHaveProperty("claimConflict"); + }); + + it("REGRESSION: a real claim-conflict LOSS is surfaced verbatim in the final JSON result", async () => { + const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const lossResult = { checked: true as const, isWinner: false as const, winnerNumber: 5, competingCount: 1, closeResult: { action: "close_pr", code: 0 } }; + + 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({ + runMinerAttempt: async () => ({ + outcome: "submitted", + spec: { command: "gh pr create", cwd: "/fake", timeoutMs: 1000 }, + execResult: { code: 0, stdout: "https://github.com/acme/widgets/pull/6\n" }, + loopResult: {}, + }), + }), + resolveClaimConflict: async () => lossResult, + }); + + expect(JSON.parse(String(log.mock.calls[0]?.[0])).claimConflict).toEqual(lossResult); + }); + it("resolves live mode only when --live is passed, and threads it through to the real loopInput", async () => { const { allocator, claimLedger, eventLedger, attemptLog, governorLedger } = tempLedgers(); const log = vi.spyOn(console, "log").mockImplementation(() => undefined); diff --git a/test/unit/miner-claim-conflict-resolver.test.ts b/test/unit/miner-claim-conflict-resolver.test.ts new file mode 100644 index 0000000000..a5c088e903 --- /dev/null +++ b/test/unit/miner-claim-conflict-resolver.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@jsonbored/gittensory-engine", async () => { + return import("../../packages/gittensory-engine/src/index"); +}); + +import { assembleCompetingClaims, resolveClaimConflict } from "../../packages/gittensory-miner/lib/claim-conflict-resolver.js"; + +function snapshot(referencingPrs: Array<{ number: number; state: "open" | "closed" | "merged"; authorLogin: string; createdAt: string | null }>) { + return { state: "open" as const, referencingPrs }; +} + +describe("assembleCompetingClaims (#4848)", () => { + it("keeps only OTHER open PRs, mapping createdAt -> claimedAt", () => { + const competing = assembleCompetingClaims( + snapshot([ + { number: 5, state: "open", authorLogin: "alice", createdAt: "2026-01-01T00:00:00Z" }, + { number: 6, state: "closed", authorLogin: "bob", createdAt: "2026-01-02T00:00:00Z" }, + ]), + 7, + "miner-bot", + ); + expect(competing).toEqual([{ number: 5, claimedAt: "2026-01-01T00:00:00Z" }]); + }); + + it("excludes self by PR number even if it somehow appears in the snapshot", () => { + const competing = assembleCompetingClaims( + snapshot([{ number: 7, state: "open", authorLogin: "miner-bot", createdAt: "2026-01-01T00:00:00Z" }]), + 7, + "miner-bot", + ); + expect(competing).toEqual([]); + }); + + it("excludes any other open PR authored by the SAME miner login, case-insensitively", () => { + const competing = assembleCompetingClaims( + snapshot([{ number: 9, state: "open", authorLogin: "Miner-Bot", createdAt: "2026-01-01T00:00:00Z" }]), + 7, + "miner-bot", + ); + expect(competing).toEqual([]); + }); + + it("returns an empty set for a null/undefined snapshot or a missing referencingPrs array", () => { + expect(assembleCompetingClaims(null, 7, "miner-bot")).toEqual([]); + expect(assembleCompetingClaims(undefined, 7, "miner-bot")).toEqual([]); + expect(assembleCompetingClaims({ state: "open", referencingPrs: undefined as never }, 7, "miner-bot")).toEqual([]); + }); +}); + +describe("resolveClaimConflict (#4848)", () => { + it("REGRESSION: two simulated competing claims are correctly adjudicated -- this miner WINS (claimed earliest) and its PR is never touched", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => + snapshot([{ number: 6, state: "open", authorLogin: "someone-else", createdAt: "2026-01-02T00:00:00Z" }]), + ); + const executeLocalWrite = vi.fn(); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + ); + + expect(result).toEqual({ checked: true, isWinner: true, winnerNumber: 5, competingCount: 1 }); + expect(executeLocalWrite).not.toHaveBeenCalled(); + }); + + it("REGRESSION: two simulated competing claims are correctly adjudicated -- this miner LOSES and its own PR is closed with a real close_pr write citing the winner", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => + snapshot([{ number: 5, state: "open", authorLogin: "someone-else", createdAt: "2026-01-01T00:00:00Z" }]), + ); + const executeLocalWrite = vi.fn(async (spec: { action: string; command: string }) => ({ action: spec.action, code: 0, stdout: "", stderr: "", timedOut: false })); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 6, selfClaimedAt: "2026-01-02T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + ); + + expect(result.checked).toBe(true); + if (!result.checked) throw new Error("expected checked"); + expect(result.isWinner).toBe(false); + expect(result.winnerNumber).toBe(5); + expect(result.competingCount).toBe(1); + + expect(executeLocalWrite).toHaveBeenCalledTimes(1); + const [spec] = executeLocalWrite.mock.calls[0]!; + expect(spec.action).toBe("close_pr"); + expect(spec.command).toContain("gh pr close 6 --repo 'acme/widgets'"); + expect(spec.command).toContain("#5"); + expect((result as { closeResult: unknown }).closeResult).toEqual({ action: "close_pr", code: 0, stdout: "", stderr: "", timedOut: false }); + }); + + it("no competing claims at all: trivial win, no live-write dependency invoked", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => snapshot([])); + const executeLocalWrite = vi.fn(); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + ); + + expect(result).toEqual({ checked: true, isWinner: true, winnerNumber: 5, competingCount: 0 }); + expect(executeLocalWrite).not.toHaveBeenCalled(); + }); + + it("fails OPEN (never closes anything) when the live snapshot can't be fetched", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => null); + const executeLocalWrite = vi.fn(); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + ); + + expect(result).toEqual({ checked: false, reason: "live_state_unavailable" }); + expect(executeLocalWrite).not.toHaveBeenCalled(); + }); + + it("fails OPEN when the live snapshot fetch throws", async () => { + const fetchLiveIssueSnapshot = vi.fn(async () => { + throw new Error("network down"); + }); + const executeLocalWrite = vi.fn(); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 5, selfClaimedAt: "2026-01-01T00:00:00Z", minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + ); + + expect(result).toEqual({ checked: false, reason: "live_state_unavailable" }); + expect(executeLocalWrite).not.toHaveBeenCalled(); + }); + + it("builds a generic comment (no winner number) when the adjudicator can't determine a display winner", async () => { + // BOTH sides sparse (no claim time on either side) -- fail-closed: this miner loses, but the winner is + // not determinable either (mirrors miner-claim-adjudication.test.ts's own fail-closed/sparse case). + const fetchLiveIssueSnapshot = vi.fn(async () => snapshot([{ number: 5, state: "open", authorLogin: "someone-else", createdAt: null }])); + const executeLocalWrite = vi.fn(async (spec: { action: string; command: string }) => ({ action: spec.action, code: 0, stdout: "", stderr: "", timedOut: false })); + + const result = await resolveClaimConflict( + { repoFullName: "acme/widgets", issueNumber: 42, selfPrNumber: 6, selfClaimedAt: null, minerLogin: "miner-bot" }, + { fetchLiveIssueSnapshot, executeLocalWrite }, + ); + + expect(result.checked).toBe(true); + if (!result.checked) throw new Error("expected checked"); + expect(result.isWinner).toBe(false); + expect(result.winnerNumber).toBeNull(); + const [spec] = executeLocalWrite.mock.calls[0]!; + expect(spec.command).not.toContain("#null"); + expect(spec.command).toContain("another open pull request already claims this issue"); + }); +}); diff --git a/test/unit/miner-live-issue-snapshot.test.ts b/test/unit/miner-live-issue-snapshot.test.ts index 3ea7a38c6f..3e47d3f03f 100644 --- a/test/unit/miner-live-issue-snapshot.test.ts +++ b/test/unit/miner-live-issue-snapshot.test.ts @@ -33,7 +33,7 @@ describe("fetchLiveIssueSnapshot (#5132)", () => { state: "OPEN", closedByPullRequestsReferences: { nodes: [ - { number: 42, state: "MERGED", author: { login: "alice" } }, + { number: 42, state: "MERGED", author: { login: "alice" }, createdAt: "2026-01-01T00:00:00Z" }, { number: 43, state: "OPEN", author: null }, ], }, @@ -49,8 +49,8 @@ describe("fetchLiveIssueSnapshot (#5132)", () => { expect(snapshot).toEqual({ state: "open", referencingPrs: [ - { number: 42, state: "merged", authorLogin: "alice" }, - { number: 43, state: "open", authorLogin: "" }, + { number: 42, state: "merged", authorLogin: "alice", createdAt: "2026-01-01T00:00:00Z" }, + { number: 43, state: "open", authorLogin: "", createdAt: null }, ], }); expect(capturedUrl).toBe("https://api.github.com/graphql"); @@ -112,7 +112,7 @@ describe("fetchLiveIssueSnapshot (#5132)", () => { }, }), }); - expect(snapshot).toEqual({ state: "open", referencingPrs: [{ number: 9, state: "closed", authorLogin: "" }] }); + expect(snapshot).toEqual({ state: "open", referencingPrs: [{ number: 9, state: "closed", authorLogin: "", createdAt: null }] }); }); it("returns null when the response is not valid JSON or the fetch itself rejects", async () => { diff --git a/test/unit/miner-pr-number-parse.test.ts b/test/unit/miner-pr-number-parse.test.ts new file mode 100644 index 0000000000..3b0d5e8ba0 --- /dev/null +++ b/test/unit/miner-pr-number-parse.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "vitest"; +import { parsePrNumberFromExecResult } from "../../packages/gittensory-miner/lib/pr-number-parse.js"; + +describe("parsePrNumberFromExecResult (#4848)", () => { + it("extracts the real PR number from a real gh pr create stdout URL, scoped to the exact repo", () => { + expect( + parsePrNumberFromExecResult({ code: 0, stdout: "https://github.com/acme/widgets/pull/123\n", timedOut: false }, "acme/widgets"), + ).toBe(123); + }); + + it("returns null when execResult is missing, timed out, or exited non-zero", () => { + expect(parsePrNumberFromExecResult(null, "acme/widgets")).toBeNull(); + expect(parsePrNumberFromExecResult(undefined, "acme/widgets")).toBeNull(); + expect(parsePrNumberFromExecResult({ code: 0, stdout: "https://github.com/acme/widgets/pull/1", timedOut: true }, "acme/widgets")).toBeNull(); + expect(parsePrNumberFromExecResult({ code: 1, stdout: "https://github.com/acme/widgets/pull/1", timedOut: false }, "acme/widgets")).toBeNull(); + }); + + it("returns null when stdout is not a string, or has no matching URL", () => { + expect(parsePrNumberFromExecResult({ code: 0, stdout: undefined, timedOut: false }, "acme/widgets")).toBeNull(); + expect(parsePrNumberFromExecResult({ code: 0, stdout: "no url here", timedOut: false }, "acme/widgets")).toBeNull(); + }); + + it("REGRESSION: never matches a URL for a DIFFERENT repo, even if it looks similar", () => { + expect( + parsePrNumberFromExecResult({ code: 0, stdout: "https://github.com/acme/other-repo/pull/9\n", timedOut: false }, "acme/widgets"), + ).toBeNull(); + }); + + it("scoping regex-escapes the repo name so a special character can't widen the match", () => { + expect( + parsePrNumberFromExecResult({ code: 0, stdout: "https://github.com/acme/widgets/pull/9\n", timedOut: false }, "acme/widget."), + ).toBeNull(); + }); + + it("REGRESSION: a matched but non-positive number (e.g. pull/0) is rejected, not returned as-is", () => { + expect( + parsePrNumberFromExecResult({ code: 0, stdout: "https://github.com/acme/widgets/pull/0\n", timedOut: false }, "acme/widgets"), + ).toBeNull(); + }); +}); diff --git a/test/unit/miner-submission-freshness-check.test.ts b/test/unit/miner-submission-freshness-check.test.ts index de8a9b954e..5367656871 100644 --- a/test/unit/miner-submission-freshness-check.test.ts +++ b/test/unit/miner-submission-freshness-check.test.ts @@ -87,7 +87,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "someone-else" }], + referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "someone-else", createdAt: null }], })); const result = await checkSubmissionFreshness( @@ -103,7 +103,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "merged" as const, authorLogin: "someone-else" }], + referencingPrs: [{ number: 99, state: "merged" as const, authorLogin: "someone-else", createdAt: null }], })); const result = await checkSubmissionFreshness( @@ -119,7 +119,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "miner-bot" }], + referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "miner-bot", createdAt: null }], })); const result = await checkSubmissionFreshness( @@ -135,7 +135,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "Miner-Bot" }], + referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "Miner-Bot", createdAt: null }], })); const result = await checkSubmissionFreshness( @@ -151,7 +151,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "SOMEONE-ELSE" }], + referencingPrs: [{ number: 99, state: "open" as const, authorLogin: "SOMEONE-ELSE", createdAt: null }], })); const result = await checkSubmissionFreshness( @@ -167,7 +167,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "open" as const, authorLogin: undefined as unknown as string }], + referencingPrs: [{ number: 99, state: "open" as const, authorLogin: undefined as unknown as string, createdAt: null }], })); const result = await checkSubmissionFreshness( @@ -183,7 +183,7 @@ describe("checkSubmissionFreshness (#3007)", () => { const { eventLedger } = stubEventLedger(); const fetchLiveIssueSnapshot = vi.fn(async () => ({ state: "open" as const, - referencingPrs: [{ number: 99, state: "closed" as const, authorLogin: "someone-else" }], + referencingPrs: [{ number: 99, state: "closed" as const, authorLogin: "someone-else", createdAt: null }], })); const result = await checkSubmissionFreshness(