Skip to content

spec: portable-core-contracts - #172

Merged
yihanzhu merged 9 commits into
mainfrom
ystack/spec/portable-core-contracts
Aug 28, 2026
Merged

spec: portable-core-contracts#172
yihanzhu merged 9 commits into
mainfrom
ystack/spec/portable-core-contracts

Conversation

@yihanzhu

@yihanzhu yihanzhu commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Closes #171

G2 decision

This PR proposes the smallest v1 contract layer for the accepted portable-core-contracts intent. It pins main's intent blob 3ed8bb434c096ec126d680019a9491ab8a113e31.

The rescoped spec defines:

  • five canonical documents: adapter manifest, profile, resolved profile, stage request, and stage result;
  • exact shared refs plus total request/result/status/outcome/evidence/time rules;
  • three closed producer, verifier, and reviewer capabilities;
  • five exact permissions with bounded resource/action meaning;
  • manifest offer → profile request → external grant → result use separation;
  • three pure supplied-document validator modes and stable errors;
  • exact downstream seams for Git profile resolution and adapter contract tests.

Scope-down record

The original draft grew to seven documents, thirteen capabilities, and fifteen permissions. Repeated final reviews then found unrelated edges across core records, role semantics, and adapter-test records. The operator ruled that #172 had expanded beyond the smallest useful v1 and authorized a same-issue, same-branch rewrite: #172 (comment)

Nothing was dropped silently:

This change still writes only work/portable-core-contracts/spec.md. It does not read Git, run an adapter, authenticate a claim, grant authority, publish, activate a profile, change /yshifu, or alter live behavior.

Review

Independent implementability and security/compliance reviews of the rescoped file are clean. The security pass specifically rechecked requested-versus-actual facts, permission escape, evidence replay, status truth, role separation, and the publisher handoff after #173 was linked.

The first formal review of the minimal v1 found four bounded gaps: instruction-byte delivery, canonical-JSON tree provenance, source/base stale selectors, and patch media type. The operator authorized one revision limited to those four findings: #172 (comment)

The next formal review did not repeat those four. It found three relation gaps: manifest/profile package-tool-config predicates, verifier target-revision binding, and deterministic skill consistency. The operator authorized only those three after a read-only consistency preflight: #172 (comment)

The preflight found no fourth issue and recorded the exact predicates before editing: #172 (comment). The current head closes only those three findings. It keeps five documents, three capabilities, five permissions, and all deferred scope unchanged. Independent implementability and security checks of the exact diff are clean.

The final formal review covered exact head 0488807ee288f6cf103f0404864008e1e3550cc0 against base a388e984768a794ccd193fb4ab7a3488254e8655 and found no actionable defect: #172 (comment). GitHub CI is green on the same head. The PR is ready for operator review and merge; no agent has merge authority.

Exact-head proof

Clean detached worktree at 0488807ee288f6cf103f0404864008e1e3550cc0 against base a388e984768a794ccd193fb4ab7a3488254e8655:

  • spec stage scope: only work/portable-core-contracts/spec.md changed;
  • required sections and intent blob: passed;
  • structure check: passed;
  • ShellCheck 0.11.0: passed;
  • north-star resolver: 74 passed, 0 failed;
  • north-star gate/consumer: 226 passed, 0 failed;
  • models parser: 63 passed, 0 failed;
  • degraded-review gates: 91 passed, 0 failed;
  • pending-stage, round-cap, and quota tests: passed;
  • rename/doctor cases: 27 passed, 0 failed;
  • rename gate and git diff --check: passed.

The spec is 628 lines after replacing the previous 839-line design and closing the authorized relation gaps. Its planned implementation remains capped at 400 normally formatted net lines and must return to G2 if that estimate cannot be proved.

Merging this PR accepts only G2 design and permits the next planning step. It does not accept a plan, implementation, profile activation, agent merge authority, or any external write. Human merge only.

@yihanzhu yihanzhu added the round-0 Review-loop counter: initial PR label Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: ee9d5f5
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The proposed contract has parsing and type contradictions that would either accept invalid document streams or make required test records impossible to represent. It also leaves an execution input outside the stated target-repository binding.

Full review comments:

  • [P1] Require exactly one JSON value before canonical comparison — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.5vdWFi8ZZd/work/portable-core-contracts/spec.md:26-30
    jq -S -c . accepts a stream of JSON values, so a file containing two canonical envelopes produces identical output and passes the byte comparison. Unless the wrapper separately enforces one root value, relational and shape checks may accept a multi-document file as one document; explicitly reject empty and multi-value streams and test both cases.

  • [P1] Make validator error codes valid test error IDs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.5vdWFi8ZZd/work/portable-core-contracts/spec.md:311-315
    expected_error_id is typed as ID, whose grammar permits only lowercase characters, but the validator's required stable errors are uppercase values such as E_PARSE and E_SHAPE. Contract-test cases therefore cannot record the exact errors produced by the core validator; define a separate error-code type or change one of the two grammars.

  • [P1] Restrict execution snapshots to the target repository — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.5vdWFi8ZZd/work/portable-core-contracts/spec.md:483-483
    input_snapshot_ref accepts any artifact_ref, including the content variant, which has no repository identity. For that input the later rule requiring the execution snapshot to match the request's target repository cannot be checked, so a valid request can provision unrelated content; require a Git-backed reference or define an explicit target-binding relation for content snapshots.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 28, 2026

Copy link
Copy Markdown

Deploying ystack with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0488807
Status: ✅  Deploy successful!
Preview URL: https://7815db9f.fabrica-6yx.pages.dev
Branch Preview URL: https://ystack-spec-portable-core-co.fabrica-6yx.pages.dev

View logs

@yihanzhu yihanzhu added round-1 Review-loop counter: revision 1 and removed round-0 Review-loop counter: initial PR labels Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 7f33e38
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The proposed v1 schema omits identities needed for deterministic change-request publishing and tool-configuration provenance. Because adding either field later requires a new major schema version, these gaps should be resolved before accepting the contract.

Full review comments:

  • [P1] Bind change requests to a named base ref — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.UcbhZYuoAA/work/portable-core-contracts/spec.md:490-490
    When opening a change request against a non-default branch, the publisher needs both the base branch name and expected commit. Here head_ref has both, but base_ref has only a commit, so the adapter must infer a branch from ambient forge state or policy and cannot verify that the intended base ref has not moved. Add a TopicRef for the base and bind it to the supplied commit.

  • [P1] Make each tool configuration source resolvable — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.UcbhZYuoAA/work/portable-core-contracts/spec.md:215-215
    When a requested tool uses committed configuration, tool_ref records only config_sha256, with no repository/path or content ref. The resolver therefore cannot locate the bytes from its declared inputs, and tool_sources cannot prove which source belongs to which tool configuration, especially when tools share a package. Include an immutable configuration ref or a keyed resolved-tool record so provenance can be checked deterministically.

@yihanzhu yihanzhu added round-2 Review-loop counter: revision 2 and removed round-1 Review-loop counter: revision 1 labels Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: cda7944
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The proposed contract permits contradictory source provenance and contains incompatible cardinality limits for otherwise valid tool bindings. These should be resolved before the implementation plan is accepted.

Full review comments:

  • [P1] Key each provenance entry by its source object — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FLWp3tuMf2/work/portable-core-contracts/spec.md:566-567
    When the same requested tool Git object has two source_value_ref entries with different value_sha256 or value_format claims, both are unique under canonical-sha256, while their projected object-ref set still equals the distinct requested refs. validate-profile-set can therefore accept contradictory provenance for one object; require exactly one source entry per exact source object.

  • [P2] Align tool-source capacity with requested tools — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FLWp3tuMf2/work/portable-core-contracts/spec.md:254-255
    A valid binding may request 129 tools with distinct package and config objects, but resolving them requires 258 source entries while tool_sources is capped at 256. Such a profile satisfies the declared requested_tool_refs limit yet cannot have any valid resolved_profile; cap the total distinct package/config refs at 256 or reshape the source representation.

@yihanzhu yihanzhu added round-3 Review-loop counter: revision 3 (cap) and removed round-2 Review-loop counter: revision 2 labels Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 5f3f9cd
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The proposed contract cannot represent a truthful executed failure when observed execution identity or environment differs from the request. That conflicts with its total-result and actual-facts requirements and should be resolved before implementation.

Review comment:

  • [P1] Preserve mismatched actual execution identities — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.tmLsOMln7q/work/portable-core-contracts/spec.md:463-467
    When an operation runs under the wrong principal, package, config, or environment, the only truthful result is an executed failure containing those observed values. These rules instead require every executed result and its evidence to equal the requested binding and environment, so the validator rejects the incident record and the caller must either lie or emit no valid stage result. Allow failed or inconclusive runs to record mismatched actual facts while keeping exact equality for successful results.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Round cap reached — operator decision needed

Current head: 5f3f9cd9453166d14d8638d800a95634bfed8cac

Formal round-3 review still found one P1 contract gap:

  • An operation may actually run under the wrong principal, adapter/package/config, or environment. The current equality rules reject that truthful executed failure because every executed result must equal the requested binding and environment. A caller would have to lie or emit no valid stage result.

Final review: #172 (comment)

Per the round cap, I am stopping edits. No code or live behavior changed.

Operator options

  1. Authorize one final bounded revision (recommended). Limit it to this truth-recording rule: successful completed results keep exact requested=actual equality; executed failed/cancelled or completed-inconclusive results may preserve mismatched observed performer/binding/environment, must identify every mismatch, and bind evidence to the observed facts. Then rerun exact-head gates and one final review.
  2. Accept the current design with a recorded reason. Not recommended: it cannot represent this execution-boundary incident truthfully.
  3. Rescope or supersede the G2 spec. Preserve this branch and open a smaller replacement decision; do not silently drop the finding.

The PR remains open, round-3, unmerged, and requires the operator's explicit ruling.

@yihanzhu yihanzhu added the needs-human Escalation: plan refresh, round cap, ambiguous spec, size, or failure label Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

Operator ruling — one final bounded revision authorized

The operator authorized one revision beyond the round cap, strictly limited to the remaining P1:

  • completed non-inconclusive results keep exact requested=actual equality;
  • executed failed, cancelled, or completed-inconclusive results may preserve observed performer, binding/config, and environment mismatches;
  • every mismatch is explicit, and evidence binds the observed actual facts;
  • no other design or scope changes are authorized.

After this revision, rerun all exact-head gates and one final formal review. If that review is not clean, stop for a new operator ruling. Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 10659fd
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The contract cannot faithfully record an important class of execution incidents, and its topic-ref validator accepts names that Git cannot use. These issues should be corrected before the specification becomes the implementation contract.

Full review comments:

  • [P1] [Bugs] Record the capability actually used on failed runs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.qreaMS0orK/work/portable-core-contracts/spec.md:611-611
    For an executed failed, cancelled, or completed-inconclusive attempt, the spec allows observed performer, binding, and environment mismatches but still requires used_capability to equal the requested capability. If dispatch invokes the wrong capability, no valid result can truthfully record the incident: it must either lie about actual use or fail validation. Allow an observed capability mismatch for these incident outcomes while retaining equality for completed non-inconclusive runs.

  • [P2] [Bugs] Reject Git-invalid leading-dot topic refs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.qreaMS0orK/work/portable-core-contracts/spec.md:189-191
    The TopicRef grammar accepts components such as refs/heads/.hidden, but Git rejects any ref component beginning with .. A document using such a ref would pass core validation and then fail at every Git-backed publisher, so the lexical rule should also forbid leading dots.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Operator-authorized final review completed — not clean

Reviewed head: 10659fd23837fdd32825c43c3c36c2fd0f867c06

The authorized bounded revision passed all exact-head gates and independent pre-review. Its one permitted final formal review still found two issues:

  1. P1: executed failed/cancelled/completed-inconclusive results can now preserve performer/binding/environment mismatches, but used_capability is still forced to equal the request. An actual wrong-capability dispatch cannot be recorded truthfully.
  2. P2: TopicRef forbids ending dots but still allows a component beginning with ., which Git rejects.

Final review: #172 (comment)

Per the operator's authorization, no further revision is allowed in this run. The PR remains open, round-3, needs-human, and unmerged.

New operator options

  • Authorize another strictly bounded revision: add observed-capability mismatch under the same failed/inconclusive-only truth rule, and reject a topic-ref component beginning with .. Then rerun all gates and one review.
  • Rescope/supersede: preserve this branch and split the contract further; do not silently drop either finding.
  • Accept the current design with a recorded reason: not recommended because the P1 still makes one execution incident unrepresentable.

Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Operator ruling — one revision limited to two findings

The operator authorized one more revision, strictly limited to the two findings from the previous final review:

  1. Add actual capability to the same failed/cancelled/completed-inconclusive mismatch rule. Completed non-inconclusive results still require requested capability = actual used capability, and evidence binds the observed capability/mismatch.
  2. Reject any TopicRef component beginning with ..

No other design or scope change is authorized. After the revision, rerun all exact-head gates and one formal review. If that review is not clean, stop for a new operator ruling. Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: e27c617
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The proposed v1 contract leaves most permission semantics undefined, cannot express a normal full-repository execution snapshot, and derives an inaccurate overall test outcome for mixed failed and inconclusive cases. These should be resolved before the schema becomes an implementation contract.

Full review comments:

  • [P1] Define the bounds of every permission ID — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Q9WSaKHFui/work/portable-core-contracts/spec.md:512-515
    When an adapter implements these contracts, only record and content reads have normative limits; permissions such as target read, scratch write, candidate execution, model invocation, and forge writes remain bare labels. Two adapters can therefore give the same validated permission different resource or action scope, including a generic write or execution escape, despite the contract's portability and closed-authority goals. Define the allowed resources and actions for every v1 permission before freezing the schema.

  • [P1] Permit root-tree snapshots — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Q9WSaKHFui/work/portable-core-contracts/spec.md:231-234
    When core.execution.provision.v1 provisions an entire target repository, its immutable snapshot is normally the commit's root tree, which has an empty repository-relative path. RepoPath rejects that path (and .), so no valid git_object_ref can represent the common full-repository snapshot. Add an explicit root-tree representation or use a revision/root-tree-specific reference.

  • [P2] Let known test failures outrank inconclusive cases — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.Q9WSaKHFui/work/portable-core-contracts/spec.md:642-645
    With multiple observations, one inconclusive case currently forces the whole result to inconclusive even when another complete case has a definite status/error mismatch or failed assertion. This masks a known contract failure and conflicts with the failed-first evidence rule used for stage results. Derive failed first when any conclusive mismatch exists, then use inconclusive only when no failure is known.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Two-item authorized review completed — not clean

Reviewed head: e27c6173b5c5d92de9936353e23dd014606b6522

The operator-authorized two-item revision passed all exact-head gates and independent pre-review. Its one formal review still found three different contract gaps:

  1. P1: most permission IDs are labels without normative resource/action bounds, so adapters could assign incompatible or generic semantics.
  2. P1: git_object_ref cannot represent the commit root tree because RepoPath forbids the empty path.
  3. P2: contract-test overall result gives inconclusive precedence over a known failed case, masking a definite failure.

Final review: #172 (comment)

Per the ruling, I am stopping. The PR remains open, round-3, needs-human, and unmerged.

Recommended next ruling

Rescope/supersede this G2 rather than authorize another cap exception. Repeated final reviews are now finding unrelated edges across three subdomains: stage/evidence records, adapter/profile/capability-permission semantics, and contract-test records. The 839-line artifact and its 400-line implementation claim are no longer credible as one normally reviewable concern.

A clean split would preserve this branch as the decision record and create separately gated children for:

  • canonical primitives plus stage request/result/evidence;
  • manifest/profile plus capability/permission semantics;
  • contract-test inventory/result semantics, aligned with the executable runner child.

Other operator options:

  • explicitly authorize another bounded revision for only the three findings above;
  • accept the current design with a recorded reason (not recommended).

No code or live behavior changed. Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Operator ruling — scope down the same G2

The operator agreed that #172 grew beyond the accepted intent's smallest-v1 requirement. This is a rescope of the existing portable-core-contracts G2, not a new G1.

Authorized direction:

  • keep the same issue, branch, PR, and accepted intent blob;
  • rewrite the spec as a minimal v1 in the same spec.md;
  • keep only the core stage/profile records and the smallest closed producer/verifier/reviewer capability set;
  • move contract-test inventory/result semantics back to the existing portable-adapter-contract-tests chain;
  • defer forge, CI, execution, identity, and publisher-write capabilities to their adapter/control-foundation work;
  • target a normally reviewable spec and implementation;
  • do not create replacement G1s and do not merge.

round-3 and needs-human remain until the rescoped exact head passes fresh gates and review. Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: a1217ba
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The proposed v1 contract accepts impossible source provenance and does not define how adapters may consume the instruction records required by every capability. It also leaves common stale cases and patch output typing unrepresentable or contradictory.

Full review comments:

  • [P1] [Security] Define access to referenced instruction bytes — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FM8rzjFWXy/work/portable-core-contracts/spec.md:152-153
    When any capability runs, it needs the bytes behind its output contract, verification plan, review policy, finish condition, and verification instructions, but these refs carry identity only and no permission allows reading them: target.read covers Git objects and the change delta, while model.invoke covers only the fixed profile binding. An implementation must therefore ignore these instructions or read them through an unmodeled side channel. Define bounded, digest-checked delivery or an exact content-read permission.

  • [P1] [Bugs] Restrict canonical JSON sources to blobs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FM8rzjFWXy/work/portable-core-contracts/spec.md:142-142
    When profile_source or manifest_source uses value_format:"canonical-json", source_value_ref still permits a tree because git_object_ref allows both blobs and trees. validate-profile-set can therefore accept a tree as the exact source of a JSON document, which the resolver cannot recover as canonical document bytes. Require canonical JSON sources to reference blobs.

  • [P2] [Bugs] Cover source and base drift in stale results — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FM8rzjFWXy/work/portable-core-contracts/spec.md:362-368
    When a review base moves or a producer's source/base changes after the request is created, no stale selector can name that request field. Because stale requires a non-empty observation set, the caller cannot truthfully record these common stale cases and must use a misleading selector or another status. Add source/base selectors, or explicitly state that these fields are never freshness baselines.

  • [P2] [Bugs] Validate media type for git-patch outputs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.FM8rzjFWXy/work/portable-core-contracts/spec.md:413-416
    When a git-patch request returns a content_ref with an unrelated media type such as image/png, these rules still accept it because they check only output count and delta_ref equality. That produces contradictory canonical metadata and leaves downstream delta consumers to interpret arbitrary content as a patch. Require the patch output and delta to use a fixed patch media type.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Rescoped final review completed — not clean

Reviewed head: a1217baaa841553cc3378e425f3cbfeda1356039

Reviewed base: a388e984768a794ccd193fb4ab7a3488254e8655

CI and every local exact-head gate passed. The one fresh formal review required by the operator ruling still found four contract gaps:

  1. P1 Security: every capability depends on referenced output-contract, verification-plan, review-policy, finish-condition, or verification-instruction bytes, but v1 defines neither bounded digest-checked delivery nor permission to read them.
  2. P1 Bugs: a canonical-json source may still point to a Git tree, so a validator can accept provenance that cannot produce one JSON byte stream.
  3. P2 Bugs: stale observations cannot name request source or base, so common source/base drift cannot be recorded as stale.
  4. P2 Bugs: a git-patch output is not restricted to a patch media type.

Formal review: #172 (comment)

These findings are inside the retained five-document/three-capability core; they do not justify restoring the deferred forge, publisher, CI, execution, identity, or adapter-test scope. The last three are narrow relation/type omissions. The first needs one explicit choice between pre-delivered digest-checked instruction bytes and a separate exact content-read permission.

Per the authorization, I am stopping without another edit or review. The PR remains open, round-3, needs-human, and unmerged.

Operator options

  1. Authorize one revision limited exactly to these four findings. Keep the five documents and three capabilities; choose the smallest instruction-delivery boundary, require JSON blobs, add source/base stale selectors, and pin the patch media type. Then rerun exact-head gates and one review.
  2. Accept the current design with a recorded reason. Not recommended because both P1s leave an impossible or unmodeled implementation path.
  3. Stop or rescope again. Preserve this branch and record the disposition; no scope is dropped silently.

Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Operator ruling — one revision limited to four findings

The operator authorized one revision beyond the round cap, limited exactly to the four findings from the rescoped final review:

  1. Define bounded, digest-checked delivery of the instruction bytes required by the three capabilities, without adding general content, file, or network read authority.
  2. Require every canonical-json source to reference a Git blob.
  3. Let stale observations name exact request source and base values.
  4. Require git-patch output/delta content to use one fixed patch media type.

Keep the five documents, three capabilities, and all previously deferred forge, publisher, CI, execution, identity, and adapter-test scope unchanged. No other design or scope change is authorized.

After this revision, rerun all exact-head gates and one formal review. If that review is not clean, stop for a new operator ruling. Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 666e29e
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The spec leaves security-critical manifest/profile relations undefined and permits verifier evidence to be associated with a different revision than the one tested. It also allows deterministic bindings to request skills that results must report as unused.

Full review comments:

  • [P1] Define the manifest-to-profile package and tool checks — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.JqN0hLBMDX/work/portable-core-contracts/spec.md:243-245
    The phrase “package/config/tool relations” never defines the required predicates. A profile can therefore point its package or requested tools at different Git objects than the selected manifest, or supply config without a manifest config contract, while borrowing that manifest's capabilities. Specify full package/tool equality and the config-presence rule so the validator cannot accept substituted code or config.

  • [P1] Bind verifier input to the requested target revision — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.JqN0hLBMDX/work/portable-core-contracts/spec.md:360-363
    A verifier request currently accepts a candidate tree from commit B when target_revision names commit A, provided both use the same repository. The resulting evidence then appears bound to A even though the verifier ran against B. Require the candidate tree's revision to equal the present target revision.

  • [P2] Reject skills on deterministic bindings — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.JqN0hLBMDX/work/portable-core-contracts/spec.md:197-200
    Deterministic bindings may currently declare non-empty skill_refs because only the model request and prompt are forbidden, but every deterministic result must report skills as not-applicable. Such requested skills can never be recorded as used or checked for conclusive execution. Either require an empty skill set for deterministic bindings or define how their actual use is recorded.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Four-item final review completed — not clean

Reviewed head: 666e29e6e3c7209917e8a2e36237e07dd6824025

Reviewed base: a388e984768a794ccd193fb4ab7a3488254e8655

All local exact-head gates and GitHub CI passed. The formal review did not repeat any of the four authorized findings, but it found three new gaps in the retained minimal core:

  1. P1: manifest/profile package, tool, and config relations are named but not stated as exact predicates, so substituted package/tool/config claims can pass.
  2. P1: verifier candidate input is restricted to the target repository but not the exact requested target revision, so evidence can appear bound to a different commit.
  3. P2: deterministic bindings may request skills even though deterministic execution must report skills as not applicable.

Formal review: #172 (comment)

These findings remain inside the accepted five-document, three-capability, five-permission scope. They do not justify restoring any deferred adapter-test, forge, publisher, CI, execution, or identity capability. The first needs an explicit closed relation matrix; the other two are single invariant gaps.

Per the operator ruling, I am stopping without another edit or review. The PR remains open, round-3, needs-human, and unmerged.

Operator options

  1. Authorize one revision limited exactly to these three findings. Before writing, perform a read-only consistency preflight of only the affected manifest/profile and verifier relations. Any different finding is reported, not silently fixed. Then add the exact package/tool/config predicates, bind the candidate tree to the target revision, and require empty skills on deterministic bindings; rerun gates and one review.
  2. Rescope or stop. Preserve this branch and record the disposition; no scope is dropped silently.
  3. Accept the current design with a recorded reason. Not recommended because both P1s permit false package or revision binding.

Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Operator ruling — read-only preflight, then one revision limited to three findings

The operator authorized only these three findings from the review of 666e29e6e3c7209917e8a2e36237e07dd6824025:

  1. State the exact manifest-to-profile package, requested-tool, and config-presence predicates.
  2. Require the verifier candidate tree revision to equal the request's present target revision.
  3. Require deterministic bindings to have an empty skill set.

Before any edit, run a read-only consistency preflight limited to those affected manifest/profile and verifier relations. If it finds a different fourth issue, stop and report it; do not silently fix it.

If the preflight finds no different issue, make one revision limited to the three findings above. Keep the five documents, three capabilities, five permissions, instruction-delivery boundary, and all deferred scope unchanged. No other design or scope change is authorized.

After the revision, rerun all exact-head gates and one formal review. If that review is not clean, stop for a new operator ruling. Human merge remains the only merge path.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Three-item read-only consistency preflight — clean

No file was changed during this preflight. Independent implementability and security checks found no fourth issue and no shape/cardinality conflict in the authorized areas.

The exact predicates for the revision are:

  • matching manifest, profile binding, and resolved package source use full-object equality;
  • each requested tool equals one offered tool_ref in full, not only by tool ID;
  • resolved tool-source IDs equal requested tool IDs, and each package/config source object and presence equals its requested tool;
  • a binding config may be present only when the manifest has a config contract; a manifest contract does not force a profile config;
  • resolved binding config presence/source equals the profile binding config;
  • verifier target revision is present and the candidate tree's full revision equals it;
  • deterministic bindings forbid model request and prompt and require empty skill_refs; their resolved skill-source set is therefore empty.

The existing five documents, three capabilities, five permissions, source cardinalities, instruction-delivery boundary, and deferred scope remain unchanged. The next edit is limited to expressing these three authorized findings.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 0488807
Reviewed-base: a388e98
reviewer: operator-default @ high

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

The new specification matches the referenced intent, preserves the stated safety boundaries, and defines internally consistent validation and relationship rules. No actionable correctness defect was identified in the changed file.

@yihanzhu yihanzhu added merge-ready Exact reviewed head+base passed; YOUR merge; void if either moves and removed needs-human Escalation: plan refresh, round cap, ambiguous spec, size, or failure labels Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

Final handoff — review clean, CI green

Head 0488807ee288f6cf103f0404864008e1e3550cc0 passed every exact-head local gate and GitHub CI against base a388e984768a794ccd193fb4ab7a3488254e8655.

The final read-only Codex review covered that exact head/base and found no actionable defect: #172 (comment)

The three-item read-only preflight found no fourth issue, and the final revision changed only work/portable-core-contracts/spec.md. The intent blob remains 3ed8bb434c096ec126d680019a9491ab8a113e31.

needs-human is cleared and merge-ready is applied. This is G2 only: it accepts the spec, not a plan, implementation, activation, or agent merge authority. Human merge only.

@yihanzhu
yihanzhu merged commit a296940 into main Aug 28, 2026
2 checks passed
@yihanzhu
yihanzhu deleted the ystack/spec/portable-core-contracts branch August 28, 2026 23:48
@yihanzhu

Copy link
Copy Markdown
Owner Author

Plan preflight — R14 budget gate failed; no implementation plan created

Source spec blob: 064f5bd5f519f7c5646303ab082e1bcf11289c7a

The intent/spec hash chain is fresh, and main is clean. Before creating ystack/impl/portable-core-contracts, two independent read-only implementation estimates tested the spec's hard 400-net-line budget. Both found it non-credible without weakening the accepted contract:

Area Evidence-backed normal-format range
contracts.jq 380–755
shell wrapper 50–110
readable fixture builder + 60 or more adversarial mutations 240–330
CI pin, docs, and restore manifest 30–60
Credible total about 800–1,100

The current repo has no reusable jq contract validator. Local jq is 1.7.1-apple, while the contract requires exact jq 1.6 behavior and a checksum-pinned CI binary. The wrapper must independently handle bounded snapshots, raw-byte canonical comparison, SHA-256 document refs, three command shapes, stable error sanitization, and cleanup. Jq must still implement all five document shapes, shared refs, profile relations, and the complete request/result truth table. The tests need a five-document digest graph plus raw-byte and semantic mutations.

Reaching 400 would require at least one forbidden tradeoff: compressed long lines, fewer negative tests, a second schema/parser, tests that reuse production logic as their oracle, system-jq drift, or weaker byte/hash/error handling.

R14 explicitly says to return to G2 when the plan cannot prove this budget. Therefore:

  • no plan.md was written;
  • no implementation branch or PR was created;
  • no code, workflow, live profile, or adapter changed.

Recommended G2 decision

Preserve the accepted v1 semantics, but replace the single total-400 promise with three ordered, independently reviewed implementation slices, each capped near 300–400 lines and with no live activation until the final slice:

  1. canonical-byte/runtime foundation and shared shape machinery;
  2. manifest/profile/resolved-profile shapes and relations;
  3. stage request/result/status/evidence relations, public wrapper, full integration tests, and final wiring.

The G2 amendment must also name exact child branches/PR gates because the current artifact convention has one deterministic implementation branch per slug.

If 400 must remain a total product budget, v1 must instead shrink to canonical/shared foundations plus one small document/command, and the remaining profile/stage contracts need separately accepted child chains. Raising one PR to roughly 1,000 lines is not recommended.

Human merge and scope decisions remain the operator's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-ready Exact reviewed head+base passed; YOUR merge; void if either moves round-3 Review-loop counter: revision 3 (cap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

G2: specify portable core contracts

1 participant