Skip to content

policy: exceptional implementation rule - #164

Merged
yihanzhu merged 4 commits into
mainfrom
codex/exceptional-implementation-rule
Aug 28, 2026
Merged

policy: exceptional implementation rule#164
yihanzhu merged 4 commits into
mainfrom
codex/exceptional-implementation-rule

Conversation

@yihanzhu

@yihanzhu yihanzhu commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Add one exceptional implementation rule

This is the independent P1 policy concern from #163. It closes a real gap: ystack already requires tests, one concern, and durable artifact links, but it did not require root-cause fixes or govern implementation workarounds and source comments.

What changed

  • AGENTS.md now defines the complete root-cause-first exception and non-blanket comment floor.
  • REVIEW.md makes unexplained, self-granted, copied, untested, or lifecycle-free exceptions Important blockers while keeping ordinary comment wording a nit.
  • routines/coder.md sends an implementation-time exception back to the accepted artifact gate instead of letting the coder improvise a workaround.
  • routines/coder-revision.md prevents review feedback from authorizing a quick workaround or copied exception.
  • templates/target-CLAUDE.md gives Claude-based target coders the same baseline and permits zero optional comments without erasing required notices, directives, documentation, invariants, or exception provenance. It now states plainly that independent reviewer delivery is separate.
  • manager/CLAUDE.md and templates/yshifu-command.md preserve a paused exception decision on the same branch or PR and select the correct resume mode.

Every exception must be accepted before implementation, isolated behind one private boundary, regression-tested in CI, linked to a durable record, and carry a temporary removal condition or permanent external invariant plus re-evaluation trigger. A link or PR discussion records provenance; it is not approval. Exceptions never waive CI, independent review, authorization boundaries, target safety rules, constitution rules, or human merge.

Enforcement boundary

No generic cross-language comment scanner or structured exception registry is added here. A central prose/regex gate would either check only wording or misclassify valid licenses, directives, generated code, and language-specific documentation. The mechanically reliable parts are explicit instead: every exception's regression test runs in target CI, and a lint/type/deterministic invariant check is required when the target can express it reliably. Root-cause and tradeoff judgment remains in review.

Scope and activation

  • This PR is separate from spec: portable-control-plane-core #154 and changes no portable-core, reviewer-adapter, install-script, or restore files.
  • Independent target-reviewer delivery is intentionally scoped out to strict-subset follow-up Deliver the exception rule to target reviewers #165; this PR no longer claims that optional target-CLAUDE.md reaches that reviewer.
  • AGENTS.md and REVIEW.md are operator-driven constitution changes.
  • The coder routines take effect when yshifu next reads them for a spawn.
  • Because the yshifu command template changes, merging this PR does not update the live command by itself; the operator must run scripts/install.sh after merge.
  • The target template affects future copies. Existing targets still receive the universal coder routine but are not silently rewritten.
  • No agent merges this PR.

Proof

Verified in a clean detached worktree at 8dff2721ffd511da23b846ed23ec684f58ca4cd8:

  • Exactly seven scoped files changed; git diff --check passed.
  • Independent builder review: clean after accepted approval sources were synchronized.
  • Independent Security/Compliance review: clean after self-grant and safety-bypass gaps were closed.
  • Independent cross-surface consistency review: clean after no-copy, CI, lifecycle, and target zero-optional-comments behavior were synchronized.
  • Formal review rounds found and resolved: duplicate round-0 re-entry, stale ready on pause/failure, missing operator decision context, unsafe raw handoff data, undefined dirty-state digests, base movement mistaken for corruption, missing fix-mode race checks, and accidental application to add-CI/greenfield process gates.
  • The final design auto-resumes only a clean, identity-matched attempt. Dirty work remains human-blocked until the operator records its disposition. Decision capsules are bounded labeled data and never authorization or tool input.
  • The round-cap target-reviewer delivery finding was split to Deliver the exception rule to target reviewers #165 under explicit operator authorization; this scoped PR covers ystack policy, its reviewer, universal coder/manager behavior, and the Claude-facing target template only.
  • ShellCheck 0.11.0 over tracked scripts: passed.
  • Required-file structure check: structure ok.
  • North-star resolver: 74 passed, 0 failed.
  • North-star gate/consumer: 226 passed, 0 failed.
  • models.conf parser: 63 passed, 0 failed.
  • degraded Codex gate: 91 passed, 0 failed.
  • Remaining v2 test suites and rename gate: passed.

Closes #163

@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: 8dff272
Status: ✅  Deploy successful!
Preview URL: https://b316b3f5.fabrica-6yx.pages.dev
Branch Preview URL: https://codex-exceptional-implementa.fabrica-6yx.pages.dev

View logs

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 9accd6a
Reviewed-base: dc05812
reviewer: operator-default @ high

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

The new escalation rules do not wire into the existing resumable-state behavior. Both initial implementation and review-time exception approvals can leave the workflow unable to continue on the intended branch or PR.

Full review comments:

  • [P1][Bugs] Preserve a resumable branch before escalating — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.rS9UA4guSb/routines/coder.md:157-159
    When an exception is discovered after step 5, this stops after the deterministic issue branch has already been created and may contain edits. The documented needs-human re-entry later spawns a fresh round-0 coder, whose branch-creation step will fail because that branch still exists, or because the shared checkout is dirty. Specify whether to preserve and resume the existing branch or clean it before stopping so operator approval can actually restart the task.

  • [P1][Bugs] Resume approved exceptions in fix mode — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.rS9UA4guSb/routines/coder-revision.md:109-113
    When this path is triggered during review, the current PR is labeled needs-human with reason ambiguous-spec, but manager/CLAUDE.md defines that reason's re-entry as updating the issue, reapplying ready, and spawning a round-0 coder. After the operator approves the exception, that starts a duplicate implementation attempt instead of continuing the existing PR. Add a revision-stage re-entry path that resumes the current branch with coder-revision.md.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 8f1ca0c
Reviewed-base: dc05812
reviewer: operator-default @ high

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

The new exception workflow can leave contradictory state labels and does not provide the operator enough information to resolve its main escalation path. Its global testing rule also conflicts with an existing supported bootstrap exception.

Full review comments:

  • [P1] [Bugs] Clear ready when resume verification fails — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.naeRlRsD43/routines/coder.md:154-156
    On an implementation-resume launch, manager/CLAUDE.md has just reapplied ready before spawning this coder. If the repo changes between the manager's check and this second check, this path adds needs-human with reason failure but leaves ready in place. The issue then advertises both blocked and runnable state, so a later label scan can spawn another coder despite the identity mismatch. Remove ready and verify it is absent on this stop path.

  • [P1] [Bugs] Include the decision question in pre-PR handoffs — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.naeRlRsD43/manager/CLAUDE.md:524-527
    When an exception is first discovered during implementation, no accepted artifact describes that new constraint yet, but this requires the coder to report only the identity tuple. The operator therefore receives a generic ambiguous-spec escalation without learning what exception or tradeoff they must approve, reject, or rescope, so the new re-entry path cannot progress. Include a sanitized plain-language description of the exception and requested decision alongside the tuple, while still excluding paths and patch content.

  • [P2] [Compliance] Reconcile CI bootstrap with the test mandate — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.naeRlRsD43/AGENTS.md:104-105
    The blanket requirement that every exception have a regression test conflicts with the existing sole-purpose add-CI exception in routines/coder.md, which applies when no PR CI exists and explicitly forbids adding a smoke test unless the issue asks for one, allowing a lint/build-only gate. After this policy lands, a coder or reviewer cannot follow both contracts without either widening the issue's scope or blocking the supported bootstrap path. Scope this mandate to source-level implementation exceptions or update the bootstrap contract accordingly.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: b6b65fd
Reviewed-base: dc05812
reviewer: operator-default @ high

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

The new rule is given to Claude coders but is not reliably visible to the independent Codex reviewer in target repositories. This leaves the central review requirement unenforced.

Review comment:

  • [P1] Publish the policy in instructions Codex loads — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.LZp6YtmLHQ/templates/target-CLAUDE.md:39-47
    Compliance: In target repos, scripts/codex-review.sh runs Codex's built-in review without a custom rubric, and Codex reads AGENTS.md, not this optional Claude-only template. The control-plane REVIEW.md is also unavailable in the target worktree. Therefore an exceptional path can receive a clean independent review without any of these checks. Put the policy in target instructions Codex loads or enforce it through the review manager or harness.

@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

needs-human — round-cap

The final authoritative review of b6b65fd412d81bee3b09de57a3809025f10b4406 leaves one P1 Compliance gap:

  • The rule reaches ystack's own AGENTS.md/REVIEW.md and the Claude coder routines.
  • An independent Codex reviewer inside another target normally reads the target's AGENTS.md; it is not guaranteed to read optional target-CLAUDE.md and cannot see ystack's control-plane REVIEW.md.
  • Therefore policy: exceptional implementation rule #164 does not yet prove that target reviewers will block unexplained exceptions.

Local full gates and remote CI are green, but this PR is not merge-ready. No further code changes are being made at the round cap.

Recommended resolution: create a separate strict-subset issue/PR that delivers the accepted rule through a target AGENTS.md template/generated section or a reviewer-adapter instruction input with one source of truth. Keep #164 open and unmerged until the operator decides whether to split that remainder or explicitly authorizes another bounded revision here.

Proposed follow-up issue creation was not performed because it requires the operator's explicit approval.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Codex reviewer (cross-vendor, read-only)

Reviewed-head: 8dff272
Reviewed-base: dc05812
reviewer: operator-default @ high

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

The new resumable handoff state conflicts with existing tracking behavior, so an interrupted re-entry can become permanently invisible.

Review comment:

  • [P2] Keep resolved handoffs visible until re-entry completes — /var/folders/rm/bkx4f4b91vqfn79vk02pcd140000gn/T/tmp.nPpvTbtaaw/manager/CLAUDE.md:512-514
    If the operator records a ruling but the session ends before the resume checks run, needs-human correctly remains set here, but the Tracking rules later say to skip items the operator already resolved. A later status session will therefore ignore the preserved handoff instead of completing its checks, leaving it stuck indefinitely. Treat resolved-but-still-labeled items as pending re-entry until the label is actually cleared.

@yihanzhu

Copy link
Copy Markdown
Owner Author

Scoped final review disposition

The remaining P2 is not a blocker and receives no code change.

The new re-entry rule says needs-human stays present until the selected branch/PR resume checks pass. An operator ruling alone is not a completed re-entry. routines/brief.md uses the label as the durable signal: it reports items still labeled needs-human and skips them only once resolution clears that label. Therefore a ruled-but-not-yet-resumed handoff remains visible to Tracking instead of being skipped.

The round-cap P1 is resolved by the operator-approved scope-down and strict-subset follow-up #165. This PR remains open and unmerged for human review.

@yihanzhu yihanzhu removed the needs-human Escalation: round cap hit, ambiguous spec, oversized PR, or failure label Aug 28, 2026
@yihanzhu

Copy link
Copy Markdown
Owner Author

The operator-authorized strict-subset follow-up is now recorded as issue #165 and G1 PR #166. PR #166 is CI-green and has a clean authoritative review at e9f51ab29722c927adbc3f0dbe05f501c90dd1a3.

#164 remains scoped to ystack policy/review, universal coder/manager behavior, and the Claude-facing target template. It remains open and unmerged for human review.

@yihanzhu
yihanzhu merged commit ebc53c6 into main Aug 28, 2026
2 checks passed
@yihanzhu
yihanzhu deleted the codex/exceptional-implementation-rule branch August 28, 2026 17:30
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.

Add an exceptional implementation rule

1 participant