Skip to content

fix: retain relative assets with explicit runtime adapters - #405

Merged
firejune merged 4 commits into
mainfrom
jinni/394-retained-relative-assets
Nov 10, 2026
Merged

firejune merged 4 commits into
mainfrom
jinni/394-retained-relative-assets

Conversation

@firejune

@firejune firejune commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Change

Closes #394.

Retain original config and requirements bytes with hash bindings. Runtime copies apply only declared config/requirements JSON-pointer substitutions, with a json-pointer/1 discriminator keeping strict checks active if a sole mapping is removed. Retain referenced editor images, figure mask and backdrop under contained literal paths, verifying declared hashes. Package regeneration preserves the retained inputs and figure-mask provenance follows the original requirements locator.

Existing absolute adapters and historical frozen-rig/1 records keep their accepted behavior. New relative adaptation checks project containment even when its config root is external. Explicit legacy absolute mappings retain their existing contract. Refused writes preserve project bytes and remove staged destinations.

Verification

Control Exit Result
Actual pre-change writer using three immutable historical source modules 0 1 writer test plus new-reader test, 15 assertions
Final integration Nova 20261110T154352Z-971b98b22da048318927990197547490 0 455 pass, 2 existing network-gated skips, 3876 assertions, 15 files
Frozen historical-record replay through check, package and retained publication 0 1 pass, 7 assertions, record SHA unchanged
Typecheck / lint 0 / 0 green
Final process boundary 0 60.92s held, quiescent

Historical writer source is fixed at 28cd35c. Its actual generated synthetic record SHA 1e16d0ef438f6d0bb6ba0f8aa9fe6fac9f46af31f3d4090e385fd845a9e57f7f remains unchanged through new-reader package regeneration and retained publication. Original source hashes and per-file output provenance are retained. The public reconstructed-layout test is separately labelled as a checker control.

Parent review caught historical implicit relocation compatibility and external-config-root containment defects before PR. Sol repaired both and exercised actual old-writer output. An earlier new metadata emission drifted generated examples, so emission was narrowed to records using the new pointer capability and exact example reproduction now passes. Raw RED evidence remains retained.

Main was integrated before the union because documentation regions also changed with #291. No generated templates changed in this PR. No actual pilot write, producer rerun or owner publication was performed. Hosted check/source-linked, single automatic AI review and exact-head Claude cross-family review remain pending.

Single-review P1 repairs and main integration at 86737be

Compatible identical asset targets now reuse retain/copyTree while reserved metadata, differing bytes and file/directory conflicts refuse. Exact config/requirements substitutions and per-reference hashes remain validated. Canonical source containment operands fix macOS /var system aliases after lexical symlink checks. A small legacy relative-row writer/checker mismatch is fixed. Deferred observations are grouped in follow-up/backlog #408.

Integrated current main c191137, including served Review2.9 pins and generated fixtures. Final native default-TMPDIR run20261110T163454Z-3159e5b0e8854c9cb7ce8b6aca65134e passed66registration tests/680assertions and105touched tests/437assertions plus actual old-record replay1/7. Final Nova20261110T163454Z-2d9c50996f8c4cb789afdb5ecd5b65fb passed242tests/2skips/1620assertions, committed remesh check1/214, old-record replay1/7 and typecheck/lint. Both consumed isolated Review2.9/Play1.24 dependencies. All1268source files stayed frozen. Actual old record SHA1e16d0ef remained unchanged on both platforms.

Mac kernel descendant tracking is unavailable. Commands exited and a scoped process observation found no matching Bun/Node processes. Linux subreaper quiescence is true. Fresh example generation was filtered under the explicit no-renderer instruction. Hosted exact-head checks remain acceptance. First native memory-admission refusal and pre-fix61pass/3fail raw evidence retained. No second AI review requested. No pilot registration performed.

firejune and others added 2 commits November 11, 2026 00:42
Bind original documents and copied assets while validating declared runtime substitutions and preserving historical registrations.

Co-Authored-By: Naru <342599188+fj-naru[bot]@users.noreply.github.com>
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 added the by:codex Written by a Codex (Naru) worker; Claude side judges the review label Nov 10, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 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-10T15:53:47.904986Z ccae2dc 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: ccae2dc4f2

ℹ️ 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/rig-version.ts Resolved
@firejune

Copy link
Copy Markdown
Contributor Author

Claude review (cross-family) at ccae2dc4

Verdict: changes needed. There is one P1 of my own. The known collision P1 is separate. Everything else holds up against the #394 ruling: the original bytes are retained, the checker re-derives the runtime copy, historical records still pass, and the Review consumer contract is unchanged.

P1 (blocking)

P1-a. Already known, accepted and being repaired: the target-collision rule at src/rig-version.ts:818. The blanket conflictsWithClaimed prefix check rejects two supported cases: an occluder image together with its containing plate directory, and repeated references to identical source assets. I did not re-derive this one. The finding below is independent of it and still applies after a narrow fix to that rule.

P1-b. Containment compares a realpath'd project root with lexical source paths. This regresses existing external-source registrations wherever an ancestor of the path is a symlink (macOS /tmp, /var).

  • src/rig-version.ts:666: dir is realpathSync(...).
  • src/rig-version.ts:799: configDir is only resolve(dir, --config).
  • :898: reqDir is dirname(reqSourcePath). For a legacy-absolute .requirements row, that is the lexical before.
  • Every comparison then mixes the two forms: :811 and :901 (choice of allowed root), :834 and :910 (lexical escape check), :842 and :916 (realpathSync(source) compared with a lexical configDir root), and :942 (requirements_source.source_path is silently omitted).

What goes wrong:

  • A config under /tmp/... or os.tmpdir() on macOS fails the realpath check at :842 against its own lexical config dir. The writer refuses with "relative input resolves outside the project".
  • A project-relative .. reference reached from such a config fails the lexical check at :834/:910, even though it really resolves inside the project.

Measured on macOS with the default TMPDIR (a /var/folders/... path, which sits under the /var symlink):

Tree Command Exit Result
base 84c8ab8 bun test tests/rig-version.test.ts 0 44 pass, 0 fail
head ccae2dc bun test tests/rig-version.test.ts non-zero 48 pass, 4 fail (3 of 3 runs)
head ccae2dc same, with TMPDIR set to its realpath 0 52 pass, 0 fail

At head, these three fail every time:

  • The pre-existing registers external frozen sources under os.tmpdir() ... (it passes on base). Error: config.requirements: relative input resolves outside the project.
  • external config root ... (config-project-pointer). Error: config.assemble.editor_receipts.receipts.0.file: relative input escapes the project.
  • checks a reconstructed pre-#394 frozen-rig/1 layout .... Error: requirements.setup_painting.figure_mask.path: relative input escapes the project.

The fourth failure, writer commit owns live publication, passes when run alone. It looks like a knock-on effect of the three above. CI and the shared farm run on Linux, where /tmp is not a symlink, so they stay green.

Suggested fix: canonicalize before comparing. Take the realpath of configDir, reqDir and each source's existing parent, or compare realpath against realpath on both sides. Keep the lexical symlink walk only for symlink refusal. Add one test whose external root is reached through a symlinked ancestor.

Points from the brief

  1. The adapted copy differs only by declared substitutions, and the checker proves it.
    • :480-513 and :557-580: the checker parses config-source and requirements-source, applies only the adapter rows (setPointerValue only replaces existing string leaves, :93-114) and requires isDeepStrictEqual with the retained runtime document.
    • For pointer rows, before must equal the source value at that pointer (:492, :563). after must be a literal, non-reserved path inside the package (:31-48, :182).
    • The adapter file is hash-bound through registration.files (:461).
    • Downgrade: a record is classified strict if it carries any one of three signals: format, pointer rows, or requirements bindings (:466-472). The writer sets format whenever it emits a pointer row (:955, :1000). It always emits pointer rows for take, editor-receipt, cut, limb, depth, reference and backdrop refs, because their autoRelocate is true (:213-335). So a new writer cannot emit a legacy-shaped record for any input that needs adaptation.
    • The legacy path is not a free pass either. The runtime config must still equal the source after only the fixed old implicit set: take run_dir/st_input and editor-receipt file rewritten to inputs/<dotPath> (:504-511).
    • Removing all three signals means rewriting the adapter bytes and their inventory hashes, which is full forgery of the record, outside this design's threat model. Not a blocker.
  2. Containment.
    • before is bound to the document value, and the copy source comes from ref.value, not from before.
    • after targets are literal (no .., :, \, or absolute path).
    • Assets get a lexical check, then a realpath check, then an ancestor symlink walk, then an lstat. copyTree/sourceBytes refuse symlinked files and directories inside directory inputs.
    • The external-config-root edge is enforced for adapted refs (:809-811, :899-901) and covered by the new test.each.
    • The defect is the canonicalization in P1-b.
  3. Historical compatibility: verified with the actual old writer, not only the reconstructed layout.
    • I generated a record with the base-tree writer: relative take run_dir/st_input/reference.painting, an editor receipt with sha256, an absolute .requirements adapter, a ../ figure mask, and a declared but unretained backdrop.
    • I then replayed it through the head checker and packager: rigRegistrationFindings = [], checkProject refusals = [].
    • After deleting and regenerating the review package, project.json SHA is unchanged, the evidence tree is byte-identical, and the package tree is byte-identical (including the legacy backdrop skip at review-package.ts:417).
  4. Scope.
    • No change to the Review consumer contract. src/review-package.ts only widens which retained inputs are re-packaged and adds the backdrop for adapted records, and the legacy package regenerates byte-identically.
    • Test data is synthetic, and no project data was added.
    • The new tests cover the adapted path, the stripped-row and stripped-binding downgrade (keeps the pointer contract after removing its sole adapter row), the legacy layout, and the external-root edge.
    • Other suites, all at head: tests/figure-mask.test.ts exit 0, 48/0. tests/review-package.test.ts exit 0, 57/0. bun run typecheck exit 0. bun run lint exit 0.

Non-blocking

  • Legacy mode drops re-verification of declared sha256 for config-named inputs. strict gates declaredSha256 at :238, :268 and :292. The writer checks them at write time and the bytes stay hash-bound, so this is hardening only. Consider verifying a declared hash whenever one is present, in both modes.
  • configAllowedRoot (:809-811) is chosen before requirements rows are known. With an external config and adaptation only in the requirements, the record becomes json-pointer/1 while its literal config inputs were taken from the external config dir. This matches the pre-existing literal contract, but the classification is order-dependent. Consider deciding pointer use over both documents first.
  • The PR's control that ran the actual pre-change writer is not committed. The in-repo legacy test is a reconstruction. Committing a small fixture generated by the old writer would pin item 3 permanently.
  • A legacy-absolute row on a relative value (after relative) is accepted by the writer at :827 but rejected by the strict checker. The writer then refuses at its final self-check, so this is safe but confusing.

🤖 Generated with Claude Code

firejune and others added 2 commits November 11, 2026 01:32
…ration

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

Copy link
Copy Markdown
Contributor Author

HQ readback, final head 86737beb (narrow, P1 only)

  • P1 collision: retain() now refuses a non-regular-file target and still refuses differing bytes ("two distinct inputs collide"). Byte-identical shared targets are reused. copyTree directory targets go through destinationExists. Reserved names, including the per-run <spec>.json, are checked both ways (prefix in either direction) through conflictsWithReserved.
  • P1 macOS canonicalization: the project dir is realpathSync'd at entry, configDir is the realpath of the config's parent, and source inputs are canonicalized only after checkSourceAncestors has walked the lexical spelling. So /var → /private/var aliases compare correctly, and source symlinks are still refused (lstat after canonicalization). The containment check insideOrEqual(configAllowedRoot, sourcePath) compares canonical paths on both sides.
  • Evidence (macOS default TMPDIR): rig-version 66 pass, review-package/figure-mask 105 pass, actual old-reader 1 pass on macOS and Linux, old project SHA unchanged. Nova focused union 242 pass / 2 skip.
  • Process: one AI review, consumed. No second review. Sign-off follows once both hosted checks are green on this exact head.

@firejune

Copy link
Copy Markdown
Contributor Author

HQ SIGN — exact head 86737beb24f5dc92dcdcdec4b42c5e9326b4eaf6. Both hosted checks green on this head (check: success, source-linked: success). The narrow P1 readback above still holds. One AI review was used and none was re-requested. The tower may land this exact head. After merge, main must be green before v15 registration.

@firejune
firejune merged commit 78d45d9 into main Nov 10, 2026
2 checks passed
@firejune
firejune deleted the jinni/394-retained-relative-assets branch November 10, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

by:codex Written by a Codex (Naru) worker; Claude side judges the review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Retain relative config and requirements assets without rewriting frozen bytes

1 participant