From 09fb02cf15ee423753d4e559a7dc864887206d69 Mon Sep 17 00:00:00 2001 From: Matysh Date: Thu, 1 Oct 2026 19:29:19 +0300 Subject: [PATCH] fix(process): bind Validate resume to the current pending round Issue: #775 User-Visible: no --- .github/workflows/_process-resume.yml | 3 +- PROCESS.md | 10 ++ scripts/mutation-registry.mjs | 48 ++++++- scripts/process-reconcile.mjs | 85 +++++++++---- scripts/process-resume.mjs | 103 ++++++++++----- test/process-pending-round.test.mjs | 175 ++++++++++++++++++++++++++ test/process-resume.test.mjs | 13 +- 7 files changed, 374 insertions(+), 63 deletions(-) create mode 100644 test/process-pending-round.test.mjs diff --git a/.github/workflows/_process-resume.yml b/.github/workflows/_process-resume.yml index 8462221a..d887be5c 100644 --- a/.github/workflows/_process-resume.yml +++ b/.github/workflows/_process-resume.yml @@ -54,6 +54,7 @@ jobs: SHA: ${{ github.event.workflow_run.head_sha }} EVENT: ${{ github.event.workflow_run.event }} STATUS: ${{ github.event.workflow_run.status }} + VALIDATE_RUN_ID: ${{ github.event.workflow_run.id }} # #751: без pipefail код конвейера — код tee, и исключение скрипта (gh, # API) проходило зелёным шагом: событие возобновления терялось до # прохода process-reconcile. @@ -61,4 +62,4 @@ jobs: set -o pipefail node scripts/process-resume.mjs \ --repo="$REPO" --branch="$BRANCH" --sha="$SHA" \ - --event="$EVENT" --status="$STATUS" --apply=true | tee -a "$GITHUB_STEP_SUMMARY" + --event="$EVENT" --status="$STATUS" --run-id="$VALIDATE_RUN_ID" --apply=true | tee -a "$GITHUB_STEP_SUMMARY" diff --git a/PROCESS.md b/PROCESS.md index f1e8be81..7a27d4e8 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -1442,6 +1442,16 @@ Low и цикла не открывает. Отсутствие прогона весь реестр проверяет ночь (`mutation-gate.yml`, 00:43 UTC). Ни ревью, ни слияние, ни релизный гейт (#541) mutant-jobs не требуют. +**Поиск ожидающего раунда (#775).** Resume и автосверка читают историю до +последней постановки review-метки, без окна «200 последних событий» и без +лимита фильтрованного поиска Actions. Чужая разметка и полностью skipped-прогоны +не заслоняют ожидание; новый запрос метки отделяет старый раунд. Resume сверяет +issue, stage, run/attempt, ветку, её текущий SHA и точный ID Validate с запечатанным +маркером. Перед перестановкой метки состояние перечитывается. При ещё невидимом +прогоне/артефакте — не более трёх снимков с паузами по 5 секунд, далее страховка +автосверкой. Поиск не перескакивает через завершённую попытку ревью без маркера: +так нельзя доказать, что повторный вызов модели безопасен. + Ожидание gates, работа модели и публикация/интеграция — три независимых jobs (#551) с отдельными бюджетами 55, 45 и 55 минут. Поэтому долгий Validate не съедает время модели (с #636 — и не занимает раннер: до этого подготовка спала diff --git a/scripts/mutation-registry.mjs b/scripts/mutation-registry.mjs index 22d4a2fb..532e744e 100644 --- a/scripts/mutation-registry.mjs +++ b/scripts/mutation-registry.mjs @@ -10328,8 +10328,8 @@ const MUTANT_DEFINITIONS = [ + 'it would spend the model a second time (#636)', patches: [{ file: 'scripts/process-resume.mjs', - find: " if (!pending) return { action: 'noop', reason: 'latest process run left no pending marker — it did not wait for Validate' };", - replace: " if (false && !pending) return { action: 'noop', reason: 'latest process run left no pending marker — it did not wait for Validate' };", + find: " if (!pending) return { action: 'noop', recheck: true, reason: 'latest process run left no pending marker — it did not wait for Validate' };", + replace: " if (false && !pending) return { action: 'noop', recheck: true, reason: 'latest process run left no pending marker — it did not wait for Validate' };", }], }, { @@ -10338,8 +10338,48 @@ const MUTANT_DEFINITIONS = [ because: 'relabelling while a process run is active queues a second round for the same request (#636)', patches: [{ file: 'scripts/process-resume.mjs', - find: " if (mine.some((run) => ACTIVE_RUN_STATES.has(run.status))) return { action: 'noop', reason: 'a process run for this issue is already active' };", - replace: " if (false && mine.some((run) => ACTIVE_RUN_STATES.has(run.status))) return { action: 'noop', reason: 'a process run for this issue is already active' };", + find: " if (mine.some((run) => ACTIVE_RUN_STATES.has(run.status))) return { action: 'noop', recheck: true, reason: 'a process run for this issue is already active' };", + replace: " if (false && mine.some((run) => ACTIVE_RUN_STATES.has(run.status))) return { action: 'noop', recheck: true, reason: 'a process run for this issue is already active' };", + }], + }, + { + id: 'pending-history-latest-200-only', + guard: 'node --test test/process-pending-round.test.mjs', + because: '#775: unrelated label traffic must not hide the pending review beyond two pages', + patches: [{ + file: 'scripts/process-reconcile.mjs', + find: ' for (let page = 1; ; page++) {', + replace: ' for (let page = 1; page <= 2; page++) {', + }], + }, + { + id: 'pending-revives-previous-request', + guard: 'node --test test/process-pending-round.test.mjs', + because: '#775: a re-applied label starts a new request even one second after the preceding run', + patches: [{ + file: 'scripts/process-reconcile.mjs', + find: ' && at(run.createdAt) >= at(request.at))', + replace: ' && at(run.createdAt) >= at(request.at) - 120_000)', + }], + }, + { + id: 'pending-resumes-another-validate', + guard: 'node --test test/process-pending-round.test.mjs', + because: '#775: the same SHA is not proof that the pending round awaited this dispatch', + patches: [{ + file: 'scripts/process-resume.mjs', + find: ' if (pending.branch !== branch || String(pending.validate_run_id) !== String(validateRun.id)) {', + replace: ' if (pending.branch !== branch) {', + }], + }, + { + id: 'pending-no-fresh-snapshot-before-write', + guard: 'node --test test/process-pending-round.test.mjs', + because: '#775: a stopped or already restarted round must not be woken from an old snapshot', + patches: [{ + file: 'scripts/process-resume.mjs', + find: ' const second = inspect();', + replace: ' const second = first;', }], }, { diff --git a/scripts/process-reconcile.mjs b/scripts/process-reconcile.mjs index 088338b3..007a80d4 100644 --- a/scripts/process-reconcile.mjs +++ b/scripts/process-reconcile.mjs @@ -72,6 +72,32 @@ export function latestReviewRequest(events = [], label = null) { return requests.at(-1) || null; } +/** Runs belonging to this request only. Never revive a previous label round. */ +export function reviewRunsForRequest(runs, issue, request) { + if (!request || !Number.isFinite(at(request.at))) return []; + const matching = runs.map((run) => run.issue ? run : parseProcessRun(run)).filter(Boolean) + .filter((run) => run.issue === Number(issue) && run.label === request.label + && at(run.createdAt) >= at(request.at)) + .sort((a, b) => at(b.createdAt) - at(a.createdAt) || Number(b.id) - Number(a.id)); + // Fully skipped workflows ran no review. All other conclusions remain barriers: + // searching past a failed or successful non-pending run risks a second model call. + const attempted = matching.filter((run) => run.conclusion !== 'skipped'); + return attempted.length ? attempted : matching; +} + +export function pendingEvidenceError(pending, { issue, run }) { + if (pending.schema !== 1 || String(pending.issue) !== String(issue) + || String(pending.run_id) !== String(run.id) || String(pending.run_attempt) !== String(run.attempt) + || pending.stage !== run.stage || !String(pending.branch || '').startsWith(`issue/${issue}-`)) { + return 'pending marker belongs to another issue/stage/run attempt/branch'; + } + if (!/^[0-9a-f]{40}$/i.test(String(pending.material_sha || '')) + || !/^[1-9][0-9]*$/.test(String(pending.validate_run_id || ''))) { + return 'pending marker has incomplete material SHA or Validate run ID'; + } + return null; +} + export function preparedEvidenceError(prepared, { issue, stage, run }) { if (!prepared) return null; const badIdentity = prepared.schema !== 1 @@ -112,13 +138,8 @@ export function decideReconciliation({ } const requestAt = at(request.at); - const matching = runs - .map((run) => run.issue ? run : parseProcessRun(run)) - .filter(Boolean) - .filter((run) => run.issue === Number(issue.number) && run.label === label - && Number.isFinite(at(run.createdAt)) && at(run.createdAt) >= requestAt - 120_000) - .sort((a, b) => at(b.createdAt) - at(a.createdAt) || Number(b.id) - Number(a.id)); - const run = matching[0] || null; + const matching = reviewRunsForRequest(runs, issue.number, request); + const run = matching.find((candidate) => ACTIVE_RUN_STATES.has(candidate.status)) || matching[0] || null; if (!run) { if (now - requestAt < graceMs) return result('wait', 'label event is still within delivery grace', { label, stage }); @@ -147,6 +168,9 @@ export function decideReconciliation({ if (Number.isFinite(settledAt) && now - settledAt < graceMs) { return result('wait', 'completed run is still within label-application grace', { label, stage, run }); } + if (run.resultArtifact) { + return result('escalate', 'sealed model result exists but was not integrated; automatic rerun would spend the model twice', { label, stage, run }); + } if (run.conclusion === 'success' && run.pending) { // #636: успешный прогон без вердикта — это не потеря, а осознанный выход // подготовки: Validate с мутантами на материале ещё шёл. Пока он идёт — @@ -160,9 +184,6 @@ export function decideReconciliation({ if (run.conclusion === 'success') { return result('escalate', 'successful run did not move the review label', { label, stage, run }); } - if (run.resultArtifact) { - return result('escalate', 'sealed model result exists but was not integrated; automatic rerun would spend the model twice', { label, stage, run }); - } if (RETRYABLE_CONCLUSIONS.has(run.conclusion)) { return result('retry', `transient process run conclusion: ${run.conclusion}`, { label, stage, run }); } @@ -201,7 +222,7 @@ function issueView(repo, number) { return ghJson(['issue', 'view', String(number), '--repo', repo, '--json', 'number,title,labels,comments,updatedAt']); } -function issueEvents(repo, number) { +export function issueEvents(repo, number) { const pages = ghJson(['api', '--paginate', '--slurp', `repos/${repo}/issues/${number}/events?per_page=100`]); return Array.isArray(pages?.[0]) ? pages.flat() : (Array.isArray(pages) ? pages : []); } @@ -216,12 +237,34 @@ function openReviewIssues(repo) { .sort((a, b) => a.number - b.number); } -export function processRuns(repo, issues = []) { - const pages = [1, 2].flatMap((page) => { - const response = ghJson(['api', `repos/${repo}/actions/workflows/process.yml/runs?event=issues&per_page=100&page=${page}`]); - return response.workflow_runs || []; - }); - return pages.map((raw) => { +export function processRuns(repo, issues = [], { since = null, getJson = ghJson } = {}) { + // #775: the former latest-200 window could silently hide the pending round. + // Bound history by the current label request, not unrelated workflow volume. + const starts = since ? [at(since)] : issues.map((issue) => { + const label = REVIEW_LABELS.find((candidate) => labelsOf(issue).includes(candidate)); + return at(latestReviewRequest(issueEvents(repo, issue.number), label)?.at); + }).filter(Number.isFinite); + if (!starts.length) return []; + const oldest = Math.min(...starts); + if (!Number.isFinite(oldest)) throw new Error('invalid process history start'); + const rows = new Map(); + for (let page = 1; ; page++) { + // Do not use event/created search filters: filtered Actions queries cap at + // 1000 results. Filter issue events locally after complete pagination. + const response = getJson(['api', `repos/${repo}/actions/workflows/process.yml/runs?per_page=100&page=${page}`]); + const batch = response.workflow_runs; + if (!Array.isArray(batch)) throw new Error('invalid process history response'); + if (!batch.length) break; + let added = 0; + for (const raw of batch) { + if (!raw.id || !Number.isFinite(at(raw.created_at))) throw new Error('invalid process history run'); + const key = `${raw.id}/${raw.run_attempt || 1}`; + if (!rows.has(key)) { rows.set(key, raw); added++; } + } + if (!added) throw new Error('process history pagination made no progress'); + if (batch.length < 100 || batch.every((raw) => at(raw.created_at) < oldest)) break; + } + return [...rows.values()].filter((raw) => raw.event === 'issues' && at(raw.created_at) >= oldest).map((raw) => { const stable = parseProcessRun(raw); if (stable) return stable; // Runs created before #555 used the issue title as display_title. Accept @@ -303,9 +346,8 @@ function hydrateRunEvidence(repo, issue, run) { return { ...run, evidenceError: 'duplicate prepared/result/pending artifacts' }; } const pending = pendingArtifacts.length === 1 ? loadSealedArtifact(repo, run, pendingArtifacts[0], 'pending.json') : null; - if (pending && (String(pending.issue) !== String(issue.number) || String(pending.run_id) !== String(run.id))) { - return { ...run, evidenceError: 'pending marker belongs to another issue/run' }; - } + const pendingError = pending && pendingEvidenceError(pending, { issue: issue.number, run }); + if (pendingError) return { ...run, evidenceError: pendingError }; return { ...run, preparedArtifact: preparedArtifacts.length === 1, @@ -370,8 +412,7 @@ async function snapshot(repo, baseRuns, issue) { const labels = labelsOf(fresh); const label = REVIEW_LABELS.find((candidate) => labels.includes(candidate)) || null; const request = latestReviewRequest(events, label); - const candidates = baseRuns.filter((run) => run.issue === issue.number && run.label === label) - .sort((a, b) => at(b.createdAt) - at(a.createdAt)); + const candidates = reviewRunsForRequest(baseRuns, issue.number, request); const hydrated = candidates.length ? [hydrateRunEvidence(repo, fresh, candidates[0]), ...candidates.slice(1)] : candidates; return { issue: fresh, request, runs: hydrated }; } diff --git a/scripts/process-resume.mjs b/scripts/process-resume.mjs index 4ac7cc2a..6ab8cf09 100644 --- a/scripts/process-resume.mjs +++ b/scripts/process-resume.mjs @@ -16,10 +16,11 @@ // модели. Страховка на потерянное событие — process-reconcile (тот же маркер). import { execFileSync } from 'node:child_process'; import { appendFileSync } from 'node:fs'; +import { setTimeout as delay } from 'node:timers/promises'; import { isMainModule } from './spawn-portable.mjs'; import { - ACTIVE_RUN_STATES, artifactNames, loadSealedArtifact, parseProcessRun, pendingArtifactName, - processRuns, relabel, + ACTIVE_RUN_STATES, artifactNames, issueEvents, latestReviewRequest, loadSealedArtifact, + pendingArtifactName, pendingEvidenceError, processRuns, relabel, reviewRunsForRequest, } from './process-reconcile.mjs'; export const REVIEW_LABEL = 'S7-code-review'; @@ -31,42 +32,49 @@ export function issueNumberFromBranch(branch) { return match ? Number(match[1]) : null; } -const at = (value) => { - const parsed = Date.parse(String(value || '')); - return Number.isFinite(parsed) ? parsed : NaN; -}; - /** * Чистое решение: будить раунд или нет. * * @param {object} p * @param {string[]} p.labels метки issue - * @param {object} p.validateRun завершённый прогон Validate: { event, status, headSha } + * @param {object} p.validateRun завершённый прогон Validate: { id, event, status, headSha } + * @param {number} p.issue номер issue + * @param {object} p.request последняя постановка S7 из timeline + * @param {string} p.branch ветка материала + * @param {string} p.headSha текущая вершина ветки * @param {string} p.sha SHA материала, на котором завершился Validate * @param {object[]} p.runs прогоны конвейера этой issue (parseProcessRun-совместимые) * @param {(run) => object|null} p.pendingOf маркер ожидания прогона либо null - * @returns {{action:'resume'|'noop', reason:string, run?:object}} + * @returns {{action:'resume'|'noop', reason:string, run?:object, recheck?:boolean}} */ -export function decideResume({ labels = [], validateRun, sha, runs = [], pendingOf = () => null }) { - if (validateRun?.event !== 'workflow_dispatch') return { action: 'noop', reason: 'not a dispatch run — push runs carry no mutants' }; +export function decideResume({ labels = [], issue, request, branch, headSha, validateRun, sha, runs = [], pendingOf = () => null }) { + if (validateRun?.event !== 'workflow_dispatch') return { action: 'noop', reason: 'not a dispatch run — only the awaited dispatch wakes the round' }; if (validateRun?.status !== 'completed') return { action: 'noop', reason: 'validate run is not completed' }; if (validateRun?.headSha && sha && validateRun.headSha !== sha) return { action: 'noop', reason: 'validate run head differs from the material' }; if (!labels.includes(REVIEW_LABEL)) return { action: 'noop', reason: 'issue is not awaiting code review' }; + if (labels.includes('S4-spec-review')) return { action: 'noop', reason: 'ambiguous review status labels' }; const stop = STOP_LABELS.find((label) => labels.includes(label)); if (stop) return { action: 'noop', reason: `owner stopped the review (${stop})` }; - const mine = runs.map((run) => run.issue ? run : parseProcessRun(run)).filter(Boolean) - .filter((run) => run.label === REVIEW_LABEL) - .sort((a, b) => at(b.createdAt) - at(a.createdAt) || Number(b.id) - Number(a.id)); - if (mine.some((run) => ACTIVE_RUN_STATES.has(run.status))) return { action: 'noop', reason: 'a process run for this issue is already active' }; + if (request?.label !== REVIEW_LABEL || !Number.isFinite(Date.parse(request.at))) { + return { action: 'noop', reason: 'current review label has no timeline request' }; + } + if (!sha || headSha !== sha) return { action: 'noop', reason: 'branch head differs from the completed Validate material' }; + const mine = reviewRunsForRequest(runs, issue, request); + if (mine.some((run) => ACTIVE_RUN_STATES.has(run.status))) return { action: 'noop', recheck: true, reason: 'a process run for this issue is already active' }; const latest = mine[0]; - if (!latest) return { action: 'noop', reason: 'no process run for this issue — reconcile owns lost requests' }; + if (!latest) return { action: 'noop', recheck: true, reason: 'no process run for this issue — reconcile owns lost requests' }; if (latest.status !== 'completed' || latest.conclusion !== 'success') { return { action: 'noop', reason: `latest process run is ${latest.status}/${latest.conclusion || 'none'} — nothing was left pending` }; } const pending = pendingOf(latest); - if (!pending) return { action: 'noop', reason: 'latest process run left no pending marker — it did not wait for Validate' }; + if (!pending) return { action: 'noop', recheck: true, reason: 'latest process run left no pending marker — it did not wait for Validate' }; + const error = pendingEvidenceError(pending, { issue, run: latest }); + if (error) return { action: 'noop', reason: error }; + if (pending.branch !== branch || String(pending.validate_run_id) !== String(validateRun.id)) { + return { action: 'noop', reason: 'pending marker waits for another branch or Validate run' }; + } if (String(pending.material_sha) !== String(sha)) { - return { action: 'noop', reason: `pending marker waits for ${String(pending.material_sha).slice(0, 8)}, not ${String(sha).slice(0, 8)}` }; + return { action: 'noop', recheck: true, reason: `pending marker waits for ${String(pending.material_sha).slice(0, 8)}, not ${String(sha).slice(0, 8)}` }; } return { action: 'resume', reason: 'the round was waiting for exactly this Validate run', run: latest }; } @@ -75,25 +83,56 @@ function gh(args) { return execFileSync('gh', args, { encoding: 'utf8' }); } -export async function resume({ repo, branch, sha, validateRun, apply = true, ops = null }) { +export async function resume({ repo, branch, sha, validateRun, apply = true, ops = null, sleep = delay }) { const issue = issueNumberFromBranch(branch); if (!issue) return { action: 'noop', reason: `branch ${branch} is not an issue branch`, issue: null }; const io = ops || { - labels: () => JSON.parse(gh(['issue', 'view', String(issue), '--repo', repo, '--json', 'labels'])).labels.map((label) => label.name), - runs: () => processRuns(repo, []), + state: () => { + const current = JSON.parse(gh(['issue', 'view', String(issue), '--repo', repo, '--json', 'labels,state'])); + const labels = current.state === 'OPEN' ? current.labels.map((label) => label.name) : []; + // A deleted branch after merge/closure is normal, not a failed recovery. + if (!labels.includes(REVIEW_LABEL) || labels.includes('S4-spec-review') || STOP_LABELS.some((label) => labels.includes(label))) { + return { labels, request: null, headSha: null }; + } + return { labels, request: latestReviewRequest(issueEvents(repo, issue), REVIEW_LABEL), + headSha: JSON.parse(gh(['api', `repos/${repo}/git/ref/heads/${branch}`])).object.sha }; + }, + runs: (request) => request ? processRuns(repo, [], { since: request.at }) : [], pendingOf: (run) => { const name = pendingArtifactName(issue, run); - const artifact = artifactNames(repo, run).find((item) => item.name === name && !item.expired); - if (!artifact) return null; - const pending = loadSealedArtifact(repo, run, artifact, 'pending.json'); - return String(pending.issue) === String(issue) && String(pending.run_id) === String(run.id) ? pending : null; + const artifacts = artifactNames(repo, run); + if (artifacts.some((item) => item.name === `review-result-${issue}-${run.id}-${run.attempt}` && !item.expired)) { + throw new Error('sealed model result exists — refusing to wake a completed review'); + } + const matches = artifacts.filter((item) => item.name === name && !item.expired); + if (matches.length > 1) throw new Error('duplicate pending artifacts'); + return matches.length ? loadSealedArtifact(repo, run, matches[0], 'pending.json') : null; }, relabel: () => relabel(repo, { number: issue }, REVIEW_LABEL), }; - const labels = io.labels(); - const runs = io.runs().filter((run) => run.issue === issue); - const decision = decideResume({ labels, validateRun, sha, runs, pendingOf: io.pendingOf }); - if (decision.action === 'resume' && apply) io.relabel(); + const inspect = () => { + const state = io.state(); + const decision = decideResume({ ...state, issue, branch, validateRun, sha, + runs: io.runs(state.request), pendingOf: io.pendingOf }); + return { state, decision }; + }; + let first = inspect(); + // A completed Validate can arrive before the process list/artifact is visible. + // Three fresh snapshots at most; never search behind a completed review barrier. + // After this short delivery grace, the scheduled reconciler remains the fallback. + for (let attempt = 1; first.decision.recheck && attempt < 3; attempt++) { + await sleep(5_000); + first = inspect(); + } + const decision = first.decision; + if (decision.action === 'resume' && apply) { + const second = inspect(); + if (second.decision.action !== 'resume' || first.state.request.id !== second.state.request?.id + || decision.run.id !== second.decision.run?.id || decision.run.attempt !== second.decision.run?.attempt) { + return { action: 'noop', reason: `state changed before write: ${second.decision.reason}`, issue, applied: false }; + } + io.relabel(); + } return { ...decision, issue, applied: decision.action === 'resume' && apply }; } @@ -102,13 +141,13 @@ if (isMainModule(import.meta.url)) { const repo = arg('repo') || process.env.GITHUB_REPOSITORY; const branch = arg('branch'); const sha = arg('sha'); - if (!repo || !branch || !sha) { - console.error('usage: process-resume.mjs --repo= --branch= --sha= --event= --status= [--apply=false]'); + if (!repo || !branch || !sha || !arg('run-id')) { + console.error('usage: process-resume.mjs --repo= --branch= --sha= --run-id= --event= --status= [--apply=false]'); process.exit(2); } const outcome = await resume({ repo, branch, sha, - validateRun: { event: arg('event'), status: arg('status') || 'completed', headSha: sha }, + validateRun: { id: arg('run-id'), event: arg('event'), status: arg('status') || 'completed', headSha: sha }, apply: arg('apply') !== 'false', }); const lines = [`action=${outcome.action}`, `issue=${outcome.issue ?? ''}`, `reason=${outcome.reason}`, `applied=${outcome.applied ? 'true' : 'false'}`]; diff --git a/test/process-pending-round.test.mjs b/test/process-pending-round.test.mjs new file mode 100644 index 00000000..f1903d5d --- /dev/null +++ b/test/process-pending-round.test.mjs @@ -0,0 +1,175 @@ +// #775: real incident identities; list-window/order variants below are synthetic +// witnesses, not a claim about GitHub's unsaved historical API responses. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { decideReconciliation, pendingEvidenceError, processRuns } from '../scripts/process-reconcile.mjs'; +import { decideResume, resume } from '../scripts/process-resume.mjs'; + +const LABEL = 'S7-code-review'; +const incidents = [ + [740, 'stairs-floor-switch', 36877086418, 36877204584, 'f23cca8f65a9641f9cc409b749347ba167cf9c65', '14:32:05', 'success'], + [748, 'canon-reconcile', 36883983363, 36884081445, '395210117e242b5b16118cdfd1985f9a3d620cef', '15:24:18', 'success'], + [744, 'floor-geometry-key', 36875649994, 36875756451, '62bd32538132462d4f5222a36db3bdf1e660b80b', '14:21:07', 'failure'], +]; +function fixture(row = incidents[0]) { + const [issue, slug, id, validateId, sha, time, conclusion] = row; + const branch = `issue/${issue}-${slug}`; + const createdAt = `2026-10-01T${time}Z`; + const run = { id, attempt: 1, issue, label: LABEL, stage: 'code', status: 'completed', conclusion: 'success', createdAt }; + const request = { id: 'request-1', label: LABEL, at: createdAt }; + const pending = { schema: 1, issue: String(issue), run_id: String(id), run_attempt: '1', stage: 'code', branch, + material_sha: sha, validate_run_id: String(validateId) }; + return { issue, branch, sha, headSha: sha, request, labels: [LABEL], run, pending, + runs: [run], validateRun: { id: validateId, event: 'workflow_dispatch', status: 'completed', headSha: sha, conclusion }, + pendingOf: (candidate) => candidate.id === id ? pending : null }; +} +const raw = (run, extra = {}) => ({ ...run, event: 'issues', + display_title: `process #${run.issue} · ${run.label} · fixture`, created_at: run.createdAt, ...extra }); + +test('#775 all three incident markers resume, including red Validate (ordinary prepare owns S6/comment)', () => { + for (const row of incidents) assert.equal(decideResume(fixture(row)).action, 'resume', `#${row[0]}`); +}); + +test('#775 history is paged to the request, not capped at 200 or filtered-search 1000 runs', () => { + for (const row of incidents) { + const f = fixture(row); + const unrelated = Array.from({ length: 1100 }, (_, n) => raw({ ...f.run, id: n + 1, issue: 999, + createdAt: '2026-10-01T16:00:00Z' }, { display_title: 'process #999 · P2 · unrelated label' })); + const rows = [...unrelated, raw(f.run), raw({ ...f.run, id: 2, createdAt: '2026-09-30T00:00:00Z' })]; + const calls = []; + const runs = processRuns('o/r', [{ number: f.issue }], { since: f.request.at, getJson: (args) => { + const url = args[1]; + calls.push(url); + assert.ok(!url.includes('event='), 'unfiltered endpoint avoids the search-result cap'); + const page = Number(new URL(`https://api.test/${url}`).searchParams.get('page')); + return { workflow_runs: rows.slice((page - 1) * 100, page * 100) }; + } }); + assert.equal(calls.length, 12); + assert.equal(decideResume({ ...f, runs }).action, 'resume', `#${f.issue}`); + } +}); + +test('#775 history errors/repeated pages are not silently interpreted as a lost request', () => { + const f = fixture(); + const page = Array.from({ length: 100 }, (_, n) => raw({ ...f.run, id: n + 1 })); + let calls = 0; + assert.throws(() => processRuns('o/r', [], { since: f.request.at, getJson: () => { + if (++calls === 2) throw new Error('API unavailable'); + return { workflow_runs: page }; + } }), /API unavailable/); + assert.throws(() => processRuns('o/r', [], { since: f.request.at, + getJson: () => ({ workflow_runs: page }) }), /pagination/); +}); + +test('#775 stop at the request time, deduplicate overlapping pages, ignore non-issue workflow events', () => { + const f = fixture(); + const first = Array.from({ length: 100 }, (_, n) => raw({ ...f.run, id: n + 1, createdAt: '2026-10-01T16:00:00Z' })); + const second = [first[99], raw(f.run), raw({ ...f.run, id: 3, createdAt: '2026-09-30T00:00:00Z' }), + raw({ ...f.run, id: 200 }, { event: 'workflow_dispatch' })]; + let count = 0; + const runs = processRuns('o/r', [], { since: f.request.at, + getJson: () => ({ workflow_runs: [first, second][count++] }) }); + assert.equal(count, 2); + assert.equal(runs.length, 101); + assert.equal(new Set(runs.map((run) => run.id)).size, runs.length); +}); + +test('#775 skipped noise can be ignored but newer attempted reviews are barriers', () => { + const f = fixture(); + const newer = { ...f.run, id: f.run.id + 1, createdAt: '2026-10-01T16:00:00Z' }; + assert.equal(decideResume({ ...f, runs: [{ ...newer, conclusion: 'skipped' }, f.run] }).action, 'resume'); + for (const conclusion of ['failure', 'cancelled', 'success']) { + assert.equal(decideResume({ ...f, runs: [{ ...newer, conclusion }, f.run] }).action, 'noop', conclusion); + } + assert.equal(decideResume({ ...f, runs: [{ ...newer, status: 'in_progress' }, f.run] }).action, 'noop'); +}); + +test('#775 a new request, branch head, different dispatch or damaged identity never wakes the old round', () => { + const f = fixture(); + for (const overrides of [ + { request: null }, { request: { ...f.request, at: '2026-10-01T16:00:00Z' } }, + { headSha: 'b'.repeat(40) }, { issue: 999 }, { branch: 'issue/740-other-branch' }, + { validateRun: { ...f.validateRun, id: f.validateRun.id + 1 } }, + { labels: [LABEL, 'S4-spec-review'] }, + ]) assert.equal(decideResume({ ...f, ...overrides }).action, 'noop', JSON.stringify(overrides)); + for (const overrides of [{ schema: 2 }, { issue: '999' }, { run_id: '1' }, { run_attempt: '2' }, + { stage: 'spec' }, { branch: 'other' }, { validate_run_id: '' }]) { + assert.equal(decideResume({ ...f, pendingOf: () => ({ ...f.pending, ...overrides }) }).action, 'noop', JSON.stringify(overrides)); + } +}); + +test('#775 write path rechecks current request, stop labels, head and runs; duplicates cannot relabel twice', async () => { + const f = fixture(); + for (const changed of [{ labels: [] }, { labels: [LABEL, 'blocked'] }, { headSha: 'b'.repeat(40) }, + { request: { ...f.request, id: 'request-2' } }]) { + let reads = 0; + let writes = 0; + const ops = { state: () => ++reads === 1 ? f : { ...f, ...changed }, runs: () => f.runs, + pendingOf: f.pendingOf, relabel: () => writes++ }; + const result = await resume({ ...f, repo: 'o/r', ops, sleep: async () => {} }); + assert.equal(result.applied, false); + assert.equal(writes, 0); + } + let reads = 0; + let writes = 0; + let state = f; + const ops = { state: () => state, runs: () => { reads++; return f.runs; }, pendingOf: f.pendingOf, + relabel: () => { writes++; state = { ...f, request: { ...f.request, id: 'request-2', at: '2026-10-01T17:00:00Z' } }; } }; + assert.equal((await resume({ ...f, repo: 'o/r', ops, sleep: async () => {} })).applied, true); + assert.equal((await resume({ ...f, repo: 'o/r', ops, sleep: async () => {} })).applied, false); + assert.equal(writes, 1); + assert.ok(reads >= 2); +}); + +test('#775 transient missing/old/no-marker snapshots recover without waiting for the schedule', async () => { + for (const row of incidents) { + const f = fixture(row); + let reads = 0; + let writes = 0; + const waits = []; + const ops = { state: () => f, runs: () => ++reads === 1 + ? (f.issue === 744 ? [] : [{ ...f.run, id: 1, createdAt: f.issue === 748 ? '2026-09-30T00:00:00Z' : f.run.createdAt }]) + : f.runs, pendingOf: f.pendingOf, relabel: () => writes++ }; + const result = await resume({ ...f, repo: 'o/r', ops, sleep: async (ms) => waits.push(ms) }); + assert.equal(result.applied, true, `#${f.issue}`); + assert.equal(writes, 1); + assert.deepEqual(waits, [5000]); + } +}); + +test('#775 absent evidence gets a bounded grace, never a guessed relabel', async () => { + const f = fixture(); + let reads = 0; + const waits = []; + const result = await resume({ ...f, repo: 'o/r', sleep: async (ms) => waits.push(ms), + ops: { state: () => f, runs: () => { reads++; return []; }, pendingOf: f.pendingOf, + relabel: () => assert.fail('no evidence') } }); + assert.equal(result.action, 'noop'); + assert.equal(result.applied, false); + assert.equal(reads, 3); + assert.deepEqual(waits, [5000, 5000]); +}); + +test('#775 reconciler uses the same request boundary, skipped-run handling and sealed identity', () => { + const f = fixture(); + const now = Date.parse('2026-10-01T17:00:00Z'); + const base = { issue: { number: f.issue, labels: [LABEL] }, request: f.request, now }; + const ready = { ...f.run, pending: f.pending, pendingValidate: 'completed' }; + const skipped = { ...f.run, id: 999, createdAt: '2026-10-01T16:00:00Z', conclusion: 'skipped' }; + assert.equal(decideReconciliation({ ...base, runs: [skipped, ready] }).action, 'retry'); + assert.equal(decideReconciliation({ ...base, runs: [{ ...ready, resultArtifact: true }] }).action, 'escalate'); + const justReapplied = { ...f.request, at: '2026-10-01T14:32:06Z' }; + assert.equal(decideReconciliation({ ...base, request: justReapplied, now: Date.parse(justReapplied.at), runs: [ready] }).action, 'wait', + 'previous run one second before the new label cannot be recovered as the new request'); + assert.equal(pendingEvidenceError(f.pending, { issue: f.issue, run: f.run }), null); + assert.match(pendingEvidenceError({ ...f.pending, run_attempt: '2' }, { issue: f.issue, run: f.run }), /attempt/); +}); + +test('#775 a run starting between snapshots prevents the write', async () => { + const f = fixture(); + let reads = 0; + const ops = { state: () => f, runs: () => ++reads === 1 ? f.runs + : [{ ...f.run, id: f.run.id + 1, status: 'in_progress', createdAt: '2026-10-01T16:00:00Z' }, f.run], + pendingOf: f.pendingOf, relabel: () => assert.fail('new process already running') }; + assert.equal((await resume({ ...f, repo: 'o/r', ops })).applied, false); +}); diff --git a/test/process-resume.test.mjs b/test/process-resume.test.mjs index a8db13e8..cbdc5ea2 100644 --- a/test/process-resume.test.mjs +++ b/test/process-resume.test.mjs @@ -5,16 +5,19 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; -import { decideResume, issueNumberFromBranch, resume, REVIEW_LABEL } from '../scripts/process-resume.mjs'; +import { decideResume as decide, issueNumberFromBranch, resume, REVIEW_LABEL } from '../scripts/process-resume.mjs'; const SHA = 'a'.repeat(40); const OTHER = 'b'.repeat(40); -const validateRun = (over = {}) => ({ event: 'workflow_dispatch', status: 'completed', headSha: SHA, ...over }); +const validateRun = (over = {}) => ({ id: 80, event: 'workflow_dispatch', status: 'completed', headSha: SHA, ...over }); +const request = { id: 'event-1', at: '2026-09-23T09:00:00Z', label: REVIEW_LABEL }; +const decideResume = (args) => decide({ issue: 636, branch: 'issue/636-x', request, headSha: SHA, ...args }); const processRun = (over = {}) => ({ id: 70, attempt: 1, issue: 636, label: REVIEW_LABEL, stage: 'code', status: 'completed', conclusion: 'success', createdAt: '2026-09-23T10:00:00Z', ...over, }); -const pending = { schema: 1, issue: 636, run_id: 70, material_sha: SHA }; +const pending = { schema: 1, issue: 636, run_id: 70, run_attempt: 1, stage: 'code', branch: 'issue/636-x', + material_sha: SHA, validate_run_id: 80 }; const pendingOf = (run) => (run.id === 70 ? pending : null); test('#636 branch → issue number', () => { @@ -75,7 +78,7 @@ test('#636 the newest process run decides, not an older pending one', () => { test('#636 resume() relabels only on a resume decision and only with apply', async () => { const relabels = []; const ops = { - labels: () => [REVIEW_LABEL], + state: () => ({ labels: [REVIEW_LABEL], request, headSha: SHA }), runs: () => [processRun()], pendingOf, relabel: () => relabels.push(REVIEW_LABEL), @@ -107,6 +110,8 @@ test('#636 workflows: prepare exits pending with a sealed marker, resume relabel assert.match(resumeWf, /github\.event\.workflow_run\.event == 'workflow_dispatch' && startsWith\(github\.event\.workflow_run\.head_branch, 'issue\/'\)/); assert.match(resumeWf, /GH_TOKEN: \$\{\{ secrets\.HP_PROCESS_TOKEN \}\}/); assert.match(resumeWf, /node scripts\/process-resume\.mjs/); + assert.match(resumeWf, /VALIDATE_RUN_ID: \$\{\{ github\.event\.workflow_run\.id \}\}/); + assert.match(resumeWf, /--run-id="\$VALIDATE_RUN_ID"/); assert.ok(!/issues: write/.test(resumeWf), 'resume relabels with HP_PROCESS_TOKEN only'); assert.ok(!/issues: write/.test(resumeCaller), 'the caller ceiling does not grant issues: write either'); assert.match(validate, /for file in process\.yml mutation-gate\.yml process-resume\.yml [^\n]*; do/);