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(