From e1d14e69fbdbc753429e4201ab9ab80d25c8e209 Mon Sep 17 00:00:00 2001 From: firejune Date: Mon, 9 Nov 2026 21:22:24 +0900 Subject: [PATCH 1/2] fix: refresh review heads after record ingestion Co-Authored-By: Jinni <342598851+fj-jinni[bot]@users.noreply.github.com> Co-Authored-By: Naru <342599188+fj-naru[bot]@users.noreply.github.com> --- src/job-host.ts | 29 +-- tests/job-refresh.test.ts | 434 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 449 insertions(+), 14 deletions(-) create mode 100644 tests/job-refresh.test.ts diff --git a/src/job-host.ts b/src/job-host.ts index bae805c..74dda83 100644 --- a/src/job-host.ts +++ b/src/job-host.ts @@ -322,11 +322,11 @@ function selectorLabel(selector: JobSelector): string { } /** - * After `job open`: show the record's current draft on the owner's page. The latest - * recorded draft (or `--draft `) is published unless the job's head publication is - * already that draft over the current record bytes, and every subject the publication - * declares is asked once. An already-acknowledged `rig` or `post` head also retries any - * unasked declared subject without being overwritten by a draft. Returns the lines `job open` prints. + * After `job open`: show or refresh the record's current publication on the owner's page. + * The latest recorded draft (or `--draft `), or the head's `rig` / `post` selector at + * later stages, is published unless the job's head publication already covers that selector + * over the current record bytes, and every subject the publication declares is asked once. + * Returns the lines `job open` prints. */ export async function openDraftPublication(root: string, host: JobOwnerHost, now: number, requested?: string): Promise { root = realpathSync(root); @@ -337,12 +337,13 @@ export async function openDraftPublication(root: string, host: JobOwnerHost, now const source = head?.source as { selector?: JobSelector; projectHash?: unknown } | undefined; const acknowledged = head ? readAcknowledgedPublication(root, host.session.jobId, head.publicationId) : null; const headSelector = acknowledged?.result.selector ?? source?.selector ?? null; - const reuseHead = Boolean(head && headSelector && ( - requested !== undefined - ? isDeepStrictEqual(headSelector, { kind: 'draft', id: requested }) && source?.projectHash === jobHash(bytes) - : headSelector.kind !== 'draft' - ? draftId === undefined || source?.projectHash === jobHash(bytes) - : source?.projectHash === jobHash(bytes) && (draftId === undefined || headSelector.id === draftId) + const targetSelector: JobSelector | undefined = draftId !== undefined + ? { kind: 'draft', id: draftId } + : headSelector && headSelector.kind !== 'draft' + ? headSelector + : undefined; + const reuseHead = Boolean(head && headSelector && source?.projectHash === jobHash(bytes) && ( + targetSelector === undefined || isDeepStrictEqual(headSelector, targetSelector) )); let selector: JobSelector; let outcomes: AskOutcome[]; @@ -355,10 +356,10 @@ export async function openDraftPublication(root: string, host: JobOwnerHost, now selector = headSelector; published = false; } else { - if (draftId === undefined) { + if (targetSelector === undefined) { return ids.length === 0 ? ['job: no recorded draft to publish'] : [`job: stage ${stage} shows no draft; name one with --draft `]; } - const result = draftPublicationResult(draftId); + const result = selectorResult(targetSelector); const release = acquireJobLock(root)!; try { const binding = readWriterBinding(root); @@ -366,7 +367,7 @@ export async function openDraftPublication(root: string, host: JobOwnerHost, now enqueuePublication(root, binding, result); ({ outcomes } = await reconcileLocked(root, publishPending)); } finally { release(); } - selector = result.selector; + selector = targetSelector; published = true; } const lines = [`job: ${selectorLabel(selector)} ${published ? 'published' : 'already published'}`]; diff --git a/tests/job-refresh.test.ts b/tests/job-refresh.test.ts new file mode 100644 index 0000000..04b3b67 --- /dev/null +++ b/tests/job-refresh.test.ts @@ -0,0 +1,434 @@ +import { afterEach, expect, test } from 'bun:test'; +import { randomUUID } from 'node:crypto'; +import { cpSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { checkProject } from '../src/check.ts'; +import { jobHash, jobHostStateRoot, jobPrivateRoot, pendingPublications, readWriterBinding } from '../src/job-outbox.ts'; +import type { JsonObject } from '../src/templates.ts'; + +const CLI = join(import.meta.dir, '../src/cli.ts'); +const WORDS = ' Synthetic owner review words.\r\n'; +const roots: string[] = []; +const opened: { proc: Bun.Subprocess; root: string }[] = []; + +afterEach(async () => { + for (const { proc, root } of opened.splice(0)) { + if (readWriterBinding(root) && pendingPublications(root).length === 0) { + await cli(root, ['job', 'close', root]); + } + if (proc.exitCode === null) proc.kill(); + await proc.exited; + } + for (const root of roots.splice(0)) { + rmSync(jobPrivateRoot(root), { recursive: true, force: true }); + rmSync(jobHostStateRoot(root), { recursive: true, force: true }); + rmSync(root, { recursive: true, force: true }); + } +}); + +const read = (root: string): JsonObject => JSON.parse(readFileSync(join(root, 'project.json'), 'utf8')) as JsonObject; +const write = (root: string, project: JsonObject): void => writeFileSync(join(root, 'project.json'), `${JSON.stringify(project, null, 2)}\n`); + +function copy(checkpoint: string, edit?: (project: JsonObject) => void): string { + const root = realpathSync(mkdtempSync(join(tmpdir(), 'create-job-refresh-'))); + roots.push(root); + const src = checkpoint === 'example-chain' + ? join(import.meta.dir, '../templates/example-chain') + : join(import.meta.dir, '../templates/example-chain/checkpoints', checkpoint); + cpSync(src, root, { recursive: true }); + if (edit) { + const project = read(root); + edit(project); + write(root, project); + } + return root; +} + +async function cli(root: string, args: string[]): Promise<{ stdout: string; stderr: string; exit: number }> { + const child = Bun.spawn([process.execPath, CLI, ...args], { cwd: root, stdout: 'pipe', stderr: 'pipe' }); + const [stdout, stderr, exit] = await Promise.all([new Response(child.stdout).text(), new Response(child.stderr).text(), child.exited]); + return { stdout, stderr, exit }; +} + +async function open(root: string): Promise<{ url: URL; lines: string[] }> { + const proc = Bun.spawn([process.execPath, CLI, 'job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner'], { + cwd: root, + stdout: 'pipe', + stderr: 'pipe', + }); + opened.push({ proc, root }); + const reader = proc.stdout.getReader(); + const decoder = new TextDecoder(); + let text = ''; + while (!/\njob: .*\n$/.test(text)) { + const chunk = await reader.read(); + if (chunk.done) break; + text += decoder.decode(chunk.value); + } + reader.releaseLock(); + const lines = text.trim().split('\n'); + return { url: new URL(lines[0]!), lines: lines.slice(1) }; +} + +async function page(url: URL, expectedAsks = 2) { + const jobId = /\/jobs\/([^/]+)\//.exec(url.pathname)![1]!; + const capability = url.hash.replace('#cap=', ''); + const base = `${url.origin}/jobs/${jobId}/`; + const headers = { origin: url.origin, 'X-Review-Capability': capability, 'content-type': 'application/json' }; + const abort = new AbortController(); + const response = await fetch(base + 'events', { headers, signal: abort.signal }); + const reader = response.body!.getReader(); + const decoder = new TextDecoder(); + let buffered = ''; + const events: JsonObject[] = []; + const pending = () => { + const head = events.filter(event => event.type === 'published').at(-1)?.publication as JsonObject | undefined; + return { + head, + asks: events + .filter(event => event.type === 'pending' && (event.ask as JsonObject).publicationId === head?.publicationId) + .map(event => event.ask as JsonObject), + }; + }; + const deadline = Date.now() + 10_000; + while (Date.now() < deadline && pending().asks.length < expectedAsks) { + const next = await reader.read(); + if (next.done) break; + buffered += decoder.decode(next.value, { stream: true }); + let newline: number; + while ((newline = buffered.indexOf('\n')) >= 0) { + events.push(JSON.parse(buffered.slice(0, newline))); + buffered = buffered.slice(newline + 1); + } + } + abort.abort(); + const { head, asks } = pending(); + const post = async (route: string, body: object) => + (await fetch(base + route, { method: 'POST', headers, body: JSON.stringify(body) })).json() as Promise; + const pageId = randomUUID(); + const displayed = await post('ui/displayed', { + publicationId: head!.publicationId, + pageId, + sourceHash: (head!.source as JsonObject).sourceHash, + }); + let expectedGeneration = Number(events.at(-1)!.generation); + return { + pageId, + head: head!, + asks, + tryAct: async (subject: string, kind: string, value: object, extra: object = {}): Promise => { + const ask = asks.find(item => (item.subject as JsonObject).kind === subject)!; + return await post('ui/acts', { + publicationId: head!.publicationId, + askId: ask.askId, + pageId, + displayedNonce: (displayed.value as JsonObject).displayedNonce, + expectedGeneration, + submissionId: randomUUID(), + kind, + value, + ...extra, + }); + }, + act: async (subject: string, kind: string, value: object, extra: object = {}): Promise => { + const ask = asks.find(item => (item.subject as JsonObject).kind === subject)!; + const result = await post('ui/acts', { + publicationId: head!.publicationId, + askId: ask.askId, + pageId, + displayedNonce: (displayed.value as JsonObject).displayedNonce, + expectedGeneration, + submissionId: randomUUID(), + kind, + value, + ...extra, + }); + expect(result.error).toBeUndefined(); + expectedGeneration++; + return result.value as JsonObject; + }, + }; +} + +function rigCapture(root: string, view: Awaited>, askId: string): object { + const png = readFileSync(join(root, 'evidence/setup.png')); + return { + capture: { + png: `data:image/png;base64,${png.toString('base64')}`, + width: png.readUInt32BE(16), + height: png.readUInt32BE(20), + camera: 'fixed', + framing: 'default', + view: 'whole-rig', + frame: 'setup', + playing: false, + binding: { + publicationId: view.head.publicationId, + instanceKey: view.head.instanceKey, + sourceHash: (view.head.source as JsonObject).sourceHash, + pageId: view.pageId, + askId, + drawGeneration: 1, + playerGeneration: 0, + }, + }, + }; +} + +test('parts-review verdict -> ingest -> triage -> job open refresh -> UI signoff -> ingest succeeds after JOB_RECORD_MOVED on the pre-triage publication', async () => { + const root = copy('1-parts-verdict'); + cpSync(join(import.meta.dir, '../templates/example-chain/evidence/diff-components.json'), join(root, 'evidence/diff-components.json')); + expect((await cli(root, ['advance', 'parts-review', root, '--parts-check', 'evidence/parts-check.json', '--parts', 'derived/drafts/chain/input'])).exit).toBe(0); + + const opening = await open(root); + expect(opening.lines).toEqual(['job: draft chain published', 'job: asked parts-verdict chain, signoff parts-review']); + + const firstView = await page(opening.url); + const names = (JSON.parse(readFileSync(join(root, 'derived/drafts/chain/input/parts.json'), 'utf8')).parts as { name: string }[]).map(part => part.name); + const verdict = await firstView.act('parts-verdict', 'parts-verdict', { + verdict: 'approved', + words: WORDS, + parts: names.map(partId => ({ partId, part: { outcome: 'clean', note: '' }, mesh: { outcome: 'clean', note: '' } })), + }); + + expect((await cli(root, ['parts-review', root, '--from-review', String(verdict.actId), '--json'])).exit).toBe(0); + expect((await cli(root, ['structure-snapshot', root, '--draft', 'chain', '--out', 'evidence/structure.json'])).exit).toBe(0); + expect((await cli(root, ['diff-classes', root, '--diff-report', 'evidence/diff-components.json', '--by', 'agent'])).exit).toBe(0); + + // The pre-ingest publication now has an old projectHash, so UI signoff on it is refused as JOB_RECORD_MOVED. + const staleSignoff = await firstView.tryAct('signoff', 'signoff', { stage: 'parts-review', decision: 'approved', words: WORDS }); + expect((staleSignoff.error as JsonObject | undefined)?.code).toBe('JOB_RECORD_MOVED'); + + // Refresh via job open republishes the draft over the current project.json and re-asks declared subjects. + const refreshed = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); + expect({ exit: refreshed.exit, stderr: refreshed.stderr }).toEqual({ exit: 0, stderr: '' }); + expect(refreshed.stdout.trim().split('\n')).toEqual([ + opening.url.href, + 'job: draft chain published', + 'job: asked parts-verdict chain, signoff parts-review', + ]); + + const refreshedView = await page(opening.url); + expect(refreshedView.head.publicationId).not.toBe(firstView.head.publicationId); + expect((refreshedView.head.source as JsonObject).selector).toEqual({ kind: 'draft', id: 'chain' }); + expect((refreshedView.head.source as JsonObject).projectHash).toBe(jobHash(readFileSync(join(root, 'project.json')))); + + const signed = await refreshedView.act('signoff', 'signoff', { stage: 'parts-review', decision: 'approved', words: WORDS }); + const signoffRes = await cli(root, ['signoff', 'parts-review', root, '--from-review', String(signed.actId), '--structure-file', 'evidence/structure.json', '--json']); + expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expect((read(root).signoffs as JsonObject[]).at(-1)).toMatchObject({ + stage: 'parts-review', + checkpoint: null, + by: 'user', + verbatim: WORDS, + draft_id: 'chain', + source: `review act ${String(signed.jobId)}/${String(signed.actId)}`, + }); +}); + +test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest preserves rig selector, updates projectHash, and keeps stale/authority guards', async () => { + const root = copy('3-rig-review', project => { + project.stage = 'rigging'; + project.advances = (project.advances as JsonObject[]).filter(entry => entry.stage !== 'rig-review'); + }); + const opening = await open(root); + expect((await cli(root, ['advance', 'rig-review', root, '--json'])).exit).toBe(0); + + const firstView = await page(opening.url); + expect((firstView.head.source as JsonObject).selector).toEqual({ kind: 'rig', version: 1 }); + const verdictAsk = firstView.asks.find(item => (item.subject as JsonObject).kind === 'review-verdict')!; + const verdict = await firstView.act( + 'review-verdict', + 'review-verdict', + { stage: 'rig-review', verdict: 'approved', words: WORDS }, + rigCapture(root, firstView, String(verdictAsk.askId)), + ); + + const reviewed = await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json']); + expect({ exit: reviewed.exit, stderr: reviewed.stderr }).toEqual({ exit: 0, stderr: '' }); + + // Submitting signoff on the pre-verdict publication fails with JOB_RECORD_MOVED because review modified project.json. + const staleSignoff = await firstView.tryAct('signoff', 'signoff', { stage: 'rig-review', decision: 'approved', words: WORDS }); + expect((staleSignoff.error as JsonObject | undefined)?.code).toBe('JOB_RECORD_MOVED'); + + // Refresh via job open republishes rig v1 with the current project.json hash and asks review-verdict and signoff. + const refreshed = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); + expect({ exit: refreshed.exit, stderr: refreshed.stderr }).toEqual({ exit: 0, stderr: '' }); + expect(refreshed.stdout.trim().split('\n')).toEqual([ + opening.url.href, + 'job: rig v1 published', + 'job: asked review-verdict rig-review, signoff rig-review', + ]); + + // Re-opening again when project.json has not moved reuses the refreshed head without republishing. + const reused = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); + expect({ exit: reused.exit, stderr: reused.stderr }).toEqual({ exit: 0, stderr: '' }); + expect(reused.stdout.trim().split('\n')).toEqual([ + opening.url.href, + 'job: rig v1 already published', + 'job: already asked review-verdict rig-review, signoff rig-review', + ]); + + const refreshedView = await page(opening.url); + expect(refreshedView.head.publicationId).not.toBe(firstView.head.publicationId); + expect((refreshedView.head.source as JsonObject).selector).toEqual({ kind: 'rig', version: 1 }); + expect((refreshedView.head.source as JsonObject).projectHash).toBe(jobHash(readFileSync(join(root, 'project.json')))); + expect(refreshedView.asks.map(ask => ask.subject)).toEqual([ + { kind: 'review-verdict', stage: 'rig-review' }, + { kind: 'signoff', stage: 'rig-review' }, + ]); + + const signed = await refreshedView.act('signoff', 'signoff', { stage: 'rig-review', decision: 'approved', words: WORDS }); + + // Negative guards: wrong version, wrong stage, or competing authority flags refuse and leave project.json byte-identical. + const beforeSignoffBytes = readFileSync(join(root, 'project.json')); + const wrongVersion = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signed.actId), '--version', '2']); + expect(wrongVersion.exit).toBe(1); + expect(wrongVersion.stderr).toContain('REVIEW_ACT_TARGET_MISMATCH:'); + expect(readFileSync(join(root, 'project.json'))).toEqual(beforeSignoffBytes); + + const wrongStage = await cli(root, ['signoff', 'direction-review', root, '--from-review', String(signed.actId)]); + expect(wrongStage.exit).toBe(1); + expect(wrongStage.stderr).toContain('REVIEW_ACT_KIND_MISMATCH:'); + expect(readFileSync(join(root, 'project.json'))).toEqual(beforeSignoffBytes); + + const mixedBy = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signed.actId), '--by', 'user']); + expect(mixedBy.exit).toBe(2); + expect(mixedBy.stderr).toContain('SIGNOFF_OPTION_INVALID:'); + expect(readFileSync(join(root, 'project.json'))).toEqual(beforeSignoffBytes); + + // Valid signoff ingest succeeds and records target.version = 1. + const signoffRes = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signed.actId), '--json']); + expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expect((read(root).signoffs as JsonObject[]).at(-1)).toMatchObject({ + stage: 'rig-review', + checkpoint: null, + by: 'user', + verbatim: WORDS, + target: { version: 1 }, + source: `review act ${String(signed.jobId)}/${String(signed.actId)}`, + }); + + // Replay of the earlier verdict act still succeeds without rewriting project.json, and corrupting a bound source refuses REVIEW_ACT_SOURCE_STALE. + const afterSignoffBytes = readFileSync(join(root, 'project.json')); + const replayVerdict = await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json']); + expect({ exit: replayVerdict.exit, stderr: replayVerdict.stderr }).toEqual({ exit: 0, stderr: '' }); + expect(readFileSync(join(root, 'project.json'))).toEqual(afterSignoffBytes); + + const modelPath = join(root, 'derived/review/v1/out/check/build/skeleton.model.json'); + const savedModel = readFileSync(modelPath); + writeFileSync(modelPath, '{}\n'); + try { + const staleReplay = await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json']); + expect(staleReplay.exit).toBe(1); + expect(staleReplay.stderr).toContain('REVIEW_ACT_SOURCE_STALE:'); + expect(readFileSync(join(root, 'project.json'))).toEqual(afterSignoffBytes); + } finally { + writeFileSync(modelPath, savedModel); + } + + const report = checkProject(root, undefined, { now: Date.now() }); + expect(report.refusals.map(f => f.code)).toEqual(['STAGE_LAGGING']); + expect(report.holes ?? []).toEqual([]); +}); + +test('direction-review verdict -> ingest -> job open refresh -> UI signoff -> ingest -> advance accepted completes cleanly', async () => { + const root = copy('example-chain', project => { + project.stage = 'post'; + project.advances = (project.advances as JsonObject[]).filter(entry => entry.stage !== 'direction-review' && entry.stage !== 'accepted'); + project.signoffs = (project.signoffs as JsonObject[]).filter(entry => entry.stage !== 'direction-review'); + const postVersion = (project.versions as JsonObject[]).find(v => v.n === 2)!; + postVersion.verdicts = []; + }); + const opening = await open(root); + expect((await cli(root, ['advance', 'direction-review', root, '--json'])).exit).toBe(0); + + const firstView = await page(opening.url); + expect((firstView.head.source as JsonObject).selector).toEqual({ kind: 'post', version: 2, rigVersion: 1 }); + const verdict = await firstView.act('review-verdict', 'review-verdict', { stage: 'direction-review', verdict: 'approved', words: WORDS }); + + const reviewed = await cli(root, ['review', '2', root, '--from-review', String(verdict.actId), '--json']); + expect({ exit: reviewed.exit, stderr: reviewed.stderr }).toEqual({ exit: 0, stderr: '' }); + + // Pre-verdict publication rejects subsequent UI signoff with JOB_RECORD_MOVED. + const staleSignoff = await firstView.tryAct('signoff', 'signoff', { stage: 'direction-review', decision: 'approved', words: WORDS }); + expect((staleSignoff.error as JsonObject | undefined)?.code).toBe('JOB_RECORD_MOVED'); + + // Refresh via job open republishes post v2 over the current project.json and asks review-verdict and signoff. + const refreshed = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); + expect({ exit: refreshed.exit, stderr: refreshed.stderr }).toEqual({ exit: 0, stderr: '' }); + expect(refreshed.stdout.trim().split('\n')).toEqual([ + opening.url.href, + 'job: post v2 published', + 'job: asked review-verdict direction-review, signoff direction-review', + ]); + + const refreshedView = await page(opening.url); + expect(refreshedView.head.publicationId).not.toBe(firstView.head.publicationId); + expect((refreshedView.head.source as JsonObject).selector).toEqual({ kind: 'post', version: 2, rigVersion: 1 }); + expect((refreshedView.head.source as JsonObject).projectHash).toBe(jobHash(readFileSync(join(root, 'project.json')))); + + const signed = await refreshedView.act('signoff', 'signoff', { stage: 'direction-review', decision: 'approved', words: WORDS }); + const signoffRes = await cli(root, ['signoff', 'direction-review', root, '--from-review', String(signed.actId), '--json']); + expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + + expect((await cli(root, ['advance', 'accepted', root, '--json'])).exit).toBe(0); + const report = checkProject(root, undefined, { now: Date.now() }); + expect(report.refusals).toEqual([]); + expect(report.warnings).toEqual([]); + expect(report.holes ?? []).toEqual([]); +}); + +test('both-acts-before-ingest remains supported without an intermediate refresh, and pending refresh failure recovers via job reconcile', async () => { + const root = copy('3-rig-review', project => { + project.stage = 'rigging'; + project.advances = (project.advances as JsonObject[]).filter(entry => entry.stage !== 'rig-review'); + }); + const opening = await open(root); + expect((await cli(root, ['advance', 'rig-review', root, '--json'])).exit).toBe(0); + + // Both UI acts submitted before any ingest remain supported. + const firstView = await page(opening.url); + const verdictAsk = firstView.asks.find(item => (item.subject as JsonObject).kind === 'review-verdict')!; + const verdict = await firstView.act( + 'review-verdict', + 'review-verdict', + { stage: 'rig-review', verdict: 'approved', words: WORDS }, + rigCapture(root, firstView, String(verdictAsk.askId)), + ); + const signedBeforeIngest = await firstView.act('signoff', 'signoff', { stage: 'rig-review', decision: 'approved', words: WORDS }); + + expect((await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json'])).exit).toBe(0); + + // Plant a corrupted package source before running job open refresh: leaves JOB_PUBLISH_PENDING in the outbox. + const partsPath = join(root, 'derived/review/v1/out/parts.json'); + const savedParts = readFileSync(partsPath); + writeFileSync(partsPath, '{}\n'); + const failedRefresh = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); + expect(failedRefresh.exit).toBe(1); + expect(failedRefresh.stderr).toContain('JOB_PUBLISH_PENDING:'); + expect(failedRefresh.stderr).toContain('JOB_PUBLICATION_INVALID'); + expect(pendingPublications(root)).toHaveLength(1); + expect(checkProject(root, undefined, { now: Date.now() }).holes?.map(h => h.code)).toEqual(['JOB_PUBLISH_PENDING']); + + // Restore the corrupted file and recover the pending refresh through job reconcile. + writeFileSync(partsPath, savedParts); + const reconciled = await cli(root, ['job', 'reconcile', root]); + expect({ exit: reconciled.exit, stderr: reconciled.stderr, stdout: reconciled.stdout.trim() }).toEqual({ + exit: 0, + stderr: '', + stdout: 'job: reconciled 1 publication(s)', + }); + expect(pendingPublications(root)).toEqual([]); + expect(checkProject(root, undefined, { now: Date.now() }).holes ?? []).toEqual([]); + + // The signoff act finalized before ingest still ingests cleanly after reconciliation. + const signoffRes = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signedBeforeIngest.actId), '--json']); + expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expect((read(root).signoffs as JsonObject[]).at(-1)).toMatchObject({ + stage: 'rig-review', + target: { version: 1 }, + source: `review act ${String(signedBeforeIngest.jobId)}/${String(signedBeforeIngest.actId)}`, + }); +}); From cfec4ad23cadc90d060e757788c6bf3462052579 Mon Sep 17 00:00:00 2001 From: firejune Date: Mon, 9 Nov 2026 21:26:29 +0900 Subject: [PATCH 2/2] test: verify integrated refresh routes with bounded CLI flows Co-Authored-By: Jinni <342598851+fj-jinni[bot]@users.noreply.github.com> Co-Authored-By: Naru <342599188+fj-naru[bot]@users.noreply.github.com> --- docs/LIVE-REVIEW.md | 15 ++++--- tests/job-refresh.test.ts | 84 +++++++++++++++++++++++---------------- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/docs/LIVE-REVIEW.md b/docs/LIVE-REVIEW.md index ade0be6..622d88d 100644 --- a/docs/LIVE-REVIEW.md +++ b/docs/LIVE-REVIEW.md @@ -268,8 +268,13 @@ effective stage). The rules are in `src/signable.ts`: Review 2.7.0 asks only for a declared subject and refuses any other with `JOB_ASK_INVALID` (`bindJobAsk`, `src/job-act.ts`). Both the ask and the act also require `project.json` to still hold the published bytes -(`assertJobSourceCurrent`, `src/job-source.ts`). So the person signs on the -publication of the record that can sign, before any later write. +(`assertJobSourceCurrent`, `src/job-source.ts`). So either the owner submits +both the verdict and `signoff` acts in the Review UI before any writer mutates +`project.json`, or after a CLI writer (`parts-review --from-review`, +`diff-classes`, or `review --from-review`) updates `project.json`, the +agent runs `rig-create job open` to republish the current selector (`draft`, +`rig`, or `post`) over the updated `project.json` bytes and re-ask the stage's +declared subjects before the owner signs off in the UI. `job reconcile` keeps the declared subjects in the acknowledged receipt. `signoff … --from-review` compares the act's subject with that list and @@ -302,9 +307,9 @@ again. | `parts` checkpoints | draft | `draft-rig` commits while the record is in `parts` and the checkpoint is enabled | | `rigging` checkpoints | rig version | `rig-version` commits in `rigging` | | `post` checkpoints | post version | `direction write` or `direction patch` commits in `post` | -| `parts-review` | draft | `draft-rig` commits in `parts-review` | -| `rig-review` | rig version | `advance rig-review` commits the move into `rig-review`, or `rig-version` commits in `rig-review` | -| `direction-review` | post version | `advance direction-review` commits the move into `direction-review`, or `direction write` / `direction patch` commits in `direction-review` | +| `parts-review` | draft | `draft-rig` commits in `parts-review`, or the agent runs `job open` to refresh the draft after `parts-review --from-review` / `diff-classes` | +| `rig-review` | rig version | `advance rig-review` commits the move into `rig-review`, `rig-version` commits in `rig-review`, or the agent runs `job open` to refresh the live `rig` head after `review --from-review` | +| `direction-review` | post version | `advance direction-review` commits the move into `direction-review`, `direction write` / `direction patch` commits in `direction-review`, or the agent runs `job open` to refresh the live `post` head after `review --from-review` | | `rigging-interview`, `post-interview` | any package publication with plan steps | only when a package writer commits in that stage, which the usual order does not do | | `interview`, `plan-approved` | none | no draft or version exists before they are signed | diff --git a/tests/job-refresh.test.ts b/tests/job-refresh.test.ts index 04b3b67..94ae6a4 100644 --- a/tests/job-refresh.test.ts +++ b/tests/job-refresh.test.ts @@ -9,6 +9,8 @@ import type { JsonObject } from '../src/templates.ts'; const CLI = join(import.meta.dir, '../src/cli.ts'); const WORDS = ' Synthetic owner review words.\r\n'; +// Measured 2.59s–3.67s per multi-spawn CLI + live host flow locally; 15_000ms tier avoids 5s default timeout flakes under load. +const REFRESH_FLOW_TIMEOUT_MS = 15_000; const roots: string[] = []; const opened: { proc: Bun.Subprocess; root: string }[] = []; @@ -176,10 +178,14 @@ function rigCapture(root: string, view: Awaited>, askId: }; } +function expectExit0(result: { stdout: string; stderr: string; exit: number }): void { + expect({ exit: result.exit, stderr: result.stderr }).toEqual({ exit: 0, stderr: '' }); +} + test('parts-review verdict -> ingest -> triage -> job open refresh -> UI signoff -> ingest succeeds after JOB_RECORD_MOVED on the pre-triage publication', async () => { const root = copy('1-parts-verdict'); cpSync(join(import.meta.dir, '../templates/example-chain/evidence/diff-components.json'), join(root, 'evidence/diff-components.json')); - expect((await cli(root, ['advance', 'parts-review', root, '--parts-check', 'evidence/parts-check.json', '--parts', 'derived/drafts/chain/input'])).exit).toBe(0); + expectExit0(await cli(root, ['advance', 'parts-review', root, '--parts-check', 'evidence/parts-check.json', '--parts', 'derived/drafts/chain/input'])); const opening = await open(root); expect(opening.lines).toEqual(['job: draft chain published', 'job: asked parts-verdict chain, signoff parts-review']); @@ -192,9 +198,9 @@ test('parts-review verdict -> ingest -> triage -> job open refresh -> UI signoff parts: names.map(partId => ({ partId, part: { outcome: 'clean', note: '' }, mesh: { outcome: 'clean', note: '' } })), }); - expect((await cli(root, ['parts-review', root, '--from-review', String(verdict.actId), '--json'])).exit).toBe(0); - expect((await cli(root, ['structure-snapshot', root, '--draft', 'chain', '--out', 'evidence/structure.json'])).exit).toBe(0); - expect((await cli(root, ['diff-classes', root, '--diff-report', 'evidence/diff-components.json', '--by', 'agent'])).exit).toBe(0); + expectExit0(await cli(root, ['parts-review', root, '--from-review', String(verdict.actId), '--json'])); + expectExit0(await cli(root, ['structure-snapshot', root, '--draft', 'chain', '--out', 'evidence/structure.json'])); + expectExit0(await cli(root, ['diff-classes', root, '--diff-report', 'evidence/diff-components.json', '--by', 'agent'])); // The pre-ingest publication now has an old projectHash, so UI signoff on it is refused as JOB_RECORD_MOVED. const staleSignoff = await firstView.tryAct('signoff', 'signoff', { stage: 'parts-review', decision: 'approved', words: WORDS }); @@ -202,7 +208,7 @@ test('parts-review verdict -> ingest -> triage -> job open refresh -> UI signoff // Refresh via job open republishes the draft over the current project.json and re-asks declared subjects. const refreshed = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); - expect({ exit: refreshed.exit, stderr: refreshed.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(refreshed); expect(refreshed.stdout.trim().split('\n')).toEqual([ opening.url.href, 'job: draft chain published', @@ -216,7 +222,7 @@ test('parts-review verdict -> ingest -> triage -> job open refresh -> UI signoff const signed = await refreshedView.act('signoff', 'signoff', { stage: 'parts-review', decision: 'approved', words: WORDS }); const signoffRes = await cli(root, ['signoff', 'parts-review', root, '--from-review', String(signed.actId), '--structure-file', 'evidence/structure.json', '--json']); - expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(signoffRes); expect((read(root).signoffs as JsonObject[]).at(-1)).toMatchObject({ stage: 'parts-review', checkpoint: null, @@ -225,7 +231,7 @@ test('parts-review verdict -> ingest -> triage -> job open refresh -> UI signoff draft_id: 'chain', source: `review act ${String(signed.jobId)}/${String(signed.actId)}`, }); -}); +}, REFRESH_FLOW_TIMEOUT_MS); test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest preserves rig selector, updates projectHash, and keeps stale/authority guards', async () => { const root = copy('3-rig-review', project => { @@ -233,7 +239,7 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p project.advances = (project.advances as JsonObject[]).filter(entry => entry.stage !== 'rig-review'); }); const opening = await open(root); - expect((await cli(root, ['advance', 'rig-review', root, '--json'])).exit).toBe(0); + expectExit0(await cli(root, ['advance', 'rig-review', root, '--json'])); const firstView = await page(opening.url); expect((firstView.head.source as JsonObject).selector).toEqual({ kind: 'rig', version: 1 }); @@ -246,7 +252,7 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p ); const reviewed = await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json']); - expect({ exit: reviewed.exit, stderr: reviewed.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(reviewed); // Submitting signoff on the pre-verdict publication fails with JOB_RECORD_MOVED because review modified project.json. const staleSignoff = await firstView.tryAct('signoff', 'signoff', { stage: 'rig-review', decision: 'approved', words: WORDS }); @@ -254,7 +260,7 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p // Refresh via job open republishes rig v1 with the current project.json hash and asks review-verdict and signoff. const refreshed = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); - expect({ exit: refreshed.exit, stderr: refreshed.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(refreshed); expect(refreshed.stdout.trim().split('\n')).toEqual([ opening.url.href, 'job: rig v1 published', @@ -263,7 +269,7 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p // Re-opening again when project.json has not moved reuses the refreshed head without republishing. const reused = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); - expect({ exit: reused.exit, stderr: reused.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(reused); expect(reused.stdout.trim().split('\n')).toEqual([ opening.url.href, 'job: rig v1 already published', @@ -284,23 +290,29 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p // Negative guards: wrong version, wrong stage, or competing authority flags refuse and leave project.json byte-identical. const beforeSignoffBytes = readFileSync(join(root, 'project.json')); const wrongVersion = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signed.actId), '--version', '2']); - expect(wrongVersion.exit).toBe(1); - expect(wrongVersion.stderr).toContain('REVIEW_ACT_TARGET_MISMATCH:'); + expect({ exit: wrongVersion.exit, stderr: wrongVersion.stderr }).toEqual({ + exit: 1, + stderr: expect.stringContaining('REVIEW_ACT_TARGET_MISMATCH:'), + }); expect(readFileSync(join(root, 'project.json'))).toEqual(beforeSignoffBytes); const wrongStage = await cli(root, ['signoff', 'direction-review', root, '--from-review', String(signed.actId)]); - expect(wrongStage.exit).toBe(1); - expect(wrongStage.stderr).toContain('REVIEW_ACT_KIND_MISMATCH:'); + expect({ exit: wrongStage.exit, stderr: wrongStage.stderr }).toEqual({ + exit: 1, + stderr: expect.stringContaining('REVIEW_ACT_KIND_MISMATCH:'), + }); expect(readFileSync(join(root, 'project.json'))).toEqual(beforeSignoffBytes); const mixedBy = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signed.actId), '--by', 'user']); - expect(mixedBy.exit).toBe(2); - expect(mixedBy.stderr).toContain('SIGNOFF_OPTION_INVALID:'); + expect({ exit: mixedBy.exit, stderr: mixedBy.stderr }).toEqual({ + exit: 2, + stderr: expect.stringContaining('SIGNOFF_OPTION_INVALID:'), + }); expect(readFileSync(join(root, 'project.json'))).toEqual(beforeSignoffBytes); // Valid signoff ingest succeeds and records target.version = 1. const signoffRes = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signed.actId), '--json']); - expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(signoffRes); expect((read(root).signoffs as JsonObject[]).at(-1)).toMatchObject({ stage: 'rig-review', checkpoint: null, @@ -313,7 +325,7 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p // Replay of the earlier verdict act still succeeds without rewriting project.json, and corrupting a bound source refuses REVIEW_ACT_SOURCE_STALE. const afterSignoffBytes = readFileSync(join(root, 'project.json')); const replayVerdict = await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json']); - expect({ exit: replayVerdict.exit, stderr: replayVerdict.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(replayVerdict); expect(readFileSync(join(root, 'project.json'))).toEqual(afterSignoffBytes); const modelPath = join(root, 'derived/review/v1/out/check/build/skeleton.model.json'); @@ -321,8 +333,10 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p writeFileSync(modelPath, '{}\n'); try { const staleReplay = await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json']); - expect(staleReplay.exit).toBe(1); - expect(staleReplay.stderr).toContain('REVIEW_ACT_SOURCE_STALE:'); + expect({ exit: staleReplay.exit, stderr: staleReplay.stderr }).toEqual({ + exit: 1, + stderr: expect.stringContaining('REVIEW_ACT_SOURCE_STALE:'), + }); expect(readFileSync(join(root, 'project.json'))).toEqual(afterSignoffBytes); } finally { writeFileSync(modelPath, savedModel); @@ -331,7 +345,7 @@ test('rig-review verdict -> ingest -> job open refresh -> UI signoff -> ingest p const report = checkProject(root, undefined, { now: Date.now() }); expect(report.refusals.map(f => f.code)).toEqual(['STAGE_LAGGING']); expect(report.holes ?? []).toEqual([]); -}); +}, REFRESH_FLOW_TIMEOUT_MS); test('direction-review verdict -> ingest -> job open refresh -> UI signoff -> ingest -> advance accepted completes cleanly', async () => { const root = copy('example-chain', project => { @@ -342,14 +356,14 @@ test('direction-review verdict -> ingest -> job open refresh -> UI signoff -> in postVersion.verdicts = []; }); const opening = await open(root); - expect((await cli(root, ['advance', 'direction-review', root, '--json'])).exit).toBe(0); + expectExit0(await cli(root, ['advance', 'direction-review', root, '--json'])); const firstView = await page(opening.url); expect((firstView.head.source as JsonObject).selector).toEqual({ kind: 'post', version: 2, rigVersion: 1 }); const verdict = await firstView.act('review-verdict', 'review-verdict', { stage: 'direction-review', verdict: 'approved', words: WORDS }); const reviewed = await cli(root, ['review', '2', root, '--from-review', String(verdict.actId), '--json']); - expect({ exit: reviewed.exit, stderr: reviewed.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(reviewed); // Pre-verdict publication rejects subsequent UI signoff with JOB_RECORD_MOVED. const staleSignoff = await firstView.tryAct('signoff', 'signoff', { stage: 'direction-review', decision: 'approved', words: WORDS }); @@ -357,7 +371,7 @@ test('direction-review verdict -> ingest -> job open refresh -> UI signoff -> in // Refresh via job open republishes post v2 over the current project.json and asks review-verdict and signoff. const refreshed = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); - expect({ exit: refreshed.exit, stderr: refreshed.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(refreshed); expect(refreshed.stdout.trim().split('\n')).toEqual([ opening.url.href, 'job: post v2 published', @@ -371,14 +385,14 @@ test('direction-review verdict -> ingest -> job open refresh -> UI signoff -> in const signed = await refreshedView.act('signoff', 'signoff', { stage: 'direction-review', decision: 'approved', words: WORDS }); const signoffRes = await cli(root, ['signoff', 'direction-review', root, '--from-review', String(signed.actId), '--json']); - expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(signoffRes); - expect((await cli(root, ['advance', 'accepted', root, '--json'])).exit).toBe(0); + expectExit0(await cli(root, ['advance', 'accepted', root, '--json'])); const report = checkProject(root, undefined, { now: Date.now() }); expect(report.refusals).toEqual([]); expect(report.warnings).toEqual([]); expect(report.holes ?? []).toEqual([]); -}); +}, REFRESH_FLOW_TIMEOUT_MS); test('both-acts-before-ingest remains supported without an intermediate refresh, and pending refresh failure recovers via job reconcile', async () => { const root = copy('3-rig-review', project => { @@ -386,7 +400,7 @@ test('both-acts-before-ingest remains supported without an intermediate refresh, project.advances = (project.advances as JsonObject[]).filter(entry => entry.stage !== 'rig-review'); }); const opening = await open(root); - expect((await cli(root, ['advance', 'rig-review', root, '--json'])).exit).toBe(0); + expectExit0(await cli(root, ['advance', 'rig-review', root, '--json'])); // Both UI acts submitted before any ingest remain supported. const firstView = await page(opening.url); @@ -399,15 +413,17 @@ test('both-acts-before-ingest remains supported without an intermediate refresh, ); const signedBeforeIngest = await firstView.act('signoff', 'signoff', { stage: 'rig-review', decision: 'approved', words: WORDS }); - expect((await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json'])).exit).toBe(0); + expectExit0(await cli(root, ['review', '1', root, '--from-review', String(verdict.actId), '--json'])); // Plant a corrupted package source before running job open refresh: leaves JOB_PUBLISH_PENDING in the outbox. const partsPath = join(root, 'derived/review/v1/out/parts.json'); const savedParts = readFileSync(partsPath); writeFileSync(partsPath, '{}\n'); const failedRefresh = await cli(root, ['job', 'open', root, '--reviewer', 'synthetic-owner', '--role', 'owner']); - expect(failedRefresh.exit).toBe(1); - expect(failedRefresh.stderr).toContain('JOB_PUBLISH_PENDING:'); + expect({ exit: failedRefresh.exit, stderr: failedRefresh.stderr }).toEqual({ + exit: 1, + stderr: expect.stringContaining('JOB_PUBLISH_PENDING:'), + }); expect(failedRefresh.stderr).toContain('JOB_PUBLICATION_INVALID'); expect(pendingPublications(root)).toHaveLength(1); expect(checkProject(root, undefined, { now: Date.now() }).holes?.map(h => h.code)).toEqual(['JOB_PUBLISH_PENDING']); @@ -425,10 +441,10 @@ test('both-acts-before-ingest remains supported without an intermediate refresh, // The signoff act finalized before ingest still ingests cleanly after reconciliation. const signoffRes = await cli(root, ['signoff', 'rig-review', root, '--from-review', String(signedBeforeIngest.actId), '--json']); - expect({ exit: signoffRes.exit, stderr: signoffRes.stderr }).toEqual({ exit: 0, stderr: '' }); + expectExit0(signoffRes); expect((read(root).signoffs as JsonObject[]).at(-1)).toMatchObject({ stage: 'rig-review', target: { version: 1 }, source: `review act ${String(signedBeforeIngest.jobId)}/${String(signedBeforeIngest.actId)}`, }); -}); +}, REFRESH_FLOW_TIMEOUT_MS);