diff --git a/AGENTS.md b/AGENTS.md index 92df235..c961b08 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,12 +1,15 @@ # Agent -Read [CONTRIBUTING.md](CONTRIBUTING.md) and [DESIGN.md](DESIGN.md) before +Read [CONTRIBUTING.md](CONTRIBUTING.md), [DESIGN.md](DESIGN.md), and +[docs/pull-request-lifecycle.md](docs/pull-request-lifecycle.md) before changing this repository. Skill contracts live next to the client: `agent skills path` (spine, review-loop, pr-review, error-fix). -A draft plus local tests is not done. Pull-request done is signed commits -on a branch in this repository, grok quality and logic then Codex quality -and logic on this head with zero findings, CI green on this head, then -ready. The authoring session does not sit those PR reviews. A human merges. -The local-CI comment schema for private product repositories is -`docs/local-ci-v1.md`. +Draft publication is immediate after the first signed task commit; see the +lifecycle. A draft plus local tests is not done. Ready for review is signed +commits on a branch in this repository, grok quality and logic then Codex +quality and logic on this head with zero findings, CI green on this head, then +leave-draft. Ready for review is still not merge and not completion. The +authoring session does not sit those PR reviews. A human merges; claim +completion only after that merge is verified. The local-CI comment schema for +private product repositories is `docs/local-ci-v1.md`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c23f5ae..7d2754c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,19 +2,19 @@ - Branch from `develop`. Never push to `develop` or `main`. - Push the branch to this repository. Do not open the pull request from a personal fork. -- Open a draft pull request. Stay draft until the pull request is **done** (below). A human merges. +- As soon as the first signed task commit exists, push and open a **draft** pull request immediately ([docs/pull-request-lifecycle.md](docs/pull-request-lifecycle.md)). Stay draft until **Ready for review** (below). A human merges; only then is the pull request completed. - Sign commits with the GitHub identity that owns the commits. - Public repository: English for commits and comments. The visible pull-request summary is an `EN:` block, optionally followed by a labeled `DE:` block. - Do not name private repositories, internal hostnames, or internal infrastructure. - Add or update tests in the same change. -- Run `pytest` before you push. Tests need PostgreSQL (`AGENT_TEST_PG` or a local `initdb`). -- Pytest (or any green local suite) is a **check**, not done. +- Run `pytest` on the exact clean signed final head before Ready for review. Tests need PostgreSQL (`AGENT_TEST_PG` or a local `initdb`). Full pytest is not a gate for the first draft publication. +- Pytest (or any green local suite) is a **check**, not Ready for review and not completion. -## Pull request done +## Ready for review -A draft plus local tests is not done. Do not claim the pull request is finished at that point. +A draft plus local tests is not done. Do not claim the pull request is finished, done, or completed at that point — including after leave-draft. Draft timing and CI ownership while the draft is open are defined in [docs/pull-request-lifecycle.md](docs/pull-request-lifecycle.md). -Done is all of: +Ready for review requires all of: 1. Signed commits on a branch in this repository, based on `develop`. 2. Four lane verdicts on **this** head, two vendor stages: grok quality and grok logic in parallel, then Codex quality and Codex logic. Quality/conformance reads this file first. The session that authored the diff does not sit those reviews. @@ -22,8 +22,8 @@ Done is all of: 4. Zero findings only after an explicit complete pass. Empty, partial, timeout, or unavailable output is not zero findings. Iterate until all four lane verdicts on this head are approved. 5. Inner implement/review rounds (`review-loop`) are not the PR reviews (`pr-review`). 6. CI green on **this** head. This public repository uses GitHub Actions. `skipped` and `cancelled` are not green unless the workflow documents that skip. The local-CI comment schema for **private** product repositories is defined in [docs/local-ci-v1.md](docs/local-ci-v1.md) and verified by `agent local-ci verify`. -7. Stay draft until the reviews and CI above hold on this head. Then one comment whose review-pass count is those four `approved` verdicts on this head, then mark the GitHub pull request ready. When spine and pr-review are attached, `agent allow --action pr-ready` only checks task state (`pushing` or `pr-review`); it is not the leave-draft verdict. Do not mark ready if it denies. -8. A human merges. When spine is attached, `agent allow --action task-done` still needs the workflow checklist and both summary sentences. +7. Stay draft until the reviews and CI above hold on this head. Then one comment whose review-pass count is those four `approved` verdicts on this head, then mark the GitHub pull request ready for review (`isDraft=false`). When spine and pr-review are attached, `agent allow --action pr-ready` only checks task state (`pushing` or `pr-review`); it is not the leave-draft verdict. Do not mark ready if it denies. Ready for review is still not merge and not completion. +8. A human merges. Claim pull-request completion only after that merge is verified. When spine is attached, `agent allow --action task-done` still needs the workflow checklist and both summary sentences; that ledger state is not proof of pull-request completion. The AI inserts `pr.open` / `comment.post`; a rejected review gate inserts `review.post`. `agent github pending` performs GitHub HTTP. A retry reuses the existing draft. diff --git a/DESIGN.md b/DESIGN.md index add7084..c4f257b 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -44,7 +44,7 @@ The AI session talks **only** to the local database. Scripts perform every actio | Outside facts | Scripts notice GitHub (and other outside) state. The agent is not told by a human and does not poll GitHub. Example: a recorded PR merges → script writes `pr.merged` on that session and knocks. | | AI vs scripts | The AI inserts local intent. Scripts perform every side effect that leaves the machine. Model text is never a state transition. | | Checks and gates | A **check** records a fact (`agent check record`). A **gate** is a policy verdict over evidence (`agent gate record`). A model claim is neither. Confidence is not proof. | -| Pull request done | A draft plus local tests is not done. CONTRIBUTING.md is the contract for this repository. When spine and pr-review are attached, grok then Codex on this head are the gates; `agent allow --action pr-ready` only checks task state. A human merges. | +| Ready for review | Draft publication is immediate after the first signed task commit ([docs/pull-request-lifecycle.md](docs/pull-request-lifecycle.md)). A draft plus local tests is not done. CONTRIBUTING.md is the contract for this repository. When spine and pr-review are attached, grok then Codex on this head are the gates; `agent allow --action pr-ready` only checks task state. Leave-draft is Ready for review, not completion. A human merges; claim completion only after that merge is verified. | | Local CI report | Frozen comment schema `dfx-local-ci/v1` in [docs/local-ci-v1.md](docs/local-ci-v1.md). `agent local-ci verify` parses it and computes pass/fail. Private product repositories attach the block to the ready comment. This public client still uses GitHub Actions for its own PRs. | | A38 | The complete central process is [docs/a38.md](docs/a38.md) plus [docs/a38-guard.md](docs/a38-guard.md). An adopter keeps only `.github/a38.json`, its integration, and a contributing pointer; plugins use short central pointers. `agent a38` runs and validates the existing report format, while `agent pr-guard` checks live author evidence without executing PR code. Private trusted-base opt-in may provide local code-gate equivalence; public/default GitHub CI, independent required GitHub-only checks, review gates, technical merge restrictions, and human merge remain unchanged. | | Merge | The client never merges. A human merges. | @@ -508,7 +508,7 @@ The rules below were already implied by §§1–17. They are now explicit so a l | Whether a command ran and what it returned | The process that ran it | `agent check record` (name, command, `pass`/`fail`/`skip`, output) | | Merge | Human | Never a client command | -A worker report such as “analysis complete” or “tests passed” is **input**. It is not the transition. Opening a draft is not done. +A worker report such as “analysis complete” or “tests passed” is **input**. It is not the transition. Opening a draft is not done. Leaving draft is Ready for review, not completion. No transition that needs deterministic evidence may be satisfied by model text alone. Malformed structured output is rejected (unknown `activity.type` → `execution_status=error`; empty, partial, timeout, or unavailable review output is not zero findings). A patch that does not apply is a failed check, not a debate. @@ -549,13 +549,13 @@ Partial multi-step actions (push, then open a pull request) record each complete Write owner, hub role, opt-in skills, required session row, and the generic `activity` catalog stay as in §2. Spine task states stay the spine skill’s states. They are not replaced by a hub machine such as `CREATED` / `ANALYZING` / `READY_FOR_PR`. -### 19.6 Pull request done +### 19.6 Ready for review -A draft plus local tests is not done. A check records the local suite. When spine and pr-review are attached, a gate records each vendor dimension on **this** head, and leave-draft is four lane verdicts (grok quality and grok logic, then Codex quality and Codex logic) on this head plus CI green on this head. Without those skills, the target repository’s written contributing rules apply. `agent allow --action pr-ready` only checks that a task is in `pushing` or `pr-review`; it is not the leave-draft verdict. `task-done` still needs the workflow checklist and both summary sentences. +Draft publication timing is [docs/pull-request-lifecycle.md](docs/pull-request-lifecycle.md): open the draft as soon as the first signed task commit exists; full tests and reviews are Ready-for-review gates, not draft gates. A draft plus local tests is not done. A check records the local suite. When spine and pr-review are attached, a gate records each vendor dimension on **this** head, and leave-draft (Ready for review) is four lane verdicts (grok quality and grok logic, then Codex quality and Codex logic) on this head plus CI green on this head. Without those skills, the target repository’s written contributing rules apply. `agent allow --action pr-ready` only checks that a task is in `pushing` or `pr-review`; it is not the leave-draft verdict. `task-done` still needs the workflow checklist and both summary sentences; ledger `task-done` is not pull-request completion. Quality and logic of one vendor stage run together. Vendors are `grok`, then `codex`. Codex runs only after both grok dimensions are `approved`. The session that authored the diff does not sit those PR reviews. If a vendor cannot run, abort loudly; do not record `approved`; do not substitute another vendor. Empty, partial, timeout, or unavailable review output is not zero findings. -CI on this head is a script-measured fact whose applicability comes from the target repository's written rules. The frozen `dfx-local-ci/v1` format and legacy `agent local-ci verify` behavior do not by themselves adopt A38 or grant permission to skip GitHub CI. For A38 adopters, follow the central [A38 standard](docs/a38.md) and [guard guide](docs/a38-guard.md): private visibility alone is not opt-in, and private local code-gate equivalence requires trusted-base opt-in through a valid A38 manifest, assessment against the canonical active policy, and a separate live join against the actual latest report-like GitHub comment by the PR author. Public A38 adopters publish and validate the author report in addition to retaining cumulative GitHub CI; private repositories without that trusted-base opt-in and non-A38 repositories retain their existing written CI rules. For applicable GitHub CI checks, `skipped` and `cancelled` are not green unless the workflow documents that skip. Independently required GitHub-only checks, technical merge restrictions, review gates, and human merge remain required. Stay draft until the applicable rules hold. One comment whose review-pass count is those four `approved` verdicts on this head, then ready. A retry reuses the existing draft. A human merges. +CI on this head is a script-measured fact whose applicability comes from the target repository's written rules. The frozen `dfx-local-ci/v1` format and legacy `agent local-ci verify` behavior do not by themselves adopt A38 or grant permission to skip GitHub CI. For A38 adopters, follow the central [A38 standard](docs/a38.md) and [guard guide](docs/a38-guard.md): private visibility alone is not opt-in, and private local code-gate equivalence requires trusted-base opt-in through a valid A38 manifest, assessment against the canonical active policy, and a separate live join against the actual latest report-like GitHub comment by the PR author. Public A38 adopters publish and validate the author report in addition to retaining cumulative GitHub CI; private repositories without that trusted-base opt-in and non-A38 repositories retain their existing written CI rules. For applicable GitHub CI checks, `skipped` and `cancelled` are not green unless the workflow documents that skip. Independently required GitHub-only checks, technical merge restrictions, review gates, and human merge remain required. Stay draft until the applicable rules hold. One comment whose review-pass count is those four `approved` verdicts on this head, then mark Ready for review (`isDraft=false`). A retry reuses the existing draft. Ready for review is still not merge and not completion. A human merges; claim completion only after that merge is verified. ## 20. Refused: hub as a coding control plane @@ -670,8 +670,8 @@ On `error.fix`: 1. `agent task create --workflow implement --error-id ` on this session (find-or-create). That copies `error_id` and `repo` from the `error.seen` row into the task payload. 2. Isolated worktree of that task `payload.repo` at the allowed base revision. Git operations are scripts. `payload.repo` is already on the task because analysis refused `error.fix` when `repo` was missing. Never fall back to the origin checkout. -3. Spine implement: mandatory checks must `pass`, then `pr.open` opens a **draft** (spine `pushed`). Title/body may be model-drafted; the GitHub API call is a script. A retry finds an existing draft for this fingerprint instead of opening a second one. -4. pr-review gates run on that head after `pushed`. +3. As soon as the first signed task commit exists, `pr.open` opens a **draft** through the existing executor flow ([docs/pull-request-lifecycle.md](docs/pull-request-lifecycle.md)). Title/body may be model-drafted; the GitHub API call is a script. A retry finds an existing draft for this fingerprint instead of opening a second one. Mandatory checks and the spine checklist key `pushed` remain final validated bookkeeping; they are not prerequisites for that first draft publication. +4. pr-review gates and applicable full checks run on the final head before Ready; closing spine `pushed` records the validated push step, not the early draft open. 5. A human merges. `pr.merged` knocks as today. The model never receives production credentials. Analysis that only reads the excerpt does not need write access to the origin branch. diff --git a/docs/a38-guard.md b/docs/a38-guard.md index e7ae617..4018160 100644 --- a/docs/a38-guard.md +++ b/docs/a38-guard.md @@ -98,7 +98,7 @@ For each reviewer, the latest substantive submitted state controls authorization An authorized migration may introduce, remove or change workflows, and may also add, remove or change `.github/pr-guard.json`, but the complete current head inventory and author report still must satisfy the approved head policy. Proposed pr-guard configuration is never used for scope until it is merged to the trusted default-branch revision. The executable guard remains pinned and never runs head commands. The **base policy's enforcement mode stays active** for this PR, even if the proposed mode is `observe`. If no valid base policy exists, explicit approval permits bootstrap under `enforce`; initial adoption cannot silently bypass reporting. -An unpublished migration proposal is first measured and verified against the still-active base policy and pushed at that same commit; that run is proposal publication evidence, not Ready evidence. Once the exact current head/base approval exists, rerun the full approved head policy and local verification, publish that newly generated report, and perform the live join. Bootstrap with a missing or invalid base policy instead follows the repository's existing pre-push checks to publish the proposal, then requires explicit approval and `enforce`; it must not invent a report or waive approval. +Publish the migration proposal draft first per the [pull request lifecycle](pull-request-lifecycle.md). Measurement and verification against the still-active base policy may follow on that same signed commit; that run is proposal evidence, not Ready evidence. Once the exact current head/base approval exists, rerun the full approved head policy and local verification, publish that newly generated report, and perform the live join. Bootstrap with a missing or invalid base policy likewise publishes the draft first under the lifecycle (full pre-push test gates are not a draft blocker), then requires explicit approval and `enforce`; it must not invent a report or waive approval. The bot identifies the active policy revision in its comment. Download `.github/a38.json` from that exact revision before generating the report. For ordinary PRs, this is the base; for explicitly approved migrations, it is the head. @@ -114,7 +114,7 @@ agent a38 run --repo . --repository OWNER/NAME \ `--repository` identifies the target repository, especially when the checkout origin is a fork. Post the complete generated report as a PR comment using the **PR author's account**. Preserve its JSON and markers. The existing local-CI wire schema remains unchanged for compatibility. -For the private opt-in process, the checkout must be clean at the final repository-required signed commit before measurement. Run the full active policy and locally verify it before push, record that verified report as `local_check_pass` evidence, then push the same SHA without an intervening commit. Any fix, amend, or rebase creates a new SHA and requires the complete run and verification again. Execution roles follow the repository's orchestration rules; reviewers remain read-only. Job adapter commands are catalogued in [A38 job adapters](a38-job-adapters.md), without duplicating their schemas here. +For the private opt-in process, the checkout must be clean at the final repository-required signed commit before Ready measurement. The draft may already exist under the [pull request lifecycle](pull-request-lifecycle.md). Run the full active policy and locally verify it before recording `local_check_pass` evidence; that verified SHA must be on the open draft with no intervening commit after measurement. Any fix, amend, or rebase creates a new SHA and requires the complete run and verification again. Execution roles follow the repository's orchestration rules; reviewers remain read-only. Job adapter commands are catalogued in [A38 job adapters](a38-job-adapters.md), without duplicating their schemas here. The latest author report-like comment, ordered by `updated_at` and numeric comment ID, is authoritative. A newer malformed or failed report never falls back to an older success. Other authors' reports cannot satisfy the requirement. Matching repository, head, visibility, full job set, names, commands, timeouts and successful measured results are mandatory, including for public repositories. diff --git a/docs/a38.md b/docs/a38.md index 0e22d7f..2b49cef 100644 --- a/docs/a38.md +++ b/docs/a38.md @@ -42,9 +42,9 @@ The guard reads the A38 manifest at the PR's immutable base SHA. A report cannot After that approval, the exact approved head manifest becomes the active policy data, but the executable guard remains pinned and the base policy's enforcement mode remains effective until merge. The full workflow inventory and report are then reassessed against the head policy. A new head or base invalidates the approval and requires a new exact approval and full reassessment. -For a new, not-yet-published migration proposal, first run and verify the final proposal commit against the still-active base policy, then push that same commit. This proposal run is not Ready evidence. After exact approval, fetch the approved head policy and repeat the complete run, local verification, author publication, and live join against the current head policy. Do not use an unapproved head manifest as the active policy. +For a new migration proposal, publish the draft first per the [pull request lifecycle](pull-request-lifecycle.md). Proposal measurement against the still-active base policy may follow that publication on the same signed commit; this proposal run is not Ready evidence. After exact approval, fetch the approved head policy and repeat the complete run, local verification, author publication, and live join against the current head policy. Do not use an unapproved head manifest as the active policy. -If the base manifest is missing or invalid, bootstrap requires the same independent explicit approval of the exact current head and base and is always `enforce`; absence is never a waiver. Before a bootstrap proposal can have an active A38 base policy, follow the repository's existing pre-push checks to publish the proposal. For review, the exact proposed head policy may be loaded outside the checkout and used to run the actual complete tests and produce a genuine author report, clearly marked as `PROPOSED` evidence; this does not activate the policy, substitute for approval, confer private A38 Ready equivalence before a valid base policy exists, or satisfy the final Ready live join. After approval, repeat the complete measured run and author report against the now-active policy, perform the live join, and rerun whenever required; the evidence and current policy must match. Never handcraft or fabricate a report. See the guard instructions for event handling and operational limits. +If the base manifest is missing or invalid, bootstrap requires the same independent explicit approval of the exact current head and base and is always `enforce`; absence is never a waiver. Publish the bootstrap proposal draft first per the [pull request lifecycle](pull-request-lifecycle.md); full pre-push test gates are not a draft-publication blocker. For review, the exact proposed head policy may be loaded outside the checkout and used to run the actual complete tests and produce a genuine author report, clearly marked as `PROPOSED` evidence; this does not activate the policy, substitute for approval, confer private A38 Ready equivalence before a valid base policy exists, or satisfy the final Ready live join. After approval, repeat the complete measured run and author report against the now-active policy, perform the live join, and rerun whenever required; the evidence and current policy must match. Never handcraft or fabricate a report. See the guard instructions for event handling and operational limits. ## Adoption and CI boundaries @@ -58,9 +58,9 @@ Public adopters, private repositories without that trusted-base opt-in, and all Execution follows the adopting repository's orchestration and role rules. Reviewers remain read-only and do not run A38 as a review lane. -For the private opt-in path, finish the changes and any rebase first, create the repository-required signed commit, and require a clean checkout—including no untracked files—at that exact final SHA before measuring. Install the repository's documented tools and local dependencies first. Obtain the current base SHA and the active `.github/a38.json`; keep the trusted external policy copy, report, and logs outside the checkout so the PR cannot replace evidence inputs or outputs. +For the private opt-in path, finish the changes and any rebase first, create the repository-required signed commit, and require a clean checkout—including no untracked files—at that exact final SHA before measuring for Ready. The draft may already exist under the [pull request lifecycle](pull-request-lifecycle.md). Install the repository's documented tools and local dependencies first. Obtain the current base SHA and the active `.github/a38.json`; keep the trusted external policy copy, report, and logs outside the checkout so the PR cannot replace evidence inputs or outputs. -Public adopters run the same complete manifest and report validation using `--public`, but retain their repository's existing commit/push order and cumulative GitHub CI gates; the private pre-push equivalence below does not apply. +Public adopters run the same complete manifest and report validation using `--public`, follow the [pull request lifecycle](pull-request-lifecycle.md) for draft timing, and retain their cumulative GitHub CI gates for Ready; the private local-code-gate equivalence below does not apply. For example, after replacing the paths and SHA with actual values: @@ -75,7 +75,7 @@ The runner identifies the GitHub repository from `origin` by default; for a fork Job adapter behavior is defined separately in [A38 job adapters](a38-job-adapters.md). Use `agent a38 job --config ''`; this standard intentionally does not duplicate adapter schemas. -Inspect the results, fix failures, and repeat the complete run on the final clean head. In the private opt-in path, locally verify the saved report before push: +Inspect the results, fix failures, and repeat the complete run on the final clean head. In the private opt-in path, locally verify the saved report before recording Ready evidence: ```sh agent a38 verify --policy /tmp/a38-run/policy.json \ @@ -83,7 +83,7 @@ agent a38 verify --policy /tmp/a38-run/policy.json \ --head HEAD_COMMIT_SHA --private ``` -Only a complete `run` plus successful local `verify` for the signed, clean final SHA may be recorded as `local_check_pass`. Push that same SHA with no intervening commit. Any fix, amend, rebase, or other new SHA requires a new signed clean commit and a complete run and verification from the beginning. +Only a complete `run` plus successful local `verify` for the signed, clean final SHA may be recorded as `local_check_pass`. That final SHA must be on the open draft with no intervening commit after the verified measurement. Early draft publication may precede this final measurement ([pull request lifecycle](pull-request-lifecycle.md)). Any fix, amend, rebase, or other new SHA requires a new signed clean commit and a complete run and verification from the beginning. Post the generated `report.md` unchanged as a PR comment **using the PR author's GitHub account**, preserving its markers and fenced JSON block. Publishing is separate from running and verification. Do not paste raw logs containing credentials into the comment and do not hand-edit or reconstruct a passing payload. diff --git a/docs/pull-request-lifecycle.md b/docs/pull-request-lifecycle.md new file mode 100644 index 0000000..f2adf78 --- /dev/null +++ b/docs/pull-request-lifecycle.md @@ -0,0 +1,76 @@ +# Pull request lifecycle + +This document is the **canonical** central rule for when a draft pull request is published, what may wait until after publication, and what Ready for review / completion still requires. Entrypoints in this repository link here. Tool plugins add only a short pointer; they do not redefine the standard. + +Session-specific review waivers are not a global default. + +## Status terms (canonical) + +Use these terms only. Do not call an earlier stage finished, done, or completed. + +| Term | Meaning | +|---|---| +| **Draft** | Open GitHub pull request with `isDraft=true`. Never finished, done, or completed — even when tests pass. | +| **Ready for review** | Leave-draft transition (`gh pr ready` / `isDraft=false`) after all required checks and reviews on the exact clean signed final head. Still not merged and still not completed. | +| **Merged / completed** | Only after a **verified human merge**. Ledger `task-done`, checklist closes, spine states, and Ready for review are **not** proof of pull-request completion. | + +A draft plus local tests is still **not** Ready for review and **not** completed. + +## Draft publication + +As soon as the **first signed task commit** exists on the feature branch, **push** that commit and **open a draft pull request immediately** through the existing activity and scoped executor flow (`pr.open`, then `agent github pending` or the equivalent knock-driven scan). + +- Reuse a matching open pull request for the same session, repository, and base; do not open a second one. +- Do **not** wait for full local test suites, A38 author reports, live joins, or independent reviews before that first draft publication. +- Before publication, perform only the **basic** secret, scope, and signature checks needed to publish safely. Those are not full test gates. +- An explicit stop, or missing permission to publish, **blocks** draft publication and must be reported promptly. Do not invent a substitute path. + +Early draft publication does **not** mark spine checklist keys such as `local_check_pass` or `pushed` as passed, and it does not claim tests, A38 evidence, or reviews are complete. `agent allow --action pr-create` already permits draft creation; no separate runtime switch is required for this rule. + +The spine checklist key `pushed` remains **final validated push bookkeeping** after the applicable measured checks for that workflow step. Early draft publication via `pr.open` is allowed independently of that checklist close. + +## After the draft exists + +Work continues on the same draft. Proposal measurement for A38 migrations or bootstrap may follow publication; it is not a precondition for opening the draft. + +Applicable full tests, A38 author evidence, current-base policy checks, and the live join remain required for **Ready for review** on the exact clean signed final head. Independently required GitHub checks and repository review gates also remain required unless a separately granted deviation says otherwise. Do not encode a one-off session waiver as the standing rule. Completion still requires human merge. + +## CI while the draft is open + +Hosted CI and other applicable checks may fail. There is no promise that CI never fails. + +**Red CI is a blocker owned by the author:** + +- Inspect the **actual** failing logs for the current head. +- Fix the root cause. +- Rerun the **actual** affected checks on the current head. +- Do not hide, skip, or override failures. +- Do not claim green from local results alone when the required hosted check is red or missing. + +Pending checks must be labeled **pending**. Do not fabricate a pass. + +If the repository's guard integration is known to be defective, require a **verified** rollout of the fixed integration before Ready for review. Do not instruct merging through red statuses. + +## Ready for review + +Stay draft until Ready for review is earned on the **exact clean signed final head**: + +1. Full applicable tests for that head (repository rules and, when adopted, the complete A38 policy run and local verification). +2. For A38 adopters: author report publication, current-base (or exact approved head) policy checks, and the live join required by [a38.md](a38.md) and [a38-guard.md](a38-guard.md). +3. Independently required GitHub checks on this head (`skipped` and `cancelled` are not green unless the workflow documents that skip). +4. Independent required reviews and approvals per the attached skills and the target repository's written rules. +5. Then the Ready comment / leave-draft steps those rules define (`isDraft=false`). + +`agent allow --action pr-ready` only checks task state when spine is attached; it is not itself the leave-draft verdict. Leaving draft is **Ready for review**, not pull-request completion. + +## Merge / completion + +A human merges. The client never merges. Report the pull request as completed only after that merge is verified. + +## Related documents + +- [CONTRIBUTING.md](../CONTRIBUTING.md) — repository contributing contract; defers lifecycle timing to this file +- [AGENTS.md](../AGENTS.md) — short agent entrypoint +- [DESIGN.md](../DESIGN.md) — product rules; error-fix opens drafts under this lifecycle +- Spine and pr-review skills — checklist bookkeeping and review gates +- [a38.md](a38.md) / [a38-guard.md](a38-guard.md) — A38 measurement, report, and Ready join (draft timing follows this lifecycle) diff --git a/src/agent_cli/skills/pr-review/SKILL.md b/src/agent_cli/skills/pr-review/SKILL.md index 842cf8c..d5b8f77 100644 --- a/src/agent_cli/skills/pr-review/SKILL.md +++ b/src/agent_cli/skills/pr-review/SKILL.md @@ -90,7 +90,11 @@ does not have the requirement. ## Pull requests -The agent does not merge. Open pull requests as drafts; a human merges. +The agent does not merge. Open pull requests as drafts as soon as the first +signed task commit exists +([pull request lifecycle](../../../../docs/pull-request-lifecycle.md)); a human +merges. Full tests and these review gates are Ready requirements, not draft +publication gates. A draft plus local tests is not done. Quality and logic of one vendor stage run in parallel on **this** head. The session that authored the diff does not @@ -112,14 +116,16 @@ human merge remain required. `agent allow --action pr-ready` only checks task state; do not mark ready if it denies. Then one comment whose review-pass count is those four `approved` verdicts on this head, then mark the GitHub -pull request ready. +pull request Ready for review (`isDraft=false`). That leave-draft step is +not merge and not pull-request completion. ## Approving Once all four lane verdicts on **this** head are `approved` and CI on this head is green, insert a `review.post` with `event: APPROVE` alongside the pass-count comment. -That is a review this account submits on the pull request, not a merge: the agent -still does not merge, and a human still does. +That is a review this account submits on the pull request, not a merge and not +completion: the agent still does not merge, and a human still does. Claim +completion only after that human merge is verified. `APPROVE` is only for that state. A rejected gate publishes `COMMENT`, never `APPROVE` and never `REQUEST_CHANGES` — the executor refuses the last one, because an diff --git a/src/agent_cli/skills/spine/SKILL.md b/src/agent_cli/skills/spine/SKILL.md index 65d56c9..a6915b4 100644 --- a/src/agent_cli/skills/spine/SKILL.md +++ b/src/agent_cli/skills/spine/SKILL.md @@ -58,9 +58,18 @@ Do not invent keys. Chains: `grok_pr_logic`, `codex_pr_quality`, `codex_pr_logic`, `mergeable` `done` still requires the workflow checklist and both summary sentences -(`agent task summary`). `local_check_pass` and inner `reviewer_approved` are -not pull-request done. A draft plus local tests is not done. See pr-review -and CONTRIBUTING.md. +(`agent task summary`). That ledger `task-done` / checklist close is not +Ready for review and not pull-request completion. `local_check_pass` and +inner `reviewer_approved` are not Ready for review. A draft plus local +tests is not done. See pr-review and CONTRIBUTING.md. + +Draft publication timing is the central +[pull request lifecycle](../../../../docs/pull-request-lifecycle.md): as soon +as the first signed task commit exists, open the draft via `pr.open` (executor +flow; `allow pr-create` already permits draft). That early publication does +**not** close `local_check_pass`, `pushed`, or any test/review checklist key. +The checklist key `pushed` is final validated push bookkeeping after the +applicable measured checks for that step. For A38 work, follow the central [A38 standard](../../../../docs/a38.md) and [guard guide](../../../../docs/a38-guard.md); this skill is only a pointer. diff --git a/tests/test_pr_done_contract.py b/tests/test_pr_done_contract.py index cfbfa07..43ded52 100644 --- a/tests/test_pr_done_contract.py +++ b/tests/test_pr_done_contract.py @@ -11,14 +11,20 @@ NOT_DONE = "A draft plus local tests is not done" -def test_contributing_states_pr_done_contract() -> None: +def _ws(text: str) -> str: + """Collapse whitespace so prose line-wrapping is not a contract failure.""" + return " ".join(text.split()) + + +def test_contributing_states_ready_for_review_contract() -> None: text = (ROOT / "CONTRIBUTING.md").read_text(encoding="utf-8") + prose = _ws(text) assert NOT_DONE in text assert "Push the branch to this repository" in text assert "Do not open the pull request from a personal fork" in text assert "Four lane verdicts" in text assert "those four `approved` verdicts on this head" in text - assert "do not substitute another vendor" in text + assert "do not substitute another vendor" in prose assert "Empty, partial, timeout, or unavailable" in text assert "is not zero findings" in text assert "`skipped` and `cancelled` are not green" in text @@ -26,43 +32,54 @@ def test_contributing_states_pr_done_contract() -> None: assert "Do not mark ready if it denies" in text assert "it is not the leave-draft verdict" in text assert "A human merges" in text - assert "session that authored the diff does not sit those reviews" in text + assert "session that authored the diff does not sit those reviews" in prose + assert "Ready for review is still not merge and not completion" in prose + assert "finished, done, or completed at that point" in prose -def test_design_locks_pr_done() -> None: +def test_design_locks_ready_for_review() -> None: text = (ROOT / "DESIGN.md").read_text(encoding="utf-8") - section = text.split("### 19.6 Pull request done", 1)[1].split("## 20.", 1)[0] - assert "Pull request done" in text + section = text.split("### 19.6 Ready for review", 1)[1].split("## 20.", 1)[0] + prose = _ws(text) + section_prose = _ws(section) + assert "Ready for review" in text assert NOT_DONE in text assert NOT_DONE in section assert "Opening a draft is not done" in text - assert "only checks that a task is in `pushing` or `pr-review`" in section - assert "When spine and pr-review are attached" in section - assert "Without those skills, the target repository" in section - assert "does not sit those PR reviews" in section - assert "Empty, partial, timeout, or unavailable" in section - assert "is not zero findings" in section - assert "agent allow --action pr-ready" in section - assert "`skipped` and `cancelled` are not green" in section - assert "do not substitute another vendor" in section - assert "Vendors are `grok`, then `codex`" in section - assert "four lane verdicts" in section - assert "those four `approved` verdicts on this head" in section + assert "Leaving draft is Ready for review, not completion" in prose + assert "only checks that a task is in `pushing` or `pr-review`" in section_prose + assert "When spine and pr-review are attached" in section_prose + assert "Without those skills, the target repository" in section_prose + assert "does not sit those PR reviews" in section_prose + assert "Empty, partial, timeout, or unavailable" in section_prose + assert "is not zero findings" in section_prose + assert "agent allow --action pr-ready" in section_prose + assert "`skipped` and `cancelled` are not green" in section_prose + assert "do not substitute another vendor" in section_prose + assert "Vendors are `grok`, then `codex`" in section_prose + assert "four lane verdicts" in section_prose + assert "those four `approved` verdicts on this head" in section_prose + assert "Ready for review is still not merge and not completion" in section_prose + assert "ledger `task-done` is not pull-request completion" in section_prose -def test_pr_review_and_spine_point_at_pr_done() -> None: +def test_pr_review_and_spine_point_at_ready_for_review() -> None: pr_review = (PACKAGED / "pr-review" / "SKILL.md").read_text(encoding="utf-8") spine = (PACKAGED / "spine" / "SKILL.md").read_text(encoding="utf-8") review_loop = (PACKAGED / "review-loop" / "SKILL.md").read_text(encoding="utf-8") + pr_review_prose = _ws(pr_review) + spine_prose = _ws(spine) assert NOT_DONE in pr_review - assert "session that authored the diff does not" in pr_review + assert "session that authored the diff does not" in pr_review_prose assert "agent allow --action pr-ready" in pr_review - assert "Do not\n substitute another vendor" in pr_review or "Do not substitute another vendor" in pr_review - assert "unavailable output is not zero findings" in pr_review - assert "four lane verdicts on this head are approved" in pr_review - assert "those four `approved` verdicts on this head" in pr_review - assert NOT_DONE in spine - assert "not pull-request done" in spine + assert "Do not substitute another vendor" in pr_review_prose + assert "unavailable output is not zero findings" in pr_review_prose + assert "four lane verdicts on this head are approved" in pr_review_prose + assert "those four `approved` verdicts on this head" in pr_review_prose + assert "not merge and not pull-request completion" in pr_review_prose + assert NOT_DONE in spine_prose + assert "not Ready for review" in spine_prose + assert "not pull-request completion" in spine_prose assert "not the pull-request review" in review_loop assert "Inner implement/review rounds (`review-loop`) are not the PR reviews" in ( ROOT / "CONTRIBUTING.md" @@ -74,8 +91,16 @@ def test_agents_md_and_readme_point_at_contributing() -> None: readme = (ROOT / "README.md").read_text(encoding="utf-8") stub = (ROOT / "skills" / "session-store" / "SKILL.md").read_text(encoding="utf-8") packaged = (PACKAGED / "session-store" / "SKILL.md").read_text(encoding="utf-8") + lifecycle = (ROOT / "docs" / "pull-request-lifecycle.md").read_text(encoding="utf-8") + agents_prose = _ws(agents) + lifecycle_prose = _ws(lifecycle) assert NOT_DONE in agents assert "CONTRIBUTING.md" in agents + assert "pull-request-lifecycle.md" in agents + assert "Ready for review is still not merge and not completion" in agents_prose + assert "Never finished, done, or completed" in lifecycle_prose + assert "Still not merged and still not completed" in lifecycle_prose + assert "Leaving draft is **Ready for review**, not pull-request completion" in lifecycle_prose assert "A draft plus local tests is not a finished pull request" in readme assert NOT_DONE in stub assert "for this repository" in stub