Skip to content

spec: portable-core-contracts - #172

Open
yihanzhu wants to merge 6 commits into
mainfrom
ystack/spec/portable-core-contracts
Open

spec: portable-core-contracts#172
yihanzhu wants to merge 6 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 exact design for the record/validation child accepted in #156. It pins main's intent blob 3ed8bb434c096ec126d680019a9491ab8a113e31.

The spec defines:

  • seven canonical document kinds with exact shared, nested, and top-level wire shapes;
  • total request/result/status/outcome/evidence/time rules;
  • thirteen closed capabilities and fifteen exact permissions;
  • manifest offer → profile request → external grant → result use separation;
  • pure supplied-document validation and stable errors;
  • exact downstream seams for Git profile resolution and executable adapter tests.

Scope boundary

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

Git object truth remains in portable-profile-resolution. Independent case execution, process isolation, Git revalidation, and the 2×2 matrix remain in portable-adapter-contract-tests. Closed PR #154 stays superseded and unmerged; parent #153 remains open.

Review

Three independent passes covered implementability, security/adversarial behavior, and roadmap/dependency consistency. Findings were resolved until all three returned CLEAN. Formal round-0 found three wire-contract gaps: multi-root JSON streams, incompatible error-code grammar, and a non-Git execution snapshot. Round-1 closed them. Formal round-1 found missing base-branch identity and non-resolvable tool configuration. Round-2 closed both. Formal round-2 found contradictory duplicate provenance and an impossible tool/source capacity edge. Round-3 closed both. Formal round-3 then found that the schema could not truthfully record execution under the wrong identity/environment. The operator authorized a bounded revision, which fixed that but its final review found two remaining issues: actual capability mismatch and a Git-invalid leading-dot topic component. The operator authorized one revision limited to those two items. This head binds capability mismatch under the same failed/inconclusive truth rule and rejects leading-dot components. The independent passes rechecked both changes CLEAN.

The artifact is 839 lines, above the repository's soft PR-size guide. It stays one exact G2 document because splitting the normative wire schema, relation rules, and capability registry across separately accepted specs would create two sources of core meaning. This is disclosed for the operator's review; it does not approve an implementation size exception. The implementation itself is capped at 400 net lines and must return to G2 if its plan cannot meet that bound.

Proof

Clean detached worktree at e27c6173b5c5d92de9936353e23dd014606b6522:

  • spec stage scope, required sections, and intent blob: passed
  • structure check: passed
  • ShellCheck 0.11.0: passed
  • north-star resolver: 74 passed
  • north-star gate/consumer: 226 passed
  • models parser: 63 passed
  • degraded-review gates: 91 passed
  • pending-stage, round-cap, quota, rename/probe tests: passed
  • /faber retirement/doctor cases: 27 passed
  • rename gate and git diff --check: passed

Merging this PR accepts G2 design and permits the next planning step. It does not accept a plan, implementation, live activation, or merge authority for an agent.

@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: e27c617
Status: ✅  Deploy successful!
Preview URL: https://27b3f1f1.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: round cap hit, ambiguous spec, oversized PR, 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.

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

Labels

needs-human Escalation: round cap hit, ambiguous spec, oversized PR, or failure 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