mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
ci(process): раунд ревью ждёт Validate событием, а не сном раннера (#636)
Стадия prepare спала ≈ 28 минут на раунд, пока шёл Validate с мутантами на материале (модель работает 10–12); за неделю ≈ 420–500 job-минут простоя и потолок бюджета стадии 55 минут. - validate-gate.mjs: `--no-wait` — гейт диспатчит прогон, убеждается, что тот встал на материал (#539 сохранён), и возвращает `pending` (код 2) вместо ожидания; завершённый зелёный/красный отдаёт сразу, как прежде. - process.yml prepare: третий исход `proceed=pending`: запечатанный маркер `review-pending-<issue>-<run>-<attempt>` (issue, stage, branch, material_sha, validate run) и выход; модель и интеграция не запускаются; возврат автору — только на явном `false`. - process-resume.yml + scripts/process-resume.mjs: на `workflow_run: completed` Validate по ветке issue/* — если метка S7 стоит, активного прогона нет и последний прогон оставил маркер на этот SHA, переставить S7 (HP_PROCESS_TOKEN); новый прогон находит завершённый dispatch сразу. Без маркера не будит. - process-reconcile.mjs: читает маркер и состояние Validate на материале; идёт — wait, завершился/пропал без продолжения — retry; без маркера — прежний escalate. Общий loadSealedArtifact, экспорт processRuns/artifactNames. - preflight сверяет process-resume.yml между main и dev наравне с process.yml. - Тесты: validate-gate (4), process-resume (8, включая контракт трёх workflow), process-reconcile (2); мутанты gate-no-wait-still-sleeps, resume-wakes-round-without-marker, resume-ignores-active-run, reconcile-wakes-pending-while-validate-active. PROCESS.md §10.4, AGENTS.md. Issue: #636 User-Visible: no
This commit is contained in:
@@ -381,7 +381,7 @@ test('#472 AC7: отсутствие Telegram-секретов не роняет
|
||||
});
|
||||
|
||||
test('#472 AC8: Validate сверяет mutation-gate.yml между main и dev наравне с process.yml', () => {
|
||||
assert.match(validateWorkflowText, /for file in process\.yml mutation-gate\.yml; do/);
|
||||
assert.match(validateWorkflowText, /for file in process\.yml mutation-gate\.yml process-resume\.yml; do/);
|
||||
});
|
||||
|
||||
// #475. Свидетель гниёт двумя способами: изменился файл, который он патчит,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import {
|
||||
validateStateOnMaterial,
|
||||
alreadyReported, applyReconciliationDecision, decideReconciliation, latestReviewRequest, markerFor,
|
||||
parseProcessRun, preparedEvidenceError, reconcileAll, reconciliationKey, relabel,
|
||||
} from '../scripts/process-reconcile.mjs';
|
||||
@@ -35,7 +36,7 @@ test('#555 maps a stable process run-name to issue and stage', () => {
|
||||
id: 70, attempt: 2, issue: 555, label: 'S7-code-review', stage: 'code',
|
||||
status: 'in_progress', conclusion: '', createdAt: '2026-09-13T10:00:01Z',
|
||||
updatedAt: null, url: null, prepared: null, preparedArtifact: false,
|
||||
resultArtifact: false, evidenceError: null,
|
||||
resultArtifact: false, pending: null, pendingValidate: null, evidenceError: null,
|
||||
});
|
||||
assert.equal(parseProcessRun({ display_title: 'unrelated label event' }), null);
|
||||
});
|
||||
@@ -215,3 +216,41 @@ test('#555 a fresh label/run completion stays inside grace instead of duplicatin
|
||||
runs: [run({ conclusion: 'cancelled', updatedAt: '2026-09-13T11:58:00Z' })], now: NOW,
|
||||
}).action, 'wait');
|
||||
});
|
||||
|
||||
// #636: подготовка вышла успешно, оставив маркер ожидания — Validate на
|
||||
// материале ещё шёл. Это не «успешный прогон без вердикта»: пока Validate идёт,
|
||||
// reconcile ждёт; завершился или пропал, а событие раунд не разбудило —
|
||||
// повторная метка. Маркер обязателен: без него правило прежнее (escalate),
|
||||
// иначе любой успешный прогон без вердикта будил бы модель второй раз.
|
||||
test('#636 pending marker: running Validate waits, finished Validate retries, no marker still escalates', () => {
|
||||
const pending = { schema: 1, issue: 636, material_sha: 'a'.repeat(40) };
|
||||
const waiting = decideReconciliation({
|
||||
issue: issue(), request, runs: [run({ conclusion: 'success', pending, pendingValidate: 'active' })], now: NOW,
|
||||
});
|
||||
assert.equal(waiting.action, 'wait');
|
||||
assert.match(waiting.reason, /still running/);
|
||||
|
||||
for (const state of ['completed', 'missing']) {
|
||||
const done = decideReconciliation({
|
||||
issue: issue(), request, runs: [run({ conclusion: 'success', pending, pendingValidate: state })], now: NOW,
|
||||
});
|
||||
assert.equal(done.action, 'retry', state);
|
||||
assert.match(done.reason, /was not resumed/);
|
||||
}
|
||||
|
||||
const noMarker = decideReconciliation({ issue: issue(), request, runs: [run({ conclusion: 'success' })], now: NOW });
|
||||
assert.equal(noMarker.action, 'escalate');
|
||||
const parsed = parseProcessRun({ display_title: 'process #555 · S7-code-review · x', id: 1, pending, pendingValidate: 'active' });
|
||||
assert.equal(parsed.pending, pending);
|
||||
assert.equal(parsed.pendingValidate, 'active');
|
||||
});
|
||||
|
||||
test('#636 validateStateOnMaterial reads only dispatch runs and prefers active over completed', () => {
|
||||
assert.equal(validateStateOnMaterial([]), 'missing');
|
||||
assert.equal(validateStateOnMaterial([{ event: 'push', status: 'completed' }]), 'missing');
|
||||
assert.equal(validateStateOnMaterial([{ event: 'workflow_dispatch', status: 'completed' }]), 'completed');
|
||||
assert.equal(validateStateOnMaterial([
|
||||
{ event: 'workflow_dispatch', status: 'completed' }, { event: 'workflow_dispatch', status: 'in_progress' },
|
||||
]), 'active');
|
||||
assert.equal(validateStateOnMaterial([{ event: 'workflow_dispatch', status: 'queued' }]), 'active');
|
||||
});
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
// #636: раунд ревью продолжается по событию завершения Validate, а не сном
|
||||
// раннера. Решение чистое: будить можно только раунд, который сам оставил
|
||||
// маркер ожидания на этот материал, при стоящей метке S7 и без активного
|
||||
// прогона конвейера. Всё остальное — noop: лишняя метка = второй вызов модели.
|
||||
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';
|
||||
|
||||
const SHA = 'a'.repeat(40);
|
||||
const OTHER = 'b'.repeat(40);
|
||||
const validateRun = (over = {}) => ({ event: 'workflow_dispatch', status: 'completed', headSha: SHA, ...over });
|
||||
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 pendingOf = (run) => (run.id === 70 ? pending : null);
|
||||
|
||||
test('#636 branch → issue number', () => {
|
||||
assert.equal(issueNumberFromBranch('issue/636-review-wait-event'), 636);
|
||||
assert.equal(issueNumberFromBranch('issue/7-x'), 7);
|
||||
assert.equal(issueNumberFromBranch('dev'), null);
|
||||
assert.equal(issueNumberFromBranch('issue/abc-x'), null);
|
||||
assert.equal(issueNumberFromBranch(''), null);
|
||||
});
|
||||
|
||||
test('#636 the round that waited for exactly this Validate is resumed', () => {
|
||||
const decision = decideResume({ labels: ['bug', REVIEW_LABEL], validateRun: validateRun(), sha: SHA, runs: [processRun()], pendingOf });
|
||||
assert.equal(decision.action, 'resume');
|
||||
assert.equal(decision.run.id, 70);
|
||||
});
|
||||
|
||||
test('#636 push runs, unfinished runs and foreign SHAs never wake a round', () => {
|
||||
const base = { labels: [REVIEW_LABEL], sha: SHA, runs: [processRun()], pendingOf };
|
||||
assert.equal(decideResume({ ...base, validateRun: validateRun({ event: 'push' }) }).action, 'noop');
|
||||
assert.equal(decideResume({ ...base, validateRun: validateRun({ status: 'in_progress' }) }).action, 'noop');
|
||||
assert.equal(decideResume({ ...base, validateRun: validateRun({ headSha: OTHER }) }).action, 'noop');
|
||||
});
|
||||
|
||||
test('#636 label state gates the wake-up: no S7, blocked or review-4 → noop', () => {
|
||||
const base = { validateRun: validateRun(), sha: SHA, runs: [processRun()], pendingOf };
|
||||
assert.equal(decideResume({ ...base, labels: ['S6-in-progress'] }).action, 'noop');
|
||||
assert.equal(decideResume({ ...base, labels: [REVIEW_LABEL, 'blocked'] }).action, 'noop');
|
||||
assert.equal(decideResume({ ...base, labels: [REVIEW_LABEL, 'review-4'] }).action, 'noop');
|
||||
});
|
||||
|
||||
test('#636 an active process run, a run without a pending marker or a marker for another SHA → noop', () => {
|
||||
const base = { labels: [REVIEW_LABEL], validateRun: validateRun(), sha: SHA };
|
||||
const active = decideResume({ ...base, runs: [processRun({ id: 71, status: 'in_progress', conclusion: '', createdAt: '2026-09-23T10:05:00Z' }), processRun()], pendingOf });
|
||||
assert.equal(active.action, 'noop');
|
||||
assert.match(active.reason, /already active/);
|
||||
const noMarker = decideResume({ ...base, runs: [processRun()], pendingOf: () => null });
|
||||
assert.equal(noMarker.action, 'noop');
|
||||
assert.match(noMarker.reason, /no pending marker/);
|
||||
const otherSha = decideResume({ ...base, runs: [processRun()], pendingOf: () => ({ ...pending, material_sha: OTHER }) });
|
||||
assert.equal(otherSha.action, 'noop');
|
||||
assert.match(otherSha.reason, /waits for bbbbbbbb/);
|
||||
const failed = decideResume({ ...base, runs: [processRun({ conclusion: 'failure' })], pendingOf });
|
||||
assert.equal(failed.action, 'noop');
|
||||
const none = decideResume({ ...base, runs: [], pendingOf });
|
||||
assert.match(none.reason, /reconcile owns lost requests/);
|
||||
});
|
||||
|
||||
test('#636 the newest process run decides, not an older pending one', () => {
|
||||
const older = processRun({ id: 60, createdAt: '2026-09-23T09:00:00Z' });
|
||||
const newerReturned = processRun({ id: 71, conclusion: 'failure', createdAt: '2026-09-23T11:00:00Z' });
|
||||
const decision = decideResume({
|
||||
labels: [REVIEW_LABEL], validateRun: validateRun(), sha: SHA, runs: [older, newerReturned],
|
||||
pendingOf: (run) => (run.id === 60 ? pending : null),
|
||||
});
|
||||
assert.equal(decision.action, 'noop');
|
||||
});
|
||||
|
||||
test('#636 resume() relabels only on a resume decision and only with apply', async () => {
|
||||
const relabels = [];
|
||||
const ops = {
|
||||
labels: () => [REVIEW_LABEL],
|
||||
runs: () => [processRun()],
|
||||
pendingOf,
|
||||
relabel: () => relabels.push(REVIEW_LABEL),
|
||||
};
|
||||
const applied = await resume({ repo: 'o/r', branch: 'issue/636-x', sha: SHA, validateRun: validateRun(), ops });
|
||||
assert.equal(applied.action, 'resume');
|
||||
assert.equal(applied.applied, true);
|
||||
assert.deepEqual(relabels, [REVIEW_LABEL]);
|
||||
const dry = await resume({ repo: 'o/r', branch: 'issue/636-x', sha: SHA, validateRun: validateRun(), ops, apply: false });
|
||||
assert.equal(dry.applied, false);
|
||||
assert.deepEqual(relabels, [REVIEW_LABEL], 'dry run does not relabel');
|
||||
const foreign = await resume({ repo: 'o/r', branch: 'dev', sha: SHA, validateRun: validateRun(), ops });
|
||||
assert.equal(foreign.action, 'noop');
|
||||
assert.equal(foreign.issue, null);
|
||||
});
|
||||
|
||||
test('#636 workflows: prepare exits pending with a sealed marker, resume relabels by the marker, preflight mirrors the new file', () => {
|
||||
const process = readFileSync(new URL('../.github/workflows/process.yml', import.meta.url), 'utf8');
|
||||
const resumeWf = readFileSync(new URL('../.github/workflows/process-resume.yml', import.meta.url), 'utf8');
|
||||
const validate = readFileSync(new URL('../.github/workflows/validate.yml', import.meta.url), 'utf8');
|
||||
assert.match(process, /validate-gate\.mjs --repo="\$\{\{ github\.repository \}\}" --ref="\$BRANCH" --sha="\$SHA" --no-wait/);
|
||||
assert.match(process, /2\) echo 'proceed=pending' >> "\$GITHUB_OUTPUT"/);
|
||||
assert.match(process, /review-pending-\$\{NUM\}-\$\{GITHUB_RUN_ID\}-\$\{GITHUB_RUN_ATTEMPT\}/);
|
||||
assert.match(process, /sha256sum pending\.json > manifest\.sha256/);
|
||||
assert.match(process, /Validate красный — вернуть автору без ревью\n\s+if: steps\.rebase\.outputs\.conflict != 'true' && steps\.gate\.outputs\.proceed == 'false'/);
|
||||
assert.match(resumeWf, /workflow_run:\n\s+workflows: \["Проверка \(CI\)"\]\n\s+types: \[completed\]/);
|
||||
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.ok(!/issues: write/.test(resumeWf), 'resume relabels with HP_PROCESS_TOKEN only');
|
||||
assert.match(validate, /for file in process\.yml mutation-gate\.yml process-resume\.yml; do/);
|
||||
assert.equal(readFileSync(new URL('../.github/workflows/validate.yml', import.meta.url), 'utf8').includes('name: Проверка (CI)'), true, 'workflow_run listens to the Validate workflow name');
|
||||
});
|
||||
@@ -628,7 +628,8 @@ test('#510 AC2: конвейер запускает Validate с мутантам
|
||||
assert.match(gateStep, /\{ echo 'proceed=true'; echo 'result=skipped'; \}/, 'skipped = proceed');
|
||||
assert.doesNotMatch(workflow.slice(modelJob), /steps\.gate\.outputs/, 'следующие jobs не читают локальные outputs prepare');
|
||||
const backStep = workflow.slice(back, deps);
|
||||
assert.match(backStep, /if: steps\.rebase\.outputs\.conflict != 'true' && steps\.gate\.outputs\.proceed != 'true'/);
|
||||
// #636: третий исход гейта — pending (Validate идёт); возврат автору только на явном false
|
||||
assert.match(backStep, /if: steps\.rebase\.outputs\.conflict != 'true' && steps\.gate\.outputs\.proceed == 'false'/);
|
||||
assert.match(backStep, /--add-label S6-in-progress --remove-label S7-code-review/);
|
||||
assert.match(backStep, /цикл ревью не израсходован/);
|
||||
assert.match(workflow.slice(modelJob, deps), /if: needs\.prepare\.outputs\.proceed == 'true'/,
|
||||
|
||||
@@ -170,3 +170,44 @@ test('#510 r1 M1: a cancelled dispatch with no replacement gets one dispatch, no
|
||||
assert.equal(outcome.result, 'green');
|
||||
assert.deepEqual(fake.dispatched, ['issue/1']);
|
||||
});
|
||||
|
||||
// #636: раннер конвейера не ждёт Validate внутри job. С `wait: false` гейт
|
||||
// возвращает завершённый прогон как раньше, а идущий — `pending`, не поллит его;
|
||||
// прогон, который ещё не появился, гейт всё же диспатчит и дожидается его
|
||||
// появления на материале (#539), потому что иначе событию завершения нечего
|
||||
// будить.
|
||||
test('#636: без ожидания завершённый зелёный dispatch принимается сразу, как и красный', async () => {
|
||||
const green = fakeOps({ snapshots: [[run()]] });
|
||||
assert.equal((await validateGate({ ref: 'issue/1', sha: SHA, ops: green.ops, wait: false })).result, 'green');
|
||||
const red = fakeOps({ snapshots: [[run({ conclusion: 'failure', url: 'https://run/red' })]] });
|
||||
assert.equal((await validateGate({ ref: 'issue/1', sha: SHA, ops: red.ops, wait: false })).result, 'failed');
|
||||
assert.deepEqual(green.dispatched, []);
|
||||
});
|
||||
|
||||
test('#636: идущий dispatch на материале — pending с его id и url, без единого sleep', async () => {
|
||||
const fake = fakeOps({ snapshots: [[run({ status: 'in_progress', conclusion: null, url: 'https://run/live', databaseId: 42 })]] });
|
||||
const outcome = await validateGate({ ref: 'issue/1', sha: SHA, ops: fake.ops, wait: false, pollMs: 1000 });
|
||||
assert.equal(outcome.result, 'pending');
|
||||
assert.equal(outcome.runId, 42);
|
||||
assert.equal(outcome.url, 'https://run/live');
|
||||
assert.equal(fake.ops.now(), 0, 'гейт не спал');
|
||||
assert.deepEqual(fake.dispatched, []);
|
||||
});
|
||||
|
||||
test('#636: без прогона гейт диспатчит, ждёт появления и возвращает pending, не завершение', async () => {
|
||||
const pushOnly = [run({ event: 'push', databaseId: 7 })];
|
||||
const live = [...pushOnly, run({ status: 'queued', conclusion: null, databaseId: 9 })];
|
||||
const fake = fakeOps({ snapshots: [pushOnly, pushOnly, live, [...pushOnly, run({ databaseId: 9 })]] });
|
||||
const outcome = await validateGate({ ref: 'issue/1', sha: SHA, ops: fake.ops, wait: false, pollMs: 1000 });
|
||||
assert.equal(outcome.result, 'pending');
|
||||
assert.equal(outcome.runId, 9);
|
||||
assert.deepEqual(fake.dispatched, ['issue/1']);
|
||||
assert.equal(fake.calls(), 3, 'остановился на первом снимке с прогоном, до его завершения не дошёл');
|
||||
});
|
||||
|
||||
test('#636: с ожиданием (умолчание) поведение прежнее — идущий прогон дожидается', async () => {
|
||||
const fake = fakeOps({ snapshots: [[run({ status: 'in_progress', conclusion: null })], [run()]] });
|
||||
const outcome = await validateGate({ ref: 'issue/1', sha: SHA, ops: fake.ops, pollMs: 1000 });
|
||||
assert.equal(outcome.result, 'green');
|
||||
assert.ok(fake.ops.now() > 0, 'один poll прошёл');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user