Skip to content

fix: freeze review package directory inputs by receipt - #364

Merged
firejune merged 3 commits into
mainfrom
jinni/328-card-328
Nov 9, 2026
Merged

firejune merged 3 commits into
mainfrom
jinni/328-card-328

Conversation

@firejune

@firejune firejune commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

A config-named occluder plate directory was rejected as a non-file by review-package. Freeze its producer receipt bytes and hash instead, resolve that receipt when reading stored packages, and keep sibling raster measurements owned by the producer. The directory identity remains the config reference resolved to its receipt in the existing files inventory.

Closes #328.

Validation: review-package 55 pass and config-inputs 40 pass; typecheck and lint exit 0. The accepted P1 overlap finding is fixed: explicitly named sibling images remain independent bound inputs, and shared writer/check validation runs before publication. Its two new controls failed before the repair and pass afterward. No second review requested. Full acceptance is PR CI.

Installed Review 2.7.0 omits occluder inputs in handoff exports. A fresh isolated Review 2.7.1 + Parts 2.36.0 probe confirms both separate and overlapping image/plate cases have all 9/9 files, versus 7/9 under 2.7.0, with four reader exits at 0. This was already fixed by rig-c/review#591; duplicate #598 is closed. This PR does not change the repository's dependency pin.

firejune and others added 2 commits November 9, 2026 21:05
Co-Authored-By: Jinni <342598851+fj-jinni[bot]@users.noreply.github.com>
Co-Authored-By: Naru <342599188+fj-naru[bot]@users.noreply.github.com>
Co-Authored-By: Naru <342599188+fj-naru[bot]@users.noreply.github.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-09T12:07:42.324515Z ce91749 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce91749003

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/review-package.ts Outdated
Comment on lines +98 to +99
if (asArray(record.files).some(f => isObject(f) && typeof f.path === 'string' && f.path.startsWith(`${name}/`) && f.path !== bound)) {
throw new Error(`config.${ref.at}: directory input ${name} must bind only ${bound}`);

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 Validate overlapping directory inputs before publishing the package

When a config names both assemble.occluders[0].plate: "inputs/plate" and assemble.occluders[0].image: "inputs/plate/plate.png", packageCommand copies the receipt and the explicitly named image, publishes the package, and updates project.json successfully. However, this new predicate rejects that same inventory, so the next check reports REVIEW_PACKAGE_INPUT_INVALID and handoff readers also throw. Apply the directory-inventory validation in the writer before publishing, so it cannot record a package that its checker immediately refuses. This also violates the shared writer/check validation requirement in AGENTS.md.

AGENTS.md reference: AGENTS.md:L95-L97

Useful? React with 👍 / 👎.

Co-Authored-By: Jinni <342598851+fj-jinni[bot]@users.noreply.github.com>
Co-Authored-By: Naru <342599188+fj-naru[bot]@users.noreply.github.com>
@firejune

firejune commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Accepted the P1 overlap finding. The writer now runs the shared input validator before publication, explicitly named siblings remain independent config inputs, and frozen registration regeneration retains them. The exact overlap and refused-write controls reproduced 2 failures before the fix; final focused verification is 55 review-package and 40 config-input tests passing, plus typecheck/lint. No second review requested.

@firejune
firejune merged commit 793f2b7 into main Nov 9, 2026
1 check passed
@firejune
firejune deleted the jinni/328-card-328 branch November 9, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

review-package: freeze a directory input (occluder plate) by its receipt

1 participant