Skip to content

Require joined review evidence for workflow delivery readiness - #1064

Merged
taras merged 8 commits into
mainfrom
codex/workflow-delivery-gates
Nov 3, 2026
Merged

taras merged 8 commits into
mainfrom
codex/workflow-delivery-gates

Conversation

@taras

@taras taras commented Nov 1, 2026 •

Copy link
Copy Markdown
Owner

Why

The implementation workflow can finish internal feedback while a required independent review is still missing. Delivery needs a recorded obligation and an observable result rather than an inference from feedback or green CI.

What changes

Before: the workflow returns feedback_ready, and the coordinating agent assembles delivery obligations separately.

After: normal launch records the independent reviewer and original approved acceptance before role execution. A delivery command computes status and next action from the joined review, exact checkout and PR head, required CI and current Main health. Missing evidence stays pending. Failed or uncertain attempts remain available for reconciliation. Delivered status requires an observed merge.

How it works

launch requirements -> implementation feedback -> explicit delivery status/review
                    -> joined review + exact source/CI + current Main health
                    -> await authorized merge -> observed delivery

Review reserves one attempt per revision, joins it, validates every original clause and the assigned commit, and retains the execution and matching canonical receipt. An occupied attempt is preserved rather than retried. Status starts no model, merges nothing and posts no review comments.

The remote adapter checks requested-PR association, head SHA and branch before selecting the latest CI run and attempt. It requires that attempt's successful green job, applies authoritative Main-health selection and rechecks remote and local source.

Launch renders the complete request before provider dispatch. Selected evidence retains its 64,000-character allowance; mandatory initialization and response schema have a separate 256,000-character complete-request ceiling. Response handling fills only omitted metadata known by the workflow and retains explicit conflicts. It invents no verdict, implementation revision or evidence. Provider and correction allocations persist; zero response corrections makes no repair prompt.

Review guide

Start with Delivery/Status.md, Delivery/Review.md and delivery.md beneath examples/adversarial-implementation. Then inspect Phase/Request.md, Phase/Metadata.md and ordinary launch allocation. Check attempt reservation, receipt/execution equality, acceptance coverage and exact PR/CI/Main identity.

How to verify it

  • Delivery and ProductionRole journeys distinguish PR association, branch/SHA mismatches, run/attempt ordering, mandatory initialization, real local TestAgent dispatch and pre-prompt oversized refusal.
  • Focused Deno execution of all six groups and Node ProductionRole passed on the candidate corresponding to signed d77c7b4. Applicable selector, lint/format and production read-only adapter evidence remains retained.
  • Required CI2384 and the Claude Fable affected review passed exact d77c7b4. Three observations are nonblocking. The earlier Astra feature verdict applies to unchanged behavior. Neither review is relabelled onto a newer commit.
  • The bounded Fable closeout passed exact 0e832230ac9bea77d593fbcb1520fe1b5f81ba6c, with no blockers and three retained nonblocking observations. CI2387 passed that head.
  • Current signed head b45bb1668902b89f819aceba217c4267938b7dbb is a clean merge of human-merged Supervise workflow phases through the existing follow stream #1058 at 9b98cbe4. There are no manual source edits. The feature patch retains the same stable patch identity; its literal diff changes README blob hashes and hunk positions after the supervision documentation insertion. Prior reviews are retained at their actual commits. The bounded Fable closeout passed this exact head, with no blocking findings and the three earlier observations retained. The actual joined reply is unchanged; an explicitly recorded invocation-only summary-length exception accepts its 1,308-character summary without another model call. The native delivery status validates this receipt and reports publication_pending only because the PR remains draft: independent review, required CI2390 attempt2, and exact-current-main CI2388 attempt2 all pass. Main is 9b98cbe4; the reviewed branch head remains unchanged. All three approved affected-review/closeout calls are consumed; no merge authority is inferred.
  • Main2388 attempt1 stalled at the known passive-supervision setup point and was cancelled after preserving its state. Its complete log was saved before one retry of the cancelled Node shard and required aggregate passed unchanged source. The branch then reran only its failed main-health gate and aggregate. This restores authoritative health, not the underlying cause: flake #1091 stays open with both official log links and causal-investigation limits.

Actual adoption

Main used the frozen workflow for genuine #1058 normal implementation, original-role corrections/feedback and computed delivery. Integration used it for genuine #1084 initial planning and a truthful discovery hold, then continued required work with the original roles through supported entrypoints. No throwaway qualification call counts as adoption.

This evidence is qualified: checkout preparation failures, a bounded Journal-contract amendment, reviewer-model selection, include precedence and an invocation-only review-length exception remain recorded. These exceptions are not silently adopted into shared source. Integration's initial launch does not claim automatic initial discovery continuation; that separate repair is #1084/PR #1092. Both owners' genuine #1058 supervision uses retain their respective publication owners.

Scope

The adversarial implementation example, documentation and tests. #1062 and #1059 retain their separate owners. No dependency or generated artifact is introduced by this feature. Existing executions and role conversations are retained.

Risks and limitations

Local artifacts are trusted execution records, not remote attestations. This does not replace branch protection. Failed or uncertain effects require reconciliation; there is no automatic retry or allowance reset. Fable observations about launcher documentation, two direct-run fallbacks and test-only permission ordering remain nonblocking and pending.

All current-head gates pass. This PR remains draft pending the human ready-and-merge decision. No merge authority is inferred. Separately queued theory/scope work waits for actual #1064 merge.

Scope confirmation

  • Feature paths support delivery, launch or response handling.
  • Unrelated cleanup is excluded.
  • Main integration and feature changes are distinguished.
  • Historical passing evidence is separate from pending current-head gates.

@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

PR #1064: Require joined review evidence for workflow delivery readiness

51 files, +1266 / -173

Scope

🔴 PR has 1439 lines changed. Split into focused PRs.

🟡 1439 lines changed. PRs under 400 receive more thorough review.

🟡 51 files changed. Are all changes related?

Structural

✅ No structural bloat detected.

Slop

✅ Slop indicators look low.

Static Analysis

✅ Oxlint found no issues.

Correctness

No extraneous code patterns detected.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review of the workflow compatibility correction at 14aeedec0e45eaa9d8d0189f72bb51ca91957019 (delta from reviewed f1aab697): PASS — no findings.

The required Fable request was refused before review because the bundled Claude executable does not support the requested model. Its joined failure is preserved. Under the repository’s authorized fallback policy, an independent Codex Astra (gpt-6-astra, medium) reviewed the same exact commit and original acceptance clauses. The actual joined reply and canonical receipt agree; source stayed unchanged.

The review traced explicit provider/model/effort routing, original sessions, correction budgets, foundation repair and retained continuation through their actual consumers. It reused the passing five-suite focused regression and strengthened ProductionRole witness. No additional tests or UI benchmarks were needed. The affected Planner acceptance review also passed at this commit.

This fulfills the required independent review for the compatibility correction. Required CI, current-main health and genuine full normal-run owner adoption remain separate delivery requirements. Mock tests and standalone Phase use do not establish adoption. This comment authorizes no merge.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Affected independent review: PASS at 9747fed559ce3906211ec5938c15be392085017e.

Codex gpt-6-astra, medium, in the original independent reviewer conversation, reviewed only the three-file CI test partition. All seven original Test bodies and setup are preserved; each group is selected once and the 120-second wrapper timeout is unchanged. The actual Node wrapper passed all six groups; both affected groups passed directly under Deno. No blocking correctness or minimalism findings. Unchanged allocation, budget and review-policy judgments are retained from the prior joined review.

The affected original Planner also passed this signed, clean commit. Required CI remains pending. Full normal-workflow adoption remains incomplete; Taras selected #743 as Main’s next genuine normal run after the compatibility correction is green. This review authorizes no merge. The earlier failed Fable attempt remains preserved; this is its authorized Astra fallback conversation, not an additional full-review stage.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Affected independent review: PASS at b1c75d54fb1bc957f5fd4e709468f9487122ed06.

Codex gpt-6-astra, medium, in the original independent reviewer conversation, reviewed the frozen apiConflict projection and the affected validation/correspondence/downstream routing. The minimal conditional copy preserves the original conflict without weakening validation or equality. The regression fails before the fix and passes afterward; changed and removed conflicts remain rejected within the selected correction bound, with no canonical publication. Focused Deno ProductionRole/Friction and the actual Node wrapper's ProductionRole step pass. Unchanged allocation, budget, partition and review-policy judgments are retained. No blocking findings; no source edits or confidence tests by the reviewer.

The original Planner also passed this signed, clean commit. Required exact-head CI is pending. #743 supplied the genuine failure and remains fenced at its separate configuration-evidence gap; full normal-run adoption and other-owner requirements are not declared complete. This review grants no merge authority and adds no full-review stage.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent affected review by Claude Fable (fable, high) of d77c7b40bd429c0636bb27a5359186b250791c93: PASS, with no blocking findings.

This review covers the Main integration and affected test-launcher/documentation changes since b1c75d54. The production workflow components and entries remain unchanged. It reuses the unchanged feature judgments from the earlier Astra review, without relabelling that historical review.

The reviewer traced the xmd test/run --eval TestAgent difference, test-only permission plugin scoping, and retained assertions and deadlines. It reused the passing six-group Deno, Node ProductionRole and exact-head CI evidence; no test suites were rerun for this review. Recorded acceptance G1–G5 is covered.

Nonblocking observations, retained as pending:

  1. examples/adversarial-implementation/README.md line 356 (Test without paid agents) states the launcher wraps each Markdown suite in and enables absolute host paths. At d77 that is untrue for one of six suites: packages/cli/tests/adversarial-implementation.test.ts lines 31-38 run ProductionRole under xmd test with --plugin ./packages/cli/tests/fixtures/workflow-test-permissions.mjs, not inside the run --eval wrapper. The merge took main's sentence while changing the launcher. Correction: state that five suites run through run --allow-absolute --eval and ProductionRole runs under xmd test with the test-only permission plugin, because run installs no TestAgent and test carries no --allow-absolute flag.

  2. examples/adversarial-implementation/tests/Delivery.test.md line 8 and LaunchPolicy.test.md line 8 keep runner fallbacks ending in plain run, while main updated the Workflow, Friction and Implementation fallbacks to run --allow-absolute. The fallback is reached only when XMD_EXAMPLE_COMMAND is unset, and the launcher always sets it, so the receipts and CI are unaffected. A direct Markdown invocation would refuse: Fixture.Entry launches DeliveryCase.md, which opens absolute

    paths (lines 14, 24, 29). Align the two fallbacks with main's three.

  3. Minimalism: packages/cli/tests/fixtures/workflow-test-permissions.mjs installs its absolute allowance at the default max order and answers true without delegating, whereas useAbsolutePermission(true) in packages/cli/src/cli.ts line 259 installs the identical function at { at: "min" }. The behavior is equivalent here because the plugin is the outermost installation in the test command scope and any nested restriction is asked first, but passing { at: "min" } would mirror the production flag exactly and keep the runtime permissions doc's rule against answering true early. Optional.

Limitation retained from the review: no separate live-provider experiment was run at this integration commit. The existing adoption evidence and its qualifications remain recorded separately. This review does not authorize merge.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Fable 5.1 (high) — bounded closeout at 0e83223: PASS, G1–G5 covered, no blocking findings.

This continues the affected review at d77c7b40. The complete feature patch relative to its corresponding Main base remains byte-for-byte identical. The reviewer inspected the newly merged #1090 Claude adapter guard and traced startup refusal through the ACP client into the workflow’s captured execution. No conflicting continuation, permission, lifetime or ownership behavior was found. The change makes contradictory admission fail closed earlier; it does not cause a relaunch.

The three earlier nonblocking observations remain pending: launcher documentation, the two direct-run fallback flags, and optional test permission-hook ordering. The reviewer also corrected the earlier summary’s shorthand: component and entry blobs are unchanged from b1, while three Markdown suites each gained an absolute-path fallback flag.

Evidence: exact-head CI2387 passed. The closeout used source and saved evidence; it added no suite run or live-provider qualification. Historical genuine-use records and deterministic TestAgent evidence retain their original revisions. The joined execution, raw reply and canonical review receipt match. This verdict applies to this commit; current Main health, branch protection and any later head remain delivery gates.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Fable 5.1 (high) — bounded closeout at b45bb1668902b89f819aceba217c4267938b7dbb: PASS, G1–G5 covered, no blocking findings.

This continues the 0e closeout. The signed merge recomputes to the exact head tree. The feature stable patch identity is unchanged; literal diff bytes differ through README hashes and hunk positions. The reviewer traced the newly merged #1058 supervision/follow/Journal consumers and found no conflict with delivery attempt occupancy, exact receipt matching, permissions or outcome ownership. #1058's full accepted behavior was not re-reviewed. No source changes, broad suites or live-provider qualification were performed.

Three retained nonblocking observations remain pending:

  1. Retained from d77/0e, unchanged at b45bb16 (line numbers shifted by the #1058 insertion): examples/adversarial-implementation/README.md line 496 under 'Test without paid agents' (heading line 490) states the launcher wraps each Markdown suite in and enables absolute host paths. That is untrue for one of six suites: packages/cli/tests/adversarial-implementation.test.ts lines 31-38 run ProductionRole under xmd test with --plugin ./packages/cli/tests/fixtures/workflow-test-permissions.mjs, not inside the run --allow-absolute --eval wrapper. Correction: state that five suites run through run --allow-absolute --eval and ProductionRole runs under xmd test with the test-only permission plugin.

  2. Retained from d77/0e, unchanged at b45bb16: examples/adversarial-implementation/tests/Delivery.test.md line 8 and LaunchPolicy.test.md line 8 keep runner fallbacks ending in plain run, while Workflow.test.md line 8 (and Friction, Implementation per #1074) end in run --allow-absolute. The fallback is reached only when XMD_EXAMPLE_COMMAND is unset, and the launcher always sets it, so receipts and CI are unaffected. A direct Markdown invocation would refuse because DeliveryCase.md opens absolute

    paths. Align the two fallbacks with the other three.

  3. Retained from d77/0e, unchanged at b45bb16 (minimalism): packages/cli/tests/fixtures/workflow-test-permissions.mjs installs its absolute allowance at the default order and answers true without delegating, whereas useAbsolutePermission in packages/cli/src/cli.ts line 258 installs the identical function at { at: "min" }. Behavior is equivalent here because the plugin is the outermost installation in the test command scope, but passing { at: "min" } would mirror the production flag exactly. Optional.

The additional evidence note preserves the absence of live-provider proof at this head and the separation between supervision completion and the actual workflow verdict.

The actual review execution completed and joined. Its unchanged summary was 1,308 characters, exceeding the wrapper's 1,200-character formatting limit. An explicitly recorded invocation-only length exception validates the same raw reply, reviewer, exact source and all clauses; no summary rewrite, extra formatting call or repeat review occurred. Shared workflow source is unchanged. Required CI2390 and exact-current-main health remain separate pending delivery gates. This review grants no merge authority.

@taras
taras marked this pull request as ready for review November 3, 2026 19:31
@taras
taras merged commit 98cc6f1 into main Nov 3, 2026
112 of 114 checks passed
@taras
taras deleted the codex/workflow-delivery-gates branch November 3, 2026 19:32
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