Skip to content

parts-verdict: read the issue-list verdict (act@5, /4) and publish draft panels #378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
firejune merged 4 commits into from
Nov 9, 2026
Merged

parts-verdict: read the issue-list verdict (act@5, /4) and publish draft panels #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bba716b
Read act@5 and /4 parts verdicts and publish draft panels (#377)
firejune Nov 9, 2026
dc45fc8
Add /4 refusal cases to decodePartsRead tests (#377)
firejune Nov 9, 2026
01cd75e
Align act@5 and /4 issue test fixtures with producer comment/2 shape …
firejune Nov 9, 2026
44fe682
Accept retained legacy draft manifests without panels (#377)
firejune Nov 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/job-publication.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,16 +441,20 @@ export function prepareDraftManifest(root: string, project: JsonObject, id: stri
const models = files.filter(file => String(file.path).endsWith('/skeleton.model.json'));
if (models.length > 1) refuse('JOB_PUBLICATION_INVALID', `drafts[${id}]: multiple compiled models are ambiguous; retain one draft output`);
const revision = jobHash(`${JSON.stringify(draft, null, 2)}\n`);
const manifest = { schema: 'rig-review/1', id: `create-draft-${id}`, revision, title: `draft ${id}`,
const legacyManifest = { schema: 'rig-review/1', id: `create-draft-${id}`, revision, title: `draft ${id}`,
...(models.length ? { model: models[0].path } : {}),
references: context.parts!.images.map((image, index) => ({ id: `image-${index}`, label: `${image.role} ${image.path}`, type: 'image',
path: relative(root, resolve(root, dirname(context.parts!.manifest.path), image.path)).split('\\').join('/') })),
animations: [], checkpoints: [], authoring: files.filter(file => /\/(rig|motion|mesh_report)\.json$/.test(String(file.path)))
.map(file => ({ label: String(file.path).split('/').at(-1), path: file.path })) };
const manifest = { ...legacyManifest,
panels: { schema: 'rig-review/panels@1', left: ['parts.list'], stage: ['parts.canvas'], toolbar: ['parts.layers'], side: ['parts.issues', 'act.submit'] } };

@chatgpt-codex-connector chatgpt-codex-connector Bot Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve compatibility with retained draft manifests

For a project whose draft was published before this change, reopening it in a new job or reconciling a pending publication now fails: adding panels changes the generated manifest bytes, while the existing-file comparison below refuses the original review-draft-<id>.json with JOB_PUBLICATION_INVALID. Both openDraftPublication and publishPending in src/job-host.ts reach this preparation path, so an otherwise unchanged retained draft becomes unpublishable, and restoring its original bytes cannot resolve the refusal. Accept the previous manifest format or provide a migration that preserves existing publication bindings rather than unconditionally requiring the new bytes. The repository's contract-date rule also requires earlier records to remain usable rather than acquiring a new refusal.

AGENTS.md reference: AGENTS.md:L72-L78

Useful? React with 👍 / 👎.

const path = `review-draft-${id}.json`;
const bytes = Buffer.from(`${JSON.stringify(manifest, null, 2)}\n`);
const legacyBytes = Buffer.from(`${JSON.stringify(legacyManifest, null, 2)}\n`);
if (existsSync(join(root, path))) {
if (!publicationFile(root, path).equals(bytes)) refuse('JOB_PUBLICATION_INVALID', `drafts[${id}]: retained publication manifest differs; restore it before reconciling`);
const existing = publicationFile(root, path);
if (!existing.equals(bytes) && !existing.equals(legacyBytes)) refuse('JOB_PUBLICATION_INVALID', `drafts[${id}]: retained publication manifest differs; restore it before reconciling`);
} else writeFileSync(join(root, path), bytes, { flag: 'wx' });
return path;
}
Expand Down
5 changes: 4 additions & 1 deletion src/parts-act.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ export function assertPartsActSource(root: string, project: JsonObject, act: Par
for (const image of act.source.parts.images) check({ path: join(dirname(act.source.parts.manifest.path), image.path), sha256: image.sha256 });
const manifest = JSON.parse(publicationFile(root, act.source.parts.manifest.path).toString('utf8')) as { parts: { name: string }[] };
const names = manifest.parts.map(part => part.name).sort();
if (!isDeepStrictEqual(names, act.value.parts.map(part => part.partId).sort())) fail('verdict part membership differs from the bound manifest');
if (act.schema === 'rig-review/act@5') {
const manifestNames = new Set(names);
if (act.value.issues.some(issue => issue.parts.some(part => !manifestNames.has(part.name)))) fail('verdict part membership differs from the bound manifest');
} else if (!isDeepStrictEqual(names, act.value.parts.map(part => part.partId).sort())) fail('verdict part membership differs from the bound manifest');
if (act.source.model) {
if (!(draft.files as JsonObject[]).some(file => file.path === act.source.model!.path && file.sha256 === act.source.model!.sha256)) fail('displayed model is outside the selected draft');
check(act.source.model);
Expand Down
38 changes: 33 additions & 5 deletions src/parts-verdict-record.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,30 @@ export function decodePartsRead(bytes: Buffer): JsonObject {
let value: unknown;
try { value = JSON.parse(new TextDecoder('utf-8', { fatal: true }).decode(bytes)); }
catch { return fail('PARTS_REVIEW_CLI_INVALID', 'CLI stdout must be a UTF-8 JSON object'); }
if (!object(value) || !['rig-review-parts-verdict/1', 'rig-review-parts-verdict/2'].includes(String(value.schema))
if (!object(value) || !['rig-review-parts-verdict/1', 'rig-review-parts-verdict/2', 'rig-review-parts-verdict/4'].includes(String(value.schema))
|| !['approved', 'changes-requested', 'pending'].includes(String(value.status))
|| !object(value.tool) || typeof value.tool.name !== 'string' || !value.tool.name || typeof value.tool.version !== 'string' || !value.tool.version
|| !object(value.source) || !sha(value.source.sha256) || !Array.isArray(value.source.files) || !value.source.files.length
|| !value.source.files.every(file => object(file) && sha(file.sha256)) || !object(value.parts)) {
|| !value.source.files.every(file => object(file) && sha(file.sha256))) {
return fail('PARTS_REVIEW_CLI_INVALID', 'CLI response needs a supported schema, status, tool, source hashes and per-part outcomes');
}
if (value.schema === 'rig-review-parts-verdict/4') {
if (typeof value.mode !== 'string' || !value.mode || typeof value.updatedAt !== 'string' || !value.updatedAt
|| !['approved', 'changes-requested'].includes(String(value.status)) || !Array.isArray(value.issues)
|| (value.status === 'approved') !== (value.issues.length === 0)) {
return fail('PARTS_REVIEW_CLI_INVALID', 'CLI response needs a supported schema, status, tool, source hashes and issues');
}
for (const issue of value.issues) {
if (!object(issue) || typeof issue.id !== 'string' || !issue.id || typeof issue.summary !== 'string' || !issue.summary.trim()
|| !Array.isArray(issue.parts) || !issue.parts.length
|| !issue.parts.every(part => object(part) && typeof part.name === 'string' && part.name.length > 0 && sha(part.imageHash))
|| new Set(issue.parts.map(part => (part as JsonObject).name)).size !== issue.parts.length) {
return fail('PARTS_REVIEW_CLI_INVALID', 'each CLI issue needs id, summary and unique part bindings');
}
}
return value;
}
if (!object(value.parts)) {
return fail('PARTS_REVIEW_CLI_INVALID', 'CLI response needs a supported schema, status, tool, source hashes and per-part outcomes');
}
for (const part of Object.values(value.parts)) {
Expand All @@ -49,6 +68,11 @@ function partsStdout(root: string, evidence: JsonObject, path: string): Buffer {
}

export function partsVerdictNames(verdict: JsonObject): { flagged: string[]; noted: string[] } {
if (Array.isArray(verdict.issues)) {
const names = [...new Set(verdict.issues.flatMap(issue => object(issue) && Array.isArray(issue.parts)
? issue.parts.filter(object).map(part => String(part.name)) : []))];
return { flagged: names, noted: [...names] };
}
const entries = Object.entries(verdict.parts as JsonObject);
return {
flagged: entries.filter(([, part]) => (part as JsonObject).outcome === 'flagged').map(([name]) => name),
Expand Down Expand Up @@ -182,10 +206,11 @@ export function recordedPartsVerdict(root: string, project: JsonObject, draftId:
const bytes = receipt.schema === PARTS_CLI_RECEIPT ? partsStdout(root, binding, 'signoffs.parts-review')
: publicationFile(realpathSync(root), String(binding.path));
let partsFile: string;
const spec = receipt.schema === PARTS_CLI_RECEIPT ? PARTS_CLI_RECEIPT : 'rig-review/act@2';
if (evidence !== undefined && (!object(evidence) || evidence.kind !== 'parts-verdict'
|| (evidence.spec !== undefined && evidence.spec !== spec))) return fail('PARTS_VERDICT_RECEIPT_INVALID', 'signoffs.parts-review.evidence has an invalid receipt spec');
let spec: string;
if (receipt.schema === PARTS_CLI_RECEIPT) {
spec = PARTS_CLI_RECEIPT;
if (evidence !== undefined && (!object(evidence) || evidence.kind !== 'parts-verdict'
|| (evidence.spec !== undefined && evidence.spec !== spec))) return fail('PARTS_VERDICT_RECEIPT_INVALID', 'signoffs.parts-review.evidence has an invalid receipt spec');
const verdict = decodePartsRead(bytes);
assertPartsReadDraft(root, project, verdict, draftId, options);
if (verdict.status !== 'approved') return fail('PARTS_VERDICT_NOT_APPROVED', `signoffs.parts-review.evidence for draft ${draftId} is not approved`);
Expand All @@ -194,6 +219,9 @@ export function recordedPartsVerdict(root: string, project: JsonObject, draftId:
} else {
if (receipt.schema !== 'rig-create/review-act@1') return fail('PARTS_VERDICT_RECEIPT_INVALID', 'signoffs.parts-review.evidence names an unsupported receipt spec');
const act = decodeReviewAct(bytes.toString('utf8'), 'parts-verdict');
spec = act.schema;
if (evidence !== undefined && (!object(evidence) || evidence.kind !== 'parts-verdict'
|| (evidence.spec !== undefined && evidence.spec !== spec))) return fail('PARTS_VERDICT_RECEIPT_INVALID', 'signoffs.parts-review.evidence has an invalid receipt spec');
if (act.source.selector.id !== draftId) return fail('PARTS_VERDICT_DRAFT_MISMATCH', `signoffs.parts-review.draft_id differs from the retained act for ${draftId}`);
const draft = (project.drafts as JsonObject[]).find(draft => draft.id === draftId);
const draftRoot = `derived/drafts/${draftId}`;
Expand Down
56 changes: 52 additions & 4 deletions src/review-act.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,28 @@ import { collectItems, asArray, isObject } from './check.ts';
import { stepHash } from './step-hash.ts';
import type { Json, JsonObject } from './templates.ts';

export type PartsIssue = {
readonly id: string;
readonly summary: string;
readonly parts: readonly { readonly name: string; readonly imageHash: string }[];
readonly sourceHash: string;
readonly rigRegion: JsonObject;
readonly imageRegion: JsonObject | null;
readonly view: JsonObject;
readonly crop: JsonObject;
readonly nativeCrop: JsonObject | null;
};
export type InterviewPicksAct = Extract<JobAct, { kind: 'interview-picks' }>;
export type PartsVerdictAct = Extract<JobAct, { kind: 'parts-verdict' }>;
export type PartsVerdictActV2 = Extract<JobAct, { kind: 'parts-verdict' }>;
export type PartsVerdictActV5 = Omit<PartsVerdictActV2, 'schema' | 'value'> & {
readonly schema: 'rig-review/act@5';
readonly value: {
readonly verdict: 'approved' | 'changes-requested';
readonly issues: readonly PartsIssue[];
readonly words: string;
};
};
export type PartsVerdictAct = PartsVerdictActV2 | PartsVerdictActV5;
export type ReviewVerdictAct = Extract<JobAct, { kind: 'review-verdict' }> & { source: JobModelSourceBinding };
export type CheckpointAct = Extract<JobAct, { kind: 'checkpoint' }>;
export type SignoffAct = Extract<JobAct, { kind: 'signoff' }>;
Expand All @@ -40,7 +60,8 @@ export function decodeReviewAct(bytes: string, kind: 'review-verdict' | 'parts-v
let act: JsonObject;
try { const parsed: unknown = JSON.parse(bytes); if (!object(parsed)) throw Error(); act = parsed; }
catch { return failure('REVIEW_ACT_INVALID', 'act is not a JSON object'); }
if (act.schema !== 'rig-review/act@2' && !(kind === 'review-verdict' && act.schema === 'rig-review/act@3')) failure('REVIEW_ACT_SCHEMA_UNSUPPORTED', `act schema ${String(act.schema)} is unsupported, including legacy rig-review/act@1`);
if (act.schema !== 'rig-review/act@2' && !(kind === 'review-verdict' && act.schema === 'rig-review/act@3')
&& !(kind === 'parts-verdict' && act.schema === 'rig-review/act@5')) failure('REVIEW_ACT_SCHEMA_UNSUPPORTED', `act schema ${String(act.schema)} is unsupported, including legacy rig-review/act@1`);
const source = act.source;
const parts = object(source) ? source.parts : null;
const version = object(parts) && typeof parts.enumeratorVersion === 'string' ? parts.enumeratorVersion : '';
Expand Down Expand Up @@ -98,8 +119,35 @@ export function decodeReviewAct(bytes: string, kind: 'review-verdict' | 'parts-v
|| !Array.isArray(parts.images) || parts.images.some(row => !file(row) || typeof row.role !== 'string')
|| source.model !== null && !file(source.model) || source.direction !== null && !file(source.direction)
|| source.plan !== null && (!object(source.plan) || !Array.isArray(source.plan.steps)
|| source.plan.steps.some(step => !object(step) || !id(step.id) || !sha(step.hash)))
|| !Array.isArray(value.parts) || !value.parts.length) failure('REVIEW_ACT_INVALID', 'parts verdict source or rows are malformed');
|| source.plan.steps.some(step => !object(step) || !id(step.id) || !sha(step.hash)))) failure('REVIEW_ACT_INVALID', 'parts verdict source or rows are malformed');
if (act.schema === 'rig-review/act@5') {
if (!['approved', 'changes-requested'].includes(String(value.verdict)) || !Array.isArray(value.issues)
|| value.parts !== undefined) failure('REVIEW_ACT_INVALID', 'parts verdict source or issues are malformed');
const issueIds = new Set<string>();
for (const issue of value.issues) {
if (!object(issue) || !id(issue.id) || issueIds.has(issue.id)
|| typeof issue.summary !== 'string' || !issue.summary.trim()
|| !Array.isArray(issue.parts) || !issue.parts.length
|| !sha(issue.sourceHash) || !object(issue.rigRegion)
|| !('imageRegion' in issue) || (issue.imageRegion !== null && !object(issue.imageRegion))
|| !object(issue.view) || !object(issue.crop)
|| !('nativeCrop' in issue) || (issue.nativeCrop !== null && !object(issue.nativeCrop))) {
failure('REVIEW_ACT_INVALID', 'parts verdict issue is malformed');
}
issueIds.add(issue.id);
const partNames = new Set<string>();
for (const part of issue.parts) {
if (!object(part) || !id(part.name) || !sha(part.imageHash) || partNames.has(part.name)
|| !parts.images.some(img => object(img) && img.role === 'part' && img.path === `parts/${part.name}.png` && img.sha256 === part.imageHash)) {
failure('REVIEW_ACT_INVALID', 'parts verdict issue part is invalid, duplicated or outside the bound parts manifest');
}
partNames.add(part.name);
}
}
if ((value.verdict === 'approved') !== (value.issues.length === 0)) failure('REVIEW_ACT_INVALID', 'parts verdict contradicts its issues');
return act as unknown as PartsVerdictAct;
}
if (!Array.isArray(value.parts) || !value.parts.length) failure('REVIEW_ACT_INVALID', 'parts verdict source or rows are malformed');
const rows = value.parts as JsonObject[];
const ids = new Set<string>();
for (const row of rows) {
Expand Down
Loading
Loading