Skip to content

Roadmap item 4: Assemble inactive default profile - #230

Open
yihanzhu wants to merge 8 commits into
mainfrom
codex/default-profile-assembly-v1
Open

Roadmap item 4: Assemble inactive default profile#230
yihanzhu wants to merge 8 commits into
mainfrom
codex/default-profile-assembly-v1

Conversation

@yihanzhu

@yihanzhu yihanzhu commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • Bind seven durable default adapter payloads into one core-v2 profile and manifest set.
  • Keep protected roles separated and bind their decision records to the accepted Roadmap.
  • Add focused restore and identity proof.

Scope

Tracks Roadmap item 4. This is one inactive, repo-only assembly concern. The profile is not resolved, selected, qualified, installed, or activated.

Safety

No real target, credential, network operation, model invocation, publication, release, installation, activation, or deployment is performed or authorized. Every requested tool set is empty; CI, identity, and publisher request no capability or permission.

Targeted proof

  • default profile assembly: 5/5 focused checks passed
  • Bash syntax, ShellCheck 0.11.0 style, canonical JSON, and diff checks pass
  • ablation: removing the identity binding breaks the accepted seven-role set; removing reviewer authority fails core relations, so both were restored

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 4, 2026

Copy link
Copy Markdown

Deploying ystack with  Cloudflare Pages  Cloudflare Pages

Latest commit: bc705d9
Status: ✅  Deploy successful!
Preview URL: https://c30e6746.fabrica-6yx.pages.dev
Branch Preview URL: https://codex-default-profile-assemb.fabrica-6yx.pages.dev

View logs

@yihanzhu

yihanzhu commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 7fc2a04
Reviewed-base: a637451
reviewer: operator-default @ high

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

The assembled producer binding is incompatible with the producer adapter's enforced contract, so the default profile cannot perform its advertised producer operation. Its proof also incorrectly binds historical object references to mutable HEAD paths.

Full review comments:

  • [P1] Add the producer configuration required by its adapter — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.sam04enQEC/profiles/default/v1/manifests/claude-code-producer.json:1-1
    When this profile is resolved for a producer run, adapters/claude-code-producer/v1/normalize.jq requires the manifest to contain config_contract_ref and the selected binding to contain config_ref; this manifest and the producer binding in profile.json omit both. Generic document validation therefore passes, but the advertised default producer will reject every trust context. Add matching immutable configuration references and cover the assembled profile with the actual producer normalizer.

  • [P2] Resolve pinned objects at their recorded revision — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.sam04enQEC/scripts/test/default-profile-assembly.test.sh:133-134
    When a pinned adapter or prompt path is changed by a later commit, the profile remains valid because it names the historical commit and object ID, but this assertion—and the analogous prompt assertion—looks up the path in HEAD, causing CI to fail while no longer validating the recorded revision/path relationship. Preserve or fetch the pinned commit in shallow CI and run ls-tree against $commit instead.

ci and others added 2 commits September 4, 2026 17:24
…ormalizer

Completes the in-progress fix for the two review findings on this PR.

- P1: the producer manifest carries config_contract_ref and the producer
  binding carries config_ref, both bound to the immutable
  profiles/default/v1/producer-config.json blob. The test now builds a
  resolved profile for the assembled profile and runs the real Claude
  Code producer normalizer over changed and unchanged snapshots; a
  missing config is rejected.
- P2: every pinned object is resolved at its recorded revision. The test
  fetches each recorded commit from origin by exact hash into a private
  history repository and runs ls-tree there, never against HEAD.
- The profile now binds six adapters. The identity role, which had been
  bound to the GitHub forge observation payload, is dropped; that payload
  is not a forge materializer and remains a separate observation payload.

The config reference currently names the pushed commit that introduced
the config payload. Once that payload lands on main through its own PR,
the reference is re-pinned to the main commit in a follow-up commit here.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@yihanzhu

yihanzhu commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 7fc2a04
Reviewed-base: a637451
reviewer: gpt-5.5 @ high

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

The default profile assembly records an incompatible producer manifest/binding for the selected Claude producer adapter. This is a functional data issue for the new profile, despite the focused test not catching it.

Review comment:

  • [P2] Include the producer config contract in the profile — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.JdhNg06VeH/profiles/default/v1/manifests/claude-code-producer.json:1-1
    When the default profile's producer binding is used with this Claude producer manifest, the selected adapter will reject it: adapters/claude-code-producer/v1/normalize.jq requires the manifest to have config_contract_ref and the resolved producer binding to have config_ref in trust_relations, but this manifest and the matching binding in profile.json omit both. That makes any future activation of the default producer fail with E_TRUST even though the assembly test passes.

@yihanzhu

yihanzhu commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 43823f5
Reviewed-base: a637451
reviewer: gpt-5.5 @ high

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

The assembly proof relies on a PR-only config commit that may not be durable after the repo's squash-merge flow, undermining the restore/CI guarantee added by the patch.

Review comment:

  • [P2] Pin config proof to durable main history — /private/var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.ifeusKtTsF/scripts/test/default-profile-assembly.test.sh:121-122
    When config_commit comes from the new producer binding, this fetch depends on the intermediate PR commit d97b8e36.... This repo's publishing path uses squash merges, so after merge or branch cleanup that commit is not guaranteed to remain reachable from origin; the required test/restore proof can then fail at producer-config-fetch even though producer-config.json exists in the restored main commit. Please validate the config from the restored/main commit, or pin a commit that is guaranteed to survive on main.

@yihanzhu

yihanzhu commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

The remaining P2 is the planned re-pin: the config reference currently names the pushed commit that introduced producer-config.json (#231's head). Once #231 squash-merges, a follow-up commit here re-pins config_ref.revision.commit_id to that main commit, so the proof depends only on durable main history. Nothing else in the finding is open.

ci and others added 2 commits September 6, 2026 00:33
The producer binding's config_ref pointed at the config PR's branch head
while that PR was open. It now points at the squash commit on main that
carries profiles/default/v1/producer-config.json, so the reference stays
resolvable after the branch is deleted.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-assembly-v1

# Conflicts:
#	RESTORE.md
#	ci/required-files.txt
@yihanzhu

yihanzhu commented Sep 6, 2026

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 0d16d4b
Reviewed-base: 74c6662
reviewer: gpt-5.5 @ high

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

No correctness issues were found in the changed profile assembly data, restore manifest additions, or focused test script.

Merging main doubled the producer config entry in ci/required-files.txt,
which the construction publisher gate refuses as a duplicate active entry.
The config stays under its own block; the assembly block no longer repeats it.

Proof: construction-publisher-gate 134 passed, 0 failures; default-profile-assembly 7/7.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.

1 participant