fix: keepalive no-checklist handling + state-fingerprint 401/403 fallback - #2010
Conversation
Phase 6 sync-PR review surfaced two related bugs in keepalive_orchestrator_gate_runner.js (Copilot review on stranske/* sync PRs): - routeDraftToHuman() emitted "0 unchecked checklist item(s)" when the PR had no checkboxes at all, suggesting the user just needed to check boxes that didn't exist. - The branching at line 404 fell through to the same "needs human" path for both genuine missing-acceptance-items cases and PRs that legitimately have no checklist at all. This change distinguishes the no-checklist case end-to-end: - Adds a noChecklist flag in the caller (computed once where checkboxCounts is built). - Threads noChecklist through routeDraftToHuman so the comment body and summary line accurately describe "no acceptance checklist found" vs "N unchecked items". - Adds a distinct reason key 'pr-draft-no-checklist' so weekly metrics can distinguish the two cases. Lockstep edit: canonical .github/scripts/ + templates/consumer-repo/ both updated identically. node --check passes on both. Out of scope here: the perceived "missing closing brace" Copilot flagged was already addressed by PR #1985 / #1986 on 2026-04-30 — the GraphQL mutation block in markDraftReadyForReview() has the correct three closing braces. The isConcreteAgentLabel() concern about agent:rate-limited / agent:retry routing as concrete agent labels is separate and needs broader review of the keepalive loop's label-routing semantics; deferred. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No description provided. |
Workflow source neededPR #2010 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
Automated Status SummaryHead SHA: 678226a
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
|
No description provided. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the keepalive orchestrator gate runner’s draft-PR disposition logic so draft PRs with no acceptance checklist are handled distinctly from draft PRs that have unchecked checklist items, improving the accuracy of human-routing messaging and emitted reason keys (used for metrics).
Changes:
- Introduces a
noChecklistflag (checked === 0 && unchecked === 0) and threads it throughrouteDraftToHuman(). - Updates draft-disposition summary + durable comment text to say “no acceptance checklist found” vs “N unchecked items”.
- Emits a new reason key
pr-draft-no-checklist(instead of lumping intopr-draft-needs-human) when appropriate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/scripts/keepalive_orchestrator_gate_runner.js |
Adds noChecklist detection, updates human-routing messaging, and emits the new reason key for metrics. |
templates/consumer-repo/.github/scripts/keepalive_orchestrator_gate_runner.js |
Mirrors the canonical script changes to keep consumer template behavior in lockstep. |
| const draftReason = noChecklist | ||
| ? 'Keepalive found this PR still in draft with no acceptance checklist in the PR body. Draft PRs must not occupy automation capacity silently.' | ||
| : `Keepalive found this PR still in draft with ${checkboxCounts.unchecked} unchecked checklist item(s). Draft PRs must not occupy automation capacity silently.`; | ||
|
|
||
| const nextAction = noChecklist | ||
| ? 'Next human action: add an acceptance checklist to the PR body and mark it ready for review, or close/supersede the PR.' | ||
| : 'Next human action: finish the unchecked acceptance items and mark the PR ready for review, or close/supersede the PR.'; | ||
|
|
| : `Keepalive found this PR still in draft with ${checkboxCounts.unchecked} unchecked checklist item(s). Draft PRs must not occupy automation capacity silently.`; | ||
|
|
||
| const nextAction = noChecklist | ||
| ? 'Next human action: add an acceptance checklist to the PR body and mark it ready for review, or close/supersede the PR.' | ||
| : 'Next human action: finish the unchecked acceptance items and mark the PR ready for review, or close/supersede the PR.'; |
| const noChecklist = checkboxCounts.checked === 0 && checkboxCounts.unchecked === 0; | ||
| const allChecklistWorkComplete = checkboxCounts.checked > 0 && checkboxCounts.unchecked === 0; | ||
| if (allChecklistWorkComplete) { |
…TECTION_TOKEN in health-44 Wave 1's state-fingerprint helper (#1998 + #2002 wireup) broke Health 44 enforce because the workflow's GITHUB_TOKEN can't access the actions/variables endpoint even with `actions: write` set — that endpoint requires a token with Variables permission (PAT, GitHub App, or fine-grained PAT). Resulting failure observed on every PR running Health 44 enforce since #2002 merged (incl. PR #2006, PR #2007 today): GET /repos/stranske/Workflows/actions/variables/STATE_FINGERPRINT_HEALTH_44_GATE_BRANCH_PROTECTION_* failed: 403 "Resource not accessible by integration" Two-part fix: 1) `scripts/state_fingerprint.py` — `RepoVariableStorage` now treats 401/403 from the variables API as "storage unavailable" rather than fatal. Read returns None (no prior fingerprint), write skips silently, and a warning goes to stderr so the operator sees the misconfiguration in workflow logs. The existing 404 (no prior) path is unchanged. Effect: any workflow that adopts `--storage repo-variable` but doesn't have the right token degrades gracefully (skips the optimization, runs anyway) instead of failing outright. Future Wave 1+ workflows using repo-variable storage benefit from this. 2) `.github/workflows/health-44-gate-branch-protection.yml` — uses `BRANCH_PROTECTION_TOKEN` (already used downstream by `enforce`) when present, falling back to `GITHUB_TOKEN`. Now the fingerprint optimization actually works when the secret is configured. Existing 6 tests in tests/scripts/test_state_fingerprint.py still pass. py_compile clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Two related sync-review-fallout fixes from reviewing the consumer-repo sync PRs after Wave 1:
1. Keepalive orchestrator no-checklist handling (commit f8606b4)
Copilot review on multiple stranske/* sync PRs flagged that `routeDraftToHuman()` emits "this PR still in draft with 0 unchecked checklist item(s)" when a PR has no checkboxes at all. The branching at line 404 also fell through to the same "needs human" path for both genuine missing-acceptance-items cases and PRs that legitimately have no checklist.
End-to-end fix: a `noChecklist` flag (`checked === 0 && unchecked === 0`) computed at the call site and threaded through `routeDraftToHuman`. Comment body, summary line, and reason key all branch on it. New reason key `pr-draft-no-checklist` (alongside existing `pr-draft-needs-human`) for weekly metrics. Lockstep canonical + template.
The "missing closing brace" Copilot also flagged was already fixed by PR #1985 / #1986 on 2026-04-30 — the GraphQL mutation block has correct three closing braces. `isConcreteAgentLabel()` rate-limited/retry concern deferred to a separate review.
2. State-fingerprint 401/403 graceful fallback + Health 44 token fix (commit 38006bb)
Wave 1's state-fingerprint helper (#1998 + #2002 wireup) broke Health 44 `enforce` because the workflow's `GITHUB_TOKEN` can't access the `actions/variables` endpoint even with `actions: write` set — that endpoint requires a token with Variables permission. Result: every PR running Health 44 enforce has been failing since #2002 merged (incl. PR #2006 / PR #2007 today).
Two-part fix:
Test plan
🤖 Generated with Claude Code