spec: shadow-input-assembler

[yihanzhu]

Tracks #262

G2 spec for the merged intent (G1, PR #266), written to the operator's DR-1 decision (option 2: the assembler takes a resolved profile as input and validates it; it resolves nothing). Frontmatter records intent-blob 61218c3c9b3554f4a86c58fd3a7311d9e818f3b6 and risk: high (the assembler now embodies security controls: verbatim source-purity predicates, a clean shell entry, byte pins on the default profile, read-only guards; the intake had proposed routine, the spec supersedes it per REVIEW.md). Merging this PR accepts both. It does not close the intake issue.

This PR adds only work/shadow-input-assembler/spec.md.

[cloudflare-workers-and-pages]

Deploying ystack with    Cloudflare Pages

Latest commit:	d5f55b6
Status:	✅  Deploy successful!
Preview URL:	https://3bd7c003.fabrica-6yx.pages.dev
Branch Preview URL:	https://ystack-spec-shadow-input-ass.fabrica-6yx.pages.dev

View logs

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: f08a096
Reviewed-base: 5090b42
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-81102/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec's own estimated implementation size conflicts with the declared standard review-size path. This should be corrected before accepting the artifact because it affects the next PR's gate conditions.

Review comment:

• [P2] Record a review-size exception or split scope — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.moBFuAkUhe/work/shadow-input-assembler/spec.md:22-23
  This declares review_size: standard, but the same sentence plans roughly 550 lines before docs/manifest edits, which exceeds the repo's ~300–400 net-line standard budget and needs either a split or an accepted evidence-based exception (AGENTS.md:102-106, work/README.md:71-73). Without that, the implementation PR will be pre-authorized to overrun the review-size gate while still claiming the standard path.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: a9bee0e
Reviewed-base: 6526999
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-84799/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec has blocking ambiguity and compliance gaps that would guide the next implementation into an over-budget PR, incoherent environment evidence, or undefined non-default profile behavior. These should be resolved before accepting the artifact.

Full review comments:

• [P2] Record a size exception or split the work — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Udp7uNDTcF/work/shadow-input-assembler/spec.md:22-23
  This spec marks review_size: standard while estimating about 550 lines before the docs and manifest edits. The repo's standard budget is ~300–400 net lines, and larger one-concern work needs an accepted exception with an evidence-based range (AGENTS.md:102-105). As written, the implementation PR can follow this accepted spec and still exceed the standard review budget without the required exception; either split the work or change this to accepted-exception with the range.

• [P2] Bind the request to the actual environment — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Udp7uNDTcF/work/shadow-input-assembler/spec.md:188-192
  For any run with a real or fixture claim, this fixed inert environment_ref makes the stage request and materializer stage_result record one environment while reproduce.sh evaluates the caller's claim from a separate file. That leaves the materialization proof bound to the wrong execution environment, contrary to the roadmap requirement to bind proof to the execution environment (ROADMAP.md:146-151); make the environment ref an input, or keep this assembler out of self-host proof until the environment initiative can supply the real value.

• [P2] Restrict or prove non-default profiles — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Udp7uNDTcF/work/shadow-input-assembler/spec.md:193-195
  Requirement 3 makes anything other than the default profile's six exact manifest documents a refusal, but this note says another profile can still be passed. That leaves the implementation and tests with two incompatible behaviors for non-default profile-dir inputs: reject them as out of scope or accept an unproven graph. Please either require the default profile/id explicitly, or define and test the non-default contract.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: d868624
Reviewed-base: 4965175
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-94958/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The change adds only the spec artifact, with the intent hash matching the current intent and no discrete correctness issue found in the documented requirements or design.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 8a2059b
Reviewed-base: 4965175
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-96039/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec would allow misleading non-default profile inputs and excludes supported SHA-256 Git repositories, both conflicting with the stated intent for a real default-profile assembler.

Full review comments:

• [P2] Refuse non-default profile bytes — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.j8G5Jit6Rn/work/shadow-input-assembler/spec.md:69-75
  When a caller supplies a self-consistent profile directory whose profile.json reuses the id profile.default.v1, these checks only require the id, six roles/manifests, and profile_set_ok, so the assembler can emit a shadow input that claims to be from the real default profile but is actually built from different bytes. The intent requires a run “from the real default profile” (work/shadow-input-assembler/intent.md:21-22), so this spec should require comparing the supplied profile/manifests (and pinned producer config) to the shipped profiles/default/v1/ identities, or stop claiming real-default provenance.

• [P2] Preserve SHA-256 Git repository support — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.j8G5Jit6Rn/work/shadow-input-assembler/spec.md:46-48
  For any real target stored as a SHA-256 Git repository, this interface cannot assemble an input because it accepts only a 40-character SHA-1 commit and later hard-codes sha1, even though the local materializer protocol and fixture builder already support both SHA-1 and SHA-256 object formats. That narrows the intent’s “any real repository revision” outcome (work/shadow-input-assembler/intent.md:21-22); add an explicit hash-algorithm input or derive it from the source repository instead of forcing SHA-1.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 7e651ad
Reviewed-base: 4965175
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-4537/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec leaves two validation gaps that can produce inputs inconsistent with the claimed provenance or with what the materializer will accept. These should be corrected before the artifact is accepted.

Full review comments:

• [P2] Mirror all source-repository guards before emitting input — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Cbblz68xyL/work/shadow-input-assembler/spec.md:51-54
  When the supplied bare repo has a local config key that adapters/local-git-materializer/v1/materialize.sh:301-350 rejects, or a non-sample hook, this spec can still let the assembler emit an input because it only mirrors the environment plus alternates/grafts/replace/shallow/worktree checks. The driver will later report materialization.refused for that same repo, which defeats the stated goal of refusing bad inputs before producing something the driver rejects; include the materializer's config and hook guards in the source repo validation.

• [P2] Bind the resolved config source to the pinned config — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Cbblz68xyL/work/shadow-input-assembler/spec.md:115-116
  With a supplied resolved profile whose producer binding keeps the same config_source.source but changes config_source.value_sha256, the spec still accepts it: the pin proves the standalone producer-config.json bytes, while profile_set_ok only checks optional config sources point at the binding ref and that duplicate source claims agree. Since the output publishes the resolved-profile ref as real-default provenance, add an explicit check and test that the producer config source digest equals the pinned config digest.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: fddd7ce
Reviewed-base: 40582c2
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-5917/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec omits checks needed to keep the assembler from producing driver-rejected inputs and from accepting a resolved profile that disagrees with a pinned default-profile document. These are actionable correctness gaps in the proposed implementation contract.

Full review comments:

• [P2] Require full source repository admissibility checks — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.oL9vHidQie/work/shadow-input-assembler/spec.md:48-54
  For a bare repository that has an unsupported config key or a live non-sample hook, these checks can still let the assembler emit an input that shadow/v1/reproduce.sh later refuses through the materializer (E_SOURCE_CONFIG/E_SOURCE_HOOK). Since the intent says bad inputs should be refused before producing something the driver rejects, the spec should require mirroring the materializer's full source repository checks, not only the protected Git reads and alternates/grafts/shallow/worktree checks.

• [P2] Bind resolved producer config to the pinned file — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.oL9vHidQie/work/shadow-input-assembler/spec.md:111-116
  When the supplied resolved profile has a producer config_source.value.value_sha256 that differs from the pinned producer-config.json, profile_set_ok still succeeds because it checks the referenced source but not that source value's digest. Since this spec pins producer-config.json specifically because it is outside profile_set_ok, also require the resolved producer config source digest to equal the pinned file digest; otherwise the assembler can emit a protocol-valid input whose resolved profile claims a different producer config than the shipped default.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: bd16743
Reviewed-base: 40582c2
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-12860/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec promises source-refusal parity with the materializer but omits materializer checks that still cause downstream refusal for valid bare repositories. That leaves the planned component unable to meet its own stated behavior.

Review comment:

• [P2] Validate every source-side materializer refusal up front — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.6Q95APPL6j/work/shadow-input-assembler/spec.md:357-359
  For source repos that pass these two copied spans but are still rejected later by materialize.sh — for example packed-refs containing refs/replace (materialize.sh:333-347) or a commit tree with a symlink/submodule/invalid path or oversized closure (:456-465) — the assembler would still emit an input and only reproduce.sh would return materialization.refused. That violates the stated requirement that the assembler never hand the driver an input the materializer refuses, so the spec should either cover the remaining source-side checks in the assembler and tests or narrow that guarantee.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: ab23595
Reviewed-base: 40582c2
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-40163/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec would direct the implementation toward a CI-red test in the repository's current shallow checkout and leaves a materializer refusal path outside the assembler despite the stated boundary.

Full review comments:

• [P2] Make copied-span test usable in shallow CI — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ix24QcMmHA/work/shadow-input-assembler/spec.md:320-324
  On CI this proposed assertion will fail whenever the header names an older commit such as a637451...: .github/workflows/ci.yml:15 uses checkout without a fetch-depth override, so the test job has only the checked-out head and git show <commit>:... cannot read arbitrary ancestors. Since CI is a hard gate (AGENTS.md:103-106), compare against a commit guaranteed to be present or fetch the needed history before mandating this test.

• [P2] Mirror the materializer's commit-size guard — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ix24QcMmHA/work/shadow-input-assembler/spec.md:352-354
  For a valid commit object over the materializer's 1 MiB limit, this boundary still lets the assembler emit an input because it only requires commit existence and format while delegating only tree-content checks. adapters/local-git-materializer/v1/materialize.sh:355-360 rejects that same source before the tree scan, so the driver would return materialization.refused instead of the assembler's clear E_TARGET; include the same commit-size guard and a negative test here.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: ae0dc27
Reviewed-base: 40582c2
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-48203/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec misclassifies a security-control scope as routine and contains shell-entry and cleanup requirements that would leave the planned component bypassable or hard to retry in specific failure modes.

Full review comments:

• [P1] Compliance: classify the assembler as high risk — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FFvArrmqZW/work/shadow-input-assembler/spec.md:3-3
  This scope is not routine: the spec requires source-purity predicates, a clean shell entry, profile byte pins, and read-only materialization-input guards, which are security controls. If this stays risk: routine, the implementation can use the weaker routine gate instead of the required plan-only high-risk path; REVIEW.md:105-108 says risk:routine is blocking when the scope touches a security control.

• [P2] Clear aliases before accepting the clean marker — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FFvArrmqZW/work/shadow-input-assembler/spec.md:748-753
  When the script is invoked as /bin/bash ... __assemble_clean ... with BASH_ENV defining shopt -s expand_aliases; alias find=:, Bash processes that before any scrub. The copied scrub only unsets functions/exported variables, and this marker path does not re-exec through env -i, so aliases/options remain and can change the later bare find predicates despite the claim that direct marker calls gain nothing.

• [P2] Clean run_root on refusal paths — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FFvArrmqZW/work/shadow-input-assembler/spec.md:827-829
  When any validation after run_root is created refuses, for example a bad hook/source guard, emit_error exits before this final removal. Because the output directory must be empty on entry, the same output dir then cannot be retried after fixing inputs; require a trap cleanup immediately after creating the scratch directory rather than only removing it on the success path.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 332ed10
Reviewed-base: 40582c2
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-55909/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec has two correctness gaps that can lead the implementation to emit the wrong refusal class or fail its retry guarantee under normal trapped signals.

Full review comments:

• [P2] Validate the timestamp against core time rules — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.xPfvv1XLk8/work/shadow-input-assembler/spec.md:111-112
  For timestamps with the right regex shape but invalid calendar values, e.g. 2026-02-30T00:00:00Z, this only requires the YYYY-MM-DDTHH:MM:SSZ form, while the core time_ok used by stage_request rejects them. That lets an implementation pass argument validation and later fail the protocol self-check as E_RELATION, contrary to requirement 12's E_USAGE for bad timestamps; require the argument to pass the same core time validation up front.

• [P2] Arm the cleanup trap before creating scratch — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.xPfvv1XLk8/work/shadow-input-assembler/spec.md:902-905
  When INT, TERM, or HUP arrives after mkdir succeeds but before the next trap command, the current sequence can leave run_root in the otherwise-empty output directory; retry then hits E_WORKSPACE even though requirement 18 says those signals are trapped and only KILL, power loss, or failed removal can leave scratch behind. Please require the cleanup trap to be installed before creating run_root so the documented retry guarantee is true.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: e46f4ff
Reviewed-base: 1095ca4
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-64346/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec leaves a retry-breaking partial-output case in the final write path despite adding requirements around cleanup and retryability. This should be fixed before using the spec as the implementation contract.

Review comment:

• [P2] Stage final outputs before exposing them — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.YrtWz0DcKK/work/shadow-input-assembler/spec.md:1080-1085
  If the assembler is interrupted or one of these final writes fails after creating/truncating a file, the trap described above only removes run_root, leaving partial input.json/ref documents in the output directory. A retry then hits E_WORKSPACE even though requirement 18 is meant to leave refusal/interrupt paths retryable, so the implementation should either stage these files under run_root and commit them only after all writes succeed, or have the trap clean uncommitted output files.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 4ef1728
Reviewed-base: 1095ca4
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-71798/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The spec contains actionable design issues that would bake incorrect cleanup behavior and misleading error classification into the implementation and tests.

Full review comments:

• [P2] Record committed paths before each mv — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.au8Qw8dojI/work/shadow-input-assembler/spec.md:1068-1073
  For a catchable signal delivered after an output mv succeeds but before the following append runs, the moved file is not yet in the trap's list, so the trap will remove run_root but leave that output behind. That violates the stated retry guarantee and makes the next run fail E_WORKSPACE; record the destination before the rename, or otherwise close this signal window.

• [P3] Keep jq invocation failures as runtime errors — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.au8Qw8dojI/work/shadow-input-assembler/spec.md:318-318
  If this jq call fails because the core modules cannot be loaded or another runtime/package problem occurs, the spec requires E_USAGE, even though the error taxonomy below classifies missing required files and failed commands as E_RUNTIME. Only a successful time_ok result of false should be E_USAGE; otherwise broken installs get reported as caller timestamp mistakes.

[yihanzhu]

Codex reviewer (cross-vendor, read-only)

Reviewed-head: d5f55b6
Reviewed-base: e9bea97
reviewer: gpt-5.5 @ high

Posted verbatim by codex-review.sh (codex exec review --json --base refs/codex-review/273-4767/base in an isolated temp worktree, sandbox forced read-only). Comments only — Codex never pushes, approves, or merges.

The change only adds the shadow input assembler spec, and its hash link, risk classification, pinned profile digests, and described scope are consistent with the surrounding repository rules and referenced files. I did not find a discrete correctness issue that should block this patch.

[yihanzhu]

Merging under operator decision OD-1 (#275): Codex review clean at Reviewed-head d5f55b618dedb2cf69e2eaef2acd5ddb2dff8f10 / Reviewed-base e9bea97f09ff653ec02d22315f488da880d5208a (current main) after fifteen review rounds, ci green, carrier labels consistent. This merge is G2 for shadow-input-assembler: it accepts the spec and risk: high (the intake had proposed routine; DR-1 option 2 stands). Next: plan-only PR on ystack/plan/shadow-input-assembler.