diff --git a/.github/workflows/process-reconcile.yml b/.github/workflows/process-reconcile.yml index eace6efe..d62e998a 100644 --- a/.github/workflows/process-reconcile.yml +++ b/.github/workflows/process-reconcile.yml @@ -49,6 +49,7 @@ jobs: --max-actions=5 \ --output=artifacts/process-reconcile/summary.json - name: Опубликовать компактный machine-readable итог + if: always() uses: actions/upload-artifact@v4 with: name: process-reconcile-${{ github.run_id }}-${{ github.run_attempt }} diff --git a/scripts/mutation-gate.mjs b/scripts/mutation-gate.mjs index ea2785d2..a9c07b03 100644 --- a/scripts/mutation-gate.mjs +++ b/scripts/mutation-gate.mjs @@ -7948,6 +7948,41 @@ const MUTANT_DEFINITIONS = [ replace: ' if (false && retryAlreadyIssued) {', }], }, + { + id: 'process-reconcile-relabels-before-durable-marker', + guard: 'node --test test/process-reconcile.test.mjs', + because: '#555 review r1: the dedupe/diagnostic comment must exist before a fallible label ' + + 'mutation; otherwise a failed restore is silent and repeats on every schedule tick', + patches: [{ + file: 'scripts/process-reconcile.mjs', + find: ' await ops.comment(repo, issue, commentBody(issue, request, decision, key));\n' + + " if (decision.action === 'retry') await ops.relabel(repo, issue, decision.label);", + replace: " if (decision.action === 'retry') await ops.relabel(repo, issue, decision.label);\n" + + ' await ops.comment(repo, issue, commentBody(issue, request, decision, key));', + }], + }, + { + id: 'process-reconcile-ignores-failed-label-restore', + guard: 'node --test test/process-reconcile.test.mjs', + because: '#555 review r1: the bounded second add-label attempt is authoritative; ignoring ' + + 'its failure would claim success while leaving the issue outside S4/S7', + patches: [{ + file: 'scripts/process-reconcile.mjs', + find: ' if (restore.status !== 0) throw new Error(`could not restore ${label}: ${(restore.stderr || first.stderr || \'\').trim()}`);', + replace: ' if (false && restore.status !== 0) throw new Error(`mutant: restore ignored`);', + }], + }, + { + id: 'process-reconcile-write-error-aborts-summary', + guard: 'node --test test/process-reconcile.test.mjs', + because: '#555 review r1: a write failure belongs in the machine summary and must not abort ' + + 'the whole snapshot before its artifact can be published', + patches: [{ + file: 'scripts/process-reconcile.mjs', + find: ' } catch (error) {\n record.error = error instanceof Error ? error.message : String(error);', + replace: ' } catch (error) {\n throw error; // mutant: summary is lost', + }], + }, { id: 'review-integration-skips-evidence-checksum', guard: 'node --test --test-name-pattern="#551" test/review-doc-guard.test.mjs', diff --git a/scripts/process-reconcile.mjs b/scripts/process-reconcile.mjs index 47c5aa06..c6bfe225 100644 --- a/scripts/process-reconcile.mjs +++ b/scripts/process-reconcile.mjs @@ -23,7 +23,7 @@ export const DEFAULT_ACTIVE_LIMIT_MS = 4 * 60 * 60_000; const STAGE = { 'S4-spec-review': 'spec', 'S7-code-review': 'code' }; const RUN_TITLE = /^process #(\d+) · (S4-spec-review|S7-code-review)(?: ·|$)/; const MARKER_PREFIX = 'houseplan-process-reconcile:v1'; -const RETRY_COMMENT = /houseplan-process-reconcile:v1:[^\s]+[\s\S]*Автосверка процесса повторно/; +const RETRY_COMMENT = /houseplan-process-reconcile:v1:[^\s]+[\s\S]*Автосверка процесса (?:пытается )?повторно/; const at = (value) => { const parsed = Date.parse(String(value || '')); @@ -281,8 +281,8 @@ function commentBody(issue, request, decision, key) { : ''; const link = run?.url ? ` [Прогон](${run.url}).` : ''; if (decision.action === 'retry') { - return `${markerFor(key)}\nАвтосверка процесса повторно разбудила \`${decision.label}\`: ${decision.reason}.${link}${evidence}\n\n` - + 'Вердикт не применялся, цикл ревью не расходуется самой сверкой; новый запуск заново проверит актуальные метки и материал.'; + return `${markerFor(key)}\nАвтосверка процесса пытается повторно разбудить \`${decision.label}\`: ${decision.reason}.${link}${evidence}\n\n` + + 'Вердикт не применялся, цикл ревью не расходуется самой сверкой; после успешного восстановления новый запуск заново проверит актуальные метки и материал.'; } return `${markerFor(key)}\n**Автосверка процесса не стала угадывать результат.** ${decision.reason}.${link}${evidence}\n\n` + `Запрос: \`${request?.id || 'не найден'}\`, текущая метка: \`${decision.label || labelsOf(issue).join(', ') || 'нет'}\`. ` @@ -293,15 +293,29 @@ function addComment(repo, issue, body) { gh(['issue', 'comment', String(issue.number), '--repo', repo, '--body', body]); } -function relabel(repo, issue, label) { - gh(['issue', 'edit', String(issue.number), '--repo', repo, '--remove-label', label]); - const added = gh(['issue', 'edit', String(issue.number), '--repo', repo, '--add-label', label], { allowFailure: true }); - if (added.status !== 0) { - // Best-effort rollback: leaving an issue without its review state is worse - // than a loud failed reconciliation. - gh(['issue', 'edit', String(issue.number), '--repo', repo, '--add-label', label], { allowFailure: true }); - throw new Error(`could not restore ${label}: ${(added.stderr || '').trim()}`); - } +export function relabel(repo, issue, label, execute = gh) { + execute(['issue', 'edit', String(issue.number), '--repo', repo, '--remove-label', label]); + const args = ['issue', 'edit', String(issue.number), '--repo', repo, '--add-label', label]; + const first = execute(args, { allowFailure: true }); + if (first.status === 0) return; + // One bounded restore attempt. Its result is authoritative; unlike the old + // best-effort call, a second failure is retained in summary.json. + const restore = execute(args, { allowFailure: true }); + if (restore.status !== 0) throw new Error(`could not restore ${label}: ${(restore.stderr || first.stderr || '').trim()}`); +} + +const DEFAULT_APPLY_OPS = { + comment: (repo, issue, body) => addComment(repo, issue, body), + relabel: (repo, issue, label) => relabel(repo, issue, label), +}; + +/** The write path is deliberately small and dependency-injected for failure fixtures. */ +export async function applyReconciliationDecision({ repo, issue, request, decision, key, ops = DEFAULT_APPLY_OPS }) { + // Persist the dedupe/diagnostic marker before touching the status. If comment + // publication fails, no relabel happens; if relabel fails, the owner still + // gets one durable explanation and the next schedule cannot loop silently. + await ops.comment(repo, issue, commentBody(issue, request, decision, key)); + if (decision.action === 'retry') await ops.relabel(repo, issue, decision.label); } async function snapshot(repo, baseRuns, issue) { @@ -316,13 +330,23 @@ async function snapshot(repo, baseRuns, issue) { return { issue: fresh, request, runs: hydrated }; } -export async function reconcileAll({ repo, apply = false, maxActions = 5, now = Date.now() }) { - const issues = openReviewIssues(repo); - const runs = processRuns(repo, issues); +const DEFAULT_RUNTIME = { + openReviewIssues, + processRuns, + snapshot, + applyDecision: applyReconciliationDecision, +}; + +export async function reconcileAll({ + repo, apply = false, maxActions = 5, now = Date.now(), runtime = DEFAULT_RUNTIME, +}) { + const issues = await runtime.openReviewIssues(repo); + const runs = await runtime.processRuns(repo, issues); const records = []; let mutations = 0; + let failures = 0; for (const listed of issues) { - const first = await snapshot(repo, runs, listed); + const first = await runtime.snapshot(repo, runs, listed); const decision = decideReconciliation({ ...first, now }); const key = reconciliationKey(first.issue, first.request, decision); const record = { @@ -346,15 +370,19 @@ export async function reconcileAll({ repo, apply = false, maxActions = 5, now = && !alreadyReported(first.issue, key)) { // Re-read immediately before a write. A label/body/owner decision may have // changed while runs and artifacts were inspected. - const freshRuns = processRuns(repo, [first.issue]); - const second = await snapshot(repo, freshRuns, first.issue); + const freshRuns = await runtime.processRuns(repo, [first.issue]); + const second = await runtime.snapshot(repo, freshRuns, first.issue); const confirmed = decideReconciliation({ ...second, now: Date.now() }); const confirmedKey = reconciliationKey(second.issue, second.request, confirmed); if (confirmed.action === decision.action && confirmedKey === key && !alreadyReported(second.issue, key)) { - if (decision.action === 'retry') relabel(repo, second.issue, decision.label); - addComment(repo, second.issue, commentBody(second.issue, second.request, confirmed, key)); - record.applied = true; - mutations++; + try { + await runtime.applyDecision({ repo, issue: second.issue, request: second.request, decision: confirmed, key }); + record.applied = true; + mutations++; + } catch (error) { + record.error = error instanceof Error ? error.message : String(error); + failures++; + } } else { record.reason = `state changed before write: ${confirmed.action}/${confirmed.reason}`; record.action = 'noop'; @@ -369,6 +397,7 @@ export async function reconcileAll({ repo, apply = false, maxActions = 5, now = apply, counts: records.reduce((out, row) => ({ ...out, [row.action]: (out[row.action] || 0) + 1 }), {}), mutations, + failures, records, }; } @@ -391,7 +420,8 @@ if (isMainModule(import.meta.url)) { mkdirSync(dirname(output), { recursive: true }); writeFileSync(output, body); } - process.stdout.write(`${JSON.stringify({ schema: summary.schema, counts: summary.counts, mutations: summary.mutations })}\n`); + process.stdout.write(`${JSON.stringify({ schema: summary.schema, counts: summary.counts, mutations: summary.mutations, failures: summary.failures })}\n`); + if (summary.failures) process.exitCode = 1; }).catch((error) => { console.error(`process-reconcile: ${error instanceof Error ? error.stack || error.message : String(error)}`); process.exitCode = 2; diff --git a/test/process-reconcile.test.mjs b/test/process-reconcile.test.mjs index d6b93f04..62c33c4c 100644 --- a/test/process-reconcile.test.mjs +++ b/test/process-reconcile.test.mjs @@ -1,8 +1,8 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { - alreadyReported, decideReconciliation, latestReviewRequest, markerFor, - parseProcessRun, preparedEvidenceError, reconciliationKey, + alreadyReported, applyReconciliationDecision, decideReconciliation, latestReviewRequest, markerFor, + parseProcessRun, preparedEvidenceError, reconcileAll, reconciliationKey, relabel, } from '../scripts/process-reconcile.mjs'; const NOW = Date.parse('2026-09-13T12:00:00Z'); @@ -137,7 +137,7 @@ test('#555 repeat reconciliation has a stable dedupe marker', () => { ...issue(), comments: [{ createdAt: '2026-09-13T10:05:00Z', - body: `${markerFor(key)}\nАвтосверка процесса повторно разбудила \`S7-code-review\``, + body: `${markerFor(key)}\nАвтосверка процесса пытается повторно разбудить \`S7-code-review\``, }], }; const retriedRequest = { ...request, id: 'event-8', at: '2026-09-13T10:04:59Z' }; @@ -146,6 +146,64 @@ test('#555 repeat reconciliation has a stable dedupe marker', () => { assert.match(stopped.reason, /one automatic retry/); }); +test('#555 write path persists the marker before relabel and never mutates after comment failure', async () => { + const decision = decideReconciliation({ issue: issue(), request, runs: [], now: NOW }); + const key = reconciliationKey(issue(), request, decision); + const calls = []; + await assert.rejects(applyReconciliationDecision({ + repo: 'owner/repo', issue: issue(), request, decision, key, + ops: { + comment: async () => { calls.push('comment'); throw new Error('comment unavailable'); }, + relabel: async () => { calls.push('relabel'); }, + }, + }), /comment unavailable/); + assert.deepEqual(calls, ['comment']); + + calls.length = 0; + await applyReconciliationDecision({ + repo: 'owner/repo', issue: issue(), request, decision, key, + ops: { + comment: async (_repo, _issue, body) => { calls.push('comment'); assert.match(body, new RegExp(markerFor(key))); }, + relabel: async () => { calls.push('relabel'); }, + }, + }); + assert.deepEqual(calls, ['comment', 'relabel']); +}); + +test('#555 relabel checks the bounded restore attempt instead of throwing the first error blindly', () => { + const calls = []; + let adds = 0; + const recovered = (args) => { + calls.push(args.at(-2)); + if (args.includes('--remove-label')) return { status: 0, stderr: '' }; + adds++; + return adds === 1 ? { status: 1, stderr: 'transient' } : { status: 0, stderr: '' }; + }; + assert.doesNotThrow(() => relabel('owner/repo', issue(), 'S7-code-review', recovered)); + assert.equal(adds, 2); + assert.equal(calls.length, 3); + + const broken = (args) => args.includes('--remove-label') + ? { status: 0, stderr: '' } : { status: 1, stderr: 'still broken' }; + assert.throws(() => relabel('owner/repo', issue(), 'S7-code-review', broken), /still broken/); +}); + +test('#555 one write failure stays in machine summary and does not abort the snapshot', async () => { + const snapshot = { issue: issue(), request, runs: [] }; + const runtime = { + openReviewIssues: async () => [snapshot.issue], + processRuns: async () => [], + snapshot: async () => snapshot, + applyDecision: async () => { throw new Error('write failed after durable diagnosis'); }, + }; + const summary = await reconcileAll({ repo: 'owner/repo', apply: true, now: NOW, runtime }); + assert.equal(summary.failures, 1); + assert.equal(summary.mutations, 0); + assert.equal(summary.records.length, 1); + assert.equal(summary.records[0].applied, false); + assert.match(summary.records[0].error, /write failed/); +}); + test('#555 a fresh label/run completion stays inside grace instead of duplicating work', () => { assert.equal(decideReconciliation({ issue: issue(), diff --git a/test/review-doc-guard.test.mjs b/test/review-doc-guard.test.mjs index 322f1835..f9a642de 100644 --- a/test/review-doc-guard.test.mjs +++ b/test/review-doc-guard.test.mjs @@ -786,6 +786,8 @@ test('#555: bounded reconciler wakes only lost review requests and emits one mac assert.match(workflow, /secrets\.HP_PROCESS_TOKEN/); assert.match(workflow, /node scripts\/process-reconcile\.mjs[\s\S]*--apply="\$APPLY"/); assert.match(workflow, /--max-actions=5/); + assert.match(workflow, /- name: Опубликовать компактный machine-readable итог\n\s+if: always\(\)/, + 'summary artifact survives a failed write or reconciler exit'); assert.match(workflow, /process-reconcile-\$\{\{ github\.run_id \}\}-\$\{\{ github\.run_attempt \}\}/); assert.match(process, /houseplan-process-reconcile\/v1/); assert.match(process, /второй вызов модели или S8 по догадке запрещены/);