Skip to content

Fix keepalive draft GraphQL mutation - #1986

Merged
stranske merged 1 commit into
mainfrom
sync-review-1725-gate-runner-fix
May 5, 2026
Merged

Fix keepalive draft GraphQL mutation#1986
stranske merged 1 commit into
mainfrom
sync-review-1725-gate-runner-fix

Conversation

@stranske

Copy link
Copy Markdown
Owner

Summary

  • Close the keepalive draft ready-for-review GraphQL mutation correctly.
  • Split missing PR node id from unavailable GraphQL client reporting.
  • Mirror the managed consumer template script and cover both review cases in tests.

Validation

  • node --test .github/scripts/__tests__/keepalive-orchestrator-gate-runner.test.js
  • python scripts/validate_template_sync.py

Related consumer sync PR: stranske/Portable-Alpha-Extension-Model#1725

Copilot AI review requested due to automatic review settings April 30, 2026 09:20
@stranske-keepalive

Copy link
Copy Markdown
Contributor

Automated Status Summary

Head SHA: 49efae1
Latest Runs: ⏳ pending — Gate
Required contexts: Gate / gate, Health 45 Agents Guard / guard
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job Result Logs
(no jobs reported) ⏳ pending

Coverage Overview

  • Coverage history entries: 1

Coverage Trend

Metric Value
Current 93.12%
Baseline 85.00%
Delta +8.12%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
src/cli_parser.py 81.8% 4
src/percentile_calculator.py 95.0% 1
src/aggregator.py 95.0% 2
src/__init__.py 100.0% 0
src/ndjson_parser.py 100.0% 0

Updated automatically; will refresh on subsequent CI/Docker completions.


Keepalive checklist

Scope

No scope information available

Tasks

  • No tasks defined

Acceptance criteria

  • No acceptance criteria defined

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes the keepalive gate’s draft-PR “mark ready for review” GraphQL mutation by making the mutation string well-formed, improving failure reporting when GraphQL is unavailable vs when the PR node id is missing, and mirroring the change into the consumer template while extending test coverage for both cases.

Changes:

  • Introduce a named, shared MARK_PULL_REQUEST_READY_FOR_REVIEW_MUTATION constant and use it for the GraphQL call (ensuring the mutation is properly closed).
  • Split the “missing PR node id” condition from “GraphQL client unavailable” and report them distinctly in the step summary.
  • Extend unit tests to validate the mutation text shape/closure and the new unavailable-GraphQL behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
templates/consumer-repo/.github/scripts/keepalive_orchestrator_gate_runner.js Mirrors the fixed/closed GraphQL mutation and improved error-path reporting for consumer repos.
.github/scripts/keepalive_orchestrator_gate_runner.js Applies the same mutation fix and clearer branching between missing node id vs missing GraphQL client.
.github/scripts/__tests__/keepalive-orchestrator-gate-runner.test.js Adds coverage for mutation correctness and for “GraphQL client unavailable” vs “missing node id” reporting.

stranske added a commit that referenced this pull request May 5, 2026
…back (#2010)

* fix(keepalive): handle no-checklist draft PRs with accurate disposition

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>

* fix(state-fingerprint): graceful fallback on 401/403 + use BRANCH_PROTECTION_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>

* chore: apply Black formatting to state_fingerprint.py 401/403 fallback

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@stranske
stranske merged commit 84bbb17 into main May 5, 2026
45 checks passed
@stranske
stranske deleted the sync-review-1725-gate-runner-fix branch May 5, 2026 06:05
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.

2 participants