Skip to content

docs(pull-request-workflow): return a PR to draft when work resumes - #227

Merged
CybotTM merged 2 commits into
mainfrom
docs/draft-on-resume
Aug 25, 2026
Merged

docs(pull-request-workflow): return a PR to draft when work resumes#227
CybotTM merged 2 commits into
mainfrom
docs/draft-on-resume

Conversation

@CybotTM

@CybotTM CybotTM commented Aug 25, 2026

Copy link
Copy Markdown
Member

Problem

The PR Lifecycle section already covers opening every PR with --draft and the mechanics of the Draft → Ready edge, but it treats draft as a birth condition. There is no rule for the case that actually recurs: work resuming on a PR that already reads "ready for review" — a rebase onto a moved base, a round of review fixes, another commit of any kind. gh pr ready --undo appears only as an unexplained line in the command list, which reads as a remedy for a PR opened non-draft by mistake rather than as a step in the normal loop.

Why it matters

"Ready for review" is a standing request for a maintainer's time against a specific head, and mid-work heads do not deserve it. Between the first fix commit and the last, such a PR advertises for review a state you already know is incomplete — sometimes one you know is broken, when the work is a response to a reviewer's finding. Reviewers who look during that window spend attention on a diff that is about to change, and a green CI run on an intermediate head reads as an endorsement of work that is not finished.

Change

Adds the return edge to the state diagram, the rule with its reason, the tell that it was skipped (recent commits labelled as review fixes while draft: false held throughout), and the gh/glab commands. The command-list entry now says when it applies.

Observed

phpDocumentor/guides#1344, 2026-08-24. The PR sat at "ready for review" through a rebase and two rounds of fixes answering a reviewer's finding, one of which the finding had shown to be broken. It took two corrections before the state was set right, and the rule that should have covered it only spoke about gh pr create.

Assisted by claude-code:claude-opus-5 — Session

The lifecycle section covered opening every PR with --draft and the mechanics
of the Draft to Ready edge, but treated draft as a birth condition. Resuming
work on a PR that already reads "ready for review" - a rebase, a round of
review fixes, another commit - has no rule, and `gh pr ready --undo` appeared
only as an unexplained line in the command list.

Say what the state means to everyone else: "ready for review" is a standing
request for a maintainer's time against a specific head, and mid-work heads
do not deserve it. Between the first fix commit and the last, such a PR
advertises a state known to be incomplete, sometimes one known to be broken
when the work answers a reviewer's finding, and a green run on an intermediate
head reads as an endorsement of unfinished work.

Adds the return edge to the state diagram, the rule with its reason, the tell
that it was skipped, and the gh/glab commands.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_014H1xwaAmrQRWUA3vx8bJcD
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions github-actions Bot added documentation Improvements or additions to documentation skill labels Aug 25, 2026
The check offered with the new rule did not run: `gh pr view --json` spells the
field `isDraft`, while `draft` belongs to the REST payload and `gh api`. Copying
it produced `Unknown JSON field: "draft"`.

Corrects the field, names the discrepancy so the next reader does not trip on it
in the other direction, and drops a sentence that argued for the practice instead
of describing it.

Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_014H1xwaAmrQRWUA3vx8bJcD
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
@sonarqubecloud

Copy link
Copy Markdown

@CybotTM
CybotTM marked this pull request as ready for review August 25, 2026 06:13
Copilot AI lite review requested due to automatic review settings August 25, 2026 06:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@CybotTM

CybotTM commented Aug 25, 2026

Copy link
Copy Markdown
Member Author

Copilot answered the ruleset's review request on 601524f2 with "unable to review … reached their quota limit" and no comments, so no bot review exists for this head. Reviewed it myself instead.

The review was not a formality — it caught the check command shipped with the new rule. gh pr view --json spells the field isDraft; draft belongs to the REST payload and gh api. Copied as written it returned Unknown JSON field: "draft". Fixed in 601524f, which also names the discrepancy so the next reader does not trip on it from the other side, and drops a sentence that argued for the practice rather than describing it.

Verified on the head: pre-commit run over the changed file passes, scripts/verify-harness.sh reports Level 3 COMPLETE with 0 errors, scripts/spec-cleanup-guard.sh reports clean, and the corrected command was run against this PR before being committed. Branch is not behind main. No review threads.

Self-review: 601524f

Assisted by claude-code:claude-opus-5 — Session

@CybotTM
CybotTM merged commit 8f879c2 into main Aug 25, 2026
23 of 24 checks passed
@CybotTM
CybotTM deleted the docs/draft-on-resume branch August 25, 2026 06:16
CybotTM added a commit that referenced this pull request Aug 26, 2026
Draft sat near the top of the NEXT ladder and always answered ready,
masking conflicts, red checks and open threads on a deliberately parked
PR; ready itself was missing from ACTIONABLE, so the watch could
neither return on it nor hold through it — --ignore-action refused the
name and the loop heartbeated waiting: draft into the timeout. The
back-to-draft-on-resume convention (#227) and the watcher were mutually
exclusive.

Draft now ranks below the real-work branches and above the review and
merge advice, mirroring the queue-entry placement: while checks run it
reports wait (the watch holds), a red check or an open thread surfaces
normally, and only a settled draft answers ready — which is ACTIONABLE
now, so the watch returns on it and --ignore-action can hold through
it. investigate joins ACTIONABLE too: it is the terminal
check-branch-protection-manually verdict on settled checks, and a watch
that heartbeats on it idles into the timeout on a state waiting cannot
change (observed 2026-08-26 on #226, where classic-protection CodeQL
contexts kept mergeState BLOCKED for over an hour of waiting lines).

tests/test_pr_status_draft_watch.sh pins the ladder against a stubbed
gh and the action vocabulary against every action literal the script
emits (each must be ACTIONABLE or a waiting heartbeat); it was watched
failing 8 ways on the pre-fix script.

Closes #228.

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_01C7S9rbgu5giqCwnzwafrHA
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
CybotTM added a commit that referenced this pull request Aug 26, 2026
Draft sat near the top of the NEXT ladder and always answered ready,
masking conflicts, red checks and open threads on a deliberately parked
PR; ready itself was missing from ACTIONABLE, so the watch could
neither return on it nor hold through it — --ignore-action refused the
name and the loop heartbeated waiting: draft into the timeout. The
back-to-draft-on-resume convention (#227) and the watcher were mutually
exclusive.

Draft now ranks below the real-work branches and above the review and
merge advice, mirroring the queue-entry placement: a red check or an
open thread surfaces normally, checks actually running report wait (the
watch holds), and everything else answers ready — which is ACTIONABLE
now, so the watch returns on it and --ignore-action can hold through
it. checks_settled is deliberately not the gate for ready: it demands
at least one registered context and zero undispatched required ones,
which a draft often cannot satisfy (workflows that skip drafts or
trigger on ready_for_review, fork runs awaiting approval) — the review
round caught that gating on it left ready unreachable there, an endless
wait of the very shape this commit removes.

investigate joins ACTIONABLE too: it is the terminal
check-branch-protection-manually verdict on settled checks, and a watch
that heartbeats on it idles into the timeout on a state waiting cannot
change (observed 2026-08-26 on #226, where classic-protection CodeQL
contexts kept mergeState BLOCKED for over an hour of waiting lines).

tests/test_pr_status_draft_watch.sh pins the ladder against a stubbed
gh (hold on running checks; return on a thread, a red check, ready;
ready reachable with zero registered checks and with a never-dispatched
required context; plain status names a red check instead of ready) and
pins the action vocabulary against every action literal the script
emits. Watched failing against the respective pre-fix states: 9
assertions on the original script, 5 more on the version before the
checks_settled correction.

Closes #228.

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_01C7S9rbgu5giqCwnzwafrHA
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
CybotTM added a commit that referenced this pull request Aug 26, 2026
Draft sat near the top of the NEXT ladder and always answered ready,
masking conflicts, red checks and open threads on a deliberately parked
PR; ready itself was missing from ACTIONABLE, so the watch could
neither return on it nor hold through it — --ignore-action refused the
name and the loop heartbeated waiting: draft into the timeout. The
back-to-draft-on-resume convention (#227) and the watcher were mutually
exclusive.

Draft now ranks below the real-work branches and above the review and
merge advice, mirroring the queue-entry placement: a red check or an
open thread surfaces normally, checks actually running report wait (the
watch holds), and everything else answers ready — which is ACTIONABLE
now, so the watch returns on it and --ignore-action can hold through
it. checks_settled is deliberately not the gate for ready: it demands
at least one registered context and zero undispatched required ones,
which a draft often cannot satisfy (workflows that skip drafts or
trigger on ready_for_review, fork runs awaiting approval) — the review
round caught that gating on it left ready unreachable there, an endless
wait of the very shape this commit removes.

investigate joins ACTIONABLE too: it is the terminal
check-branch-protection-manually verdict on settled checks, and a watch
that heartbeats on it idles into the timeout on a state waiting cannot
change (observed 2026-08-26 on #226, where classic-protection CodeQL
contexts kept mergeState BLOCKED for over an hour of waiting lines).

tests/test_pr_status_draft_watch.sh pins the ladder against a stubbed
gh (hold on running checks; return on a thread, a red check, ready;
ready reachable with zero registered checks and with a never-dispatched
required context; plain status names a red check instead of ready) and
pins the action vocabulary against every action literal the script
emits. Watched failing against the respective pre-fix states: 9
assertions on the original script, 5 more on the version before the
checks_settled correction.

Closes #228.

Assisted-by: claude-code:claude-fable-5
Agent-Session: https://claude.ai/code/session_01C7S9rbgu5giqCwnzwafrHA
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation skill

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants