Skip to content

docs(pullrequest skill): GraphQL CI gate + local-main update (off the REST rate limit)#399

Merged
rbuergi merged 1 commit into
mainfrom
chore/pullrequest-skill-ci-gate
Jul 10, 2026
Merged

docs(pullrequest skill): GraphQL CI gate + local-main update (off the REST rate limit)#399
rbuergi merged 1 commit into
mainfrom
chore/pullrequest-skill-ci-gate

Conversation

@rbuergi

@rbuergi rbuergi commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Why

While merging #396 the /pullrequest gate's gh run watch polled the REST API hard enough to trip a 403 API rate limit exceeded — its exit=1 looked like CI-red but wasn't, nearly causing a false abort. This reworks the skill's CI gate to avoid the REST budget entirely and to close a second long-standing gap.

What changed (docs-only: .claude/skills/pullrequest/SKILL.md)

  • GraphQL check-suite poll replaces gh run watch / gh pr checks --watch. REST is a shared 5000/hr user-token budget drained by ~3s polling across concurrent sessions; GraphQL has its own budget at ~1 point/query. Gating on the check SUITE (which is COMPLETED only when every shard job finishes) also fixes the late-shard race that merged-before-tests and turned main red after fix(build): track ToolCallVisibility.cs — main CI was red (CS0103) #138.
  • Run the gate in the background → the harness delivers one "CI finished" notification (the closest a local CLI session gets to a push event — GitHub can't webhook a CLI directly).
  • Step 6: update local main with git fetch origin main:main (fast-forward the ref, no checkout — the working tree is usually a dirty feature branch).
  • 403 caveat (never merge/abort off a rate-limit; gh api /rate_limit is exempt), the GitHub-App-token durable fix, and the deferred webhook→WS push design for true zero-poll delivery.

Nothing on main is lost — the What's New (step 0.5) sections are preserved; this only rewrites the CI-gate guidance.

What's New

Skipped — pure-internal tooling change to the /pullrequest skill, no user-visible platform effect (per step 0.5's own skip rule).

🤖 Generated with Claude Code

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

Updates the /pullrequest skill documentation to change how contributors gate merges on CI completion, focusing on avoiding REST API rate-limit failures and eliminating the “late shard” race.

Changes:

  • Replaces gh pr checks --watch / gh run watch guidance with a GraphQL-based poll of the workflow check suite until status == COMPLETED, then gates merge on conclusion == SUCCESS.
  • Adds guidance to run the CI gate loop in the background (for a single completion notification) and to fast-forward the local main ref post-merge without checking it out.
  • Expands the rationale section explaining REST rate-limit failure modes and the check visibility race.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +78 to +82
Q='query($o:String!,$r:String!,$p:Int!){repository(owner:$o,name:$r){pullRequest(number:$p){commits(last:1){nodes{commit{checkSuites(first:20){nodes{status conclusion workflowRun{workflow{name}}}}}}}}}}'
suite(){ gh api graphql -f query="$Q" -f o=Systemorph -f r=MeshWeaver -F p=$PR \
--jq ".data.repository.pullRequest.commits.nodes[0].commit.checkSuites.nodes[]|select(.workflowRun.workflow.name==\"MeshWeaver Build and Test\")|.$1"; }
until [ "$(suite status)" = "COMPLETED" ]; do sleep 45; done # cheap: ~1 GraphQL point per poll
c=$(suite conclusion); echo "PR $PR CI: $c"; [ "$c" = "SUCCESS" ] # exit 0 iff green → the merge signal
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results (shard 1)

884 tests  ±0   700 ✅ ±0   2m 41s ⏱️ +5s
  8 suites ±0   184 💤 ±0 
  8 files   ±0     0 ❌ ±0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results (shard 4)

  9 files  ±  0    9 suites  ±0   2m 33s ⏱️ - 1m 1s
919 tests  - 158  919 ✅  - 158  0 💤 ±0  0 ❌ ±0 
928 runs   - 191  928 ✅  - 191  0 💤 ±0  0 ❌ ±0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

This pull request removes 187 and adds 29 tests. Note that renamed tests count towards both.
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/Analysis", expectedPrefix: "FutuRe/Analysis", expectedRemainder: null)
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/EuropeRe", expectedPrefix: "FutuRe/EuropeRe", expectedRemainder: null)
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/EuropeRe/LineOfBusiness", expectedPrefix: "FutuRe/EuropeRe/LineOfBusiness", expectedRemainder: null)
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/EuropeRe/Overview", expectedPrefix: "FutuRe/EuropeRe", expectedRemainder: "Overview")
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuRe_ShouldNotReturnNull
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_UnknownDeepPath_ShouldReturnNull
MeshWeaver.Persistence.Test.ConcurrentRequestsTest ‑ ConcurrentRequests_MultipleNodeTypes_AllLoadWithoutHanging
MeshWeaver.Persistence.Test.DocumentationCodeBlockCompilationTest ‑ ExecutedCsharpBlocks_MustCompile(embeddedResourceName: "MeshWeaver.Documentation.Data.AI.AgenticAI.md")
MeshWeaver.Persistence.Test.DocumentationCodeBlockCompilationTest ‑ ExecutedCsharpBlocks_MustCompile(embeddedResourceName: "MeshWeaver.Documentation.Data.AI.ChatCommands.md")
MeshWeaver.Persistence.Test.DocumentationCodeBlockCompilationTest ‑ ExecutedCsharpBlocks_MustCompile(embeddedResourceName: "MeshWeaver.Documentation.Data.AI.ExecuteScript.md")
…
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ Coverage_DoesNotRegress
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryCourseCell_Compiles(relativePath: "CSharpBasics/Module02/Exercise/AverageStock/Soluti"···, cellId: "averagestocksolution")
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryGreenCell_ExecutesInKernel(relativePath: "CSharpBasics/Module01/Theory/01-HelloControls.md", cellId: "hellotext")
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryGreenCell_ExecutesInKernel(relativePath: "CSharpBasics/Module01/Theory/01-HelloControls.md", cellId: "modulelist")
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryGreenCell_ExecutesInKernel(relativePath: "CSharpBasics/Module01/Theory/01-HelloControls.md", cellId: "stackoflabels")
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryGreenCell_ExecutesInKernel(relativePath: "CSharpBasics/Module02/Exercise/AverageStock/Soluti"···, cellId: "averagestocksolution")
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryGreenCell_ExecutesInKernel(relativePath: "CSharpBasics/Module02/Theory/01-WorkingWithData.md", cellId: "inventoryvalue")
MeshWeaver.Persistence.Test.CourseCellExecutionTest ‑ EveryGreenCell_ExecutesInKernel(relativePath: "CSharpBasics/Module02/Theory/01-WorkingWithData.md", cellId: "productgrid")
MeshWeaver.Persistence.Test.DataContextIntegrationTest ‑ GraphHub_InitializesWithConfiguration
MeshWeaver.Persistence.Test.DataContextIntegrationTest ‑ MeshNode_ChildrenAvailable_ViaPersistence
…

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results (shard 5)

827 tests  +12   826 ✅ +12   6m 4s ⏱️ -12s
 10 suites ± 0     1 💤 ± 0 
 10 files   ± 0     0 ❌ ± 0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results (shard 3)

1 042 tests  ±0   1 042 ✅ ±0   3m 23s ⏱️ +15s
   10 suites ±0       0 💤 ±0 
   10 files   ±0       0 ❌ ±0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results (shard 2)

731 tests  ±0   729 ✅ ±0   1m 59s ⏱️ -1s
 10 suites ±0     2 💤 ±0 
 10 files   ±0     0 ❌ ±0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results (shard 0)

983 tests  ±0   979 ✅ ±0   6m 6s ⏱️ -10s
 11 suites ±0     4 💤 ±0 
 11 files   ±0     0 ❌ ±0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

Test Results

   58 files  ±  0     58 suites  ±0   22m 48s ⏱️ - 1m 4s
5 386 tests  - 146  5 195 ✅  - 146  191 💤 ±0  0 ❌ ±0 
5 395 runs   - 179  5 204 ✅  - 179  191 💤 ±0  0 ❌ ±0 

Results for commit ced74dc. ± Comparison against base commit e6a191a.

This pull request removes 187 and adds 41 tests. Note that renamed tests count towards both.
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/Analysis", expectedPrefix: "FutuRe/Analysis", expectedRemainder: null)
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/EuropeRe", expectedPrefix: "FutuRe/EuropeRe", expectedRemainder: null)
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/EuropeRe/LineOfBusiness", expectedPrefix: "FutuRe/EuropeRe/LineOfBusiness", expectedRemainder: null)
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuReSubPaths_ShouldResolve(path: "FutuRe/EuropeRe/Overview", expectedPrefix: "FutuRe/EuropeRe", expectedRemainder: "Overview")
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_FutuRe_ShouldNotReturnNull
MeshWeaver.Persistence.Test.ApplicationPageResolutionTest ‑ ResolvePathAsync_UnknownDeepPath_ShouldReturnNull
MeshWeaver.Persistence.Test.ConcurrentRequestsTest ‑ ConcurrentRequests_MultipleNodeTypes_AllLoadWithoutHanging
MeshWeaver.Persistence.Test.DocumentationCodeBlockCompilationTest ‑ ExecutedCsharpBlocks_MustCompile(embeddedResourceName: "MeshWeaver.Documentation.Data.AI.AgenticAI.md")
MeshWeaver.Persistence.Test.DocumentationCodeBlockCompilationTest ‑ ExecutedCsharpBlocks_MustCompile(embeddedResourceName: "MeshWeaver.Documentation.Data.AI.ChatCommands.md")
MeshWeaver.Persistence.Test.DocumentationCodeBlockCompilationTest ‑ ExecutedCsharpBlocks_MustCompile(embeddedResourceName: "MeshWeaver.Documentation.Data.AI.ExecuteScript.md")
…
MeshWeaver.Hosting.Monolith.Test.CessionLayoutAreaTest ‑ MotorXL_LayoutArea_ReturnsContent
MeshWeaver.Hosting.Monolith.Test.CessionLayoutAreaTest ‑ MotorXL_Overview_ShouldRender
MeshWeaver.Hosting.Monolith.Test.MeshNodeCompilationIntegrationTest ‑ CompileFailsWhenSourceCodeIsInvalid
MeshWeaver.Hosting.Monolith.Test.MeshNodeCompilationIntegrationTest ‑ CompileWithMultipleSourceLocationsPullsInExternalCode
MeshWeaver.Hosting.Monolith.Test.MeshNodeCompilationIntegrationTest ‑ CompilesSimpleNodeTypeWithDefaultSources
MeshWeaver.Hosting.Monolith.Test.MeshNodeStreamCacheConcurrencyTest ‑ GetQuery_ConcurrentDifferentIds_AllResolveIndependently
MeshWeaver.Hosting.Monolith.Test.MeshNodeStreamCacheConcurrencyTest ‑ GetQuery_ManyConcurrentCallersSameId_AllSeeSameSnapshot
MeshWeaver.Hosting.Monolith.Test.MeshNodeStreamCacheConcurrencyTest ‑ GetQuery_ReturnsLiveUpdatesAfterRuntimeCreate
MeshWeaver.Hosting.Monolith.Test.OverviewHeaderRenderTest ‑ CreateNode_StampsCreatedAndLastModified
MeshWeaver.Hosting.Monolith.Test.OverviewHeaderRenderTest ‑ Delete_Renders_ForExistingNode
…

♻️ This comment has been updated with latest results.

…ate, off the REST rate limit

Replaces the `gh run watch` / `gh pr checks --watch` guidance (polls REST every ~3s → drains the
shared 5000/hr user-token budget → 403s that masquerade as CI-red) with a GraphQL check-suite poll
on its own budget. Gating on the check SUITE (COMPLETED only when every shard finishes) also closes
the late-shard race that turned main red after #138. Adds:
- step 6: fast-forward local main with `git fetch origin main:main` (no checkout — the tree is
  usually a dirty feature branch)
- run the step-3 gate in the background so the harness delivers ONE "CI finished" notification (the
  completion event; a local CLI session can't be webhooked directly)
- a 403 caveat (never merge off a rate-limit; /rate_limit is exempt), the GitHub-App-token durable
  fix, and the deferred webhook->WS push design

The poll collapses to the LATEST check-suite (`| last`) so a workflow re-run's extra suite can't make
`$(suite …)` multi-line and stall the gate (Copilot review).

What's New: skipped — pure-internal tooling change to the /pullrequest skill, no user-visible effect.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rbuergi rbuergi force-pushed the chore/pullrequest-skill-ci-gate branch from adbdcf7 to ced74dc Compare July 10, 2026 07:55
@rbuergi rbuergi merged commit 140c615 into main Jul 10, 2026
15 checks passed
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