Skip to content

fix(publication): stop repository pool on first throttle - #1145

Draft
brokemac79 wants to merge 8 commits into
mainfrom
codex/csw-123-repository-pool-circuit
Draft

fix(publication): stop repository pool on first throttle#1145
brokemac79 wants to merge 8 commits into
mainfrom
codex/csw-123-repository-pool-circuit

Conversation

@brokemac79

@brokemac79 brokemac79 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a default-disabled interlock that stops repository_actions exact-review publication egress on the first GitHub throttle observed for that credential pool.

The interlock is a pool-sharded SQLite Durable Object. Each external operation acquires and starts a short fenced permit. A 403/429 advances the shared epoch synchronously, so already-claimed siblings that have not started are rejected before wire and durably requeued as attempted=false. Calls already on wire may finish and are counted. Recovery preserves authoritative reset headers, admits exactly one half-open probe, and ramps through fixed 1 -> 2 -> 4 -> 8 cohorts.

Enforcement remains off by default. This PR neither activates the coordinator nor changes scheduled admission (300/hour, burst 30), hot-review cadence, queue contents, credentials, gates, or production state.

Problem

Phase 0 telemetry established that publication work can claim up to eight sibling batches before the first batch observes a shared GitHub throttle. Without a pool-wide fence, siblings learn about the unavailable credential independently, creating avoidable requests and retry amplification.

The constrained resource is the credential pool, not an item or target repository. The required invariant is: after the first authoritative throttle observation, no not-yet-started operation using that pool may reach GitHub; already-on-wire work remains bounded and completes under existing mutation and idempotency rules.

This is deliberately not a fleet-wide quota cure. Phase 0 measured public_read_fallback and target_app as separate cohorts. This PR coordinates only the corrected repository_actions publication boundary and never infers a pool from generic rate-limit text.

Implementation

  • Add GithubEgressPoolCoordinator, a SQLite-backed Durable Object keyed by a server-derived private credential-pool identity. It is not keyed by item/repository and is not one global singleton.
  • Require one declared-budget permit per external gh invocation, with separate acquire/start/finish/throttle transitions and epoch fencing.
  • On the first valid 403/429 observation, atomically open a new epoch. Acquired-but-unstarted siblings receive a bounded pre-wire rejection; already-started siblings may finish and contribute to permits_in_flight_at_open and already_on_wire_completions.
  • Preserve credible Retry-After and X-RateLimit-Reset authority. Missing or non-credible deadlines use one persisted bounded exponential backoff with deterministic jitter rather than independent 60-second reopen loops.
  • Admit exactly one fenced probe at/after reset. Probe failure reopens the shared circuit; success starts fixed recovery cohorts of 1, 2, 4, and 8. Cohorts cannot refill after partial completion, cannot advance with unresolved work, and reopen on failure or expiry.
  • Route coordinator deferrals through the durable publication completion contract as attempted=false, without consuming publication failure, retry, DLQ, or mutation budgets. Genuinely attempted failures retain existing accounting.
  • Fence the actual repository-token publication surfaces: runtime gh api, lifecycle/comment routers, artifact download, repository-token workflow dispatch, terminal confirmation, and repository-token reads in apply-decisions. Target-App mutations remain outside this coordinator, so the change does not transfer pressure to App installations.
  • Route the repository-token path through the runner even while enforcement is disabled so the existing attempt receipt remains truthful. Disabled mode still reaches the legacy wire path; it does not acquire a coordinator permit.
  • Require an invocation-private rate-limit detail sink before an enabled operation reaches GitHub. If isolation cannot be created, the started permit is finished as unexecuted_failure, the member is deferred as attempted=false, and no wire call occurs.
  • Preserve tuple identity, mutation fences, direct publication correctness, retry semantics for attempted failures, and Phase 0 v1/v2 telemetry.
  • Expose only bounded sanitized operator state: pool class/state/epoch, reset provenance/authority, blocked-until, permits in flight at open, already-on-wire completions, rejected/avoided operations, probe/ramp state, configuration, and telemetry completeness. No token, installation ID, private pool shard, repository/item, URL, request ID, ETag, cursor, branch, or body is public.

Scope and dependencies

Base: 56cb78d60734ddc62b5f1e49981bbb4556dcb58d
Head: 6f78f960950c822c923f7cf7f94a5211ce20f95b

The base includes Phase 0.5 PR #1150. Phase 0.5 owns signed observational reset propagation into durable queue credential circuits, deterministic per-member recovery jitter, and objective handoff recovery reasons. This PR consumes those current-main contracts without duplicating or weakening them. The coordinator remains independently reviewable and default-disabled; its later activation limits require a separate maintainer decision and stable production evidence.

Observed Phase 0.5 recovery is calibration/design evidence only: an authoritative repository_actions hold stopped dispatch through reset plus bounded jitter, then resumed service and drained backlog. It is not evidence that this Phase 1 coordinator has been activated or safely calibrated in production.

The PR remains independent of #1110's adaptive hot allocator and does not copy its scheduler observations or cursor machinery. It also excludes generic batching, request elimination/cache work, and the broader Cloudflare architecture.

Real Behavior Proof

Claim

Eight publication siblings share one repository_actions pool. Three reach the started boundary before the first 403. That throttle advances the shared epoch; five acquired-but-unstarted siblings never reach the loopback GitHub server, and the two other already-on-wire calls are the only bounded completions. State persists across Worker restart. At reset, one of eight contenders receives the probe; successful recovery proceeds through fixed 1 -> 2 -> 4 -> 8 cohorts. Target-App work remains independent.

Exercised surface

  • exact candidate 6f78f960950c822c923f7cf7f94a5211ce20f95b from a checksum-pinned Git bundle;
  • real Wrangler 4.107.0 Worker and SQLite Durable Object;
  • production signed coordinator routes/client, publication runner, artifact-to-apply path, terminal confirmation, and durable completion accounting;
  • real GitHub CLI 2.88.1 over an unmocked loopback TLS GitHub server;
  • authoritative and headerless reset paths, restart persistence, epoch fencing, one probe, recovery ramp, acknowledgement loss, process-signal forwarding, disabled-mode attempt accounting, private-detail-sink failure, attempted=false, pool independence, rollback, and privacy sentinels.

Environment and command

Docker-backed Crabbox local-container:

  • provider: local-container;
  • lease: cbx_8e8581647ea2;
  • container: 0dd7aca21129;
  • image: clawsweeper-proof:node24-jq (Node 24.19.0);
  • proof command: bash docs/proof/github-egress-pool-coordinator/run-proof.sh with GITHUB_EGRESS_POOL_PROOF_SOURCE_SHA=6f78f960950c822c923f7cf7f94a5211ce20f95b;
  • source bundle SHA-256: 2616a39c7f845fae89e36d64c9eeb70976062458662e8728eabd8f00790d056c;
  • proof receipt SHA-256: 4b112df68f60c2e124daf6341a91458803fde1e9063d9a699aa397aacc73236a.

Observed result

  • sibling permits: 8; started at first throttle: 3; rejected before start: 5;
  • loopback requests before reset: 3; already-on-wire completions: 2;
  • replayed throttle observation remained idempotent; restart persistence and stale-epoch rejection passed;
  • reset contenders: 8; probes granted: exactly 1;
  • successful recovery: 1 -> 2 -> 4 -> 8 -> closed, with no cohort refill while unresolved;
  • failed probe extension and bounded headerless fallback passed with deterministic fake time;
  • full artifact-to-apply first throttle opened the pool; sibling was attempted=false; one request was avoided;
  • real gh run download first throttle opened the pool; sibling performed zero loopback requests;
  • terminal confirmation propagated a header-only throttle and stopped its sibling;
  • over-horizon reset input and lost throttle acknowledgement entered persisted fallback safely;
  • attempted member receipt present; deferred sibling receipt absent;
  • target-App mutation progressed independently without App-pressure transfer;
  • disabled rollback reached legacy egress and retained truthful attempt accounting;
  • failure to isolate private throttle details finished the permit as unexecuted, emitted no attempt receipt, left zero permits in flight, and made zero GitHub loopback requests;
  • public privacy scan passed and telemetry was complete;
  • production mutations: 0; OpenClaw Bay affected: false; run status: succeeded.

The retained proof package contains proof-summary.json, sanitized public coordinator state, focused-test/build/install logs, and redacted Wrangler output.

Limits

  • Responses are synthetic loopback responses; the proof does not consume live quota or mutate GitHub.
  • One permit currently fences one external gh invocation. Opaque binary artifact transport internals are not fabricated as page counts.
  • Phase 0/0.5 production evidence is calibration and design input, not proof that enforcement is safe to activate.
  • Initial permit budgets, TTLs, and fallback bounds remain conservative named configuration. Production activation requires a separate decision after stable observation across complete quota windows.

Validation

  • Exact-head Docker-backed real-behavior proof above: passed; production mutations 0.
  • Exact-head broad Linux gate passed: lockfile/supply-chain checks; active-surface/static checks; core, repair, and dashboard builds; format; all lint partitions; docs; limits; changed/full coverage; and diff checks.
  • actionlint 1.7.12 passed for all workflows, excluding only the repository's documented concurrency.queue compatibility warning.
  • pnpm run test:workflow-sparse-checkout: 4/4 passed.
  • pnpm run test:no-build: 3,442 tests; 3,434 passed; 0 failed; 8 expected platform skips.
  • Final focused runner regression set: 14/14 passed in Linux, including POSIX signal forwarding; host run passed 12 with 2 POSIX-only skips.
  • git diff --check 56cb78d60734ddc62b5f1e49981bbb4556dcb58d..6f78f960950c822c923f7cf7f94a5211ce20f95b: passed; worktree clean.
  • Dirty Codex review loops resolved valid findings covering stale response replay, recovery cohorts and expiry, over-horizon resets, lost acknowledgements, attempted-bit propagation, terminal confirmation fencing, disabled-mode attempt accounting, and private throttle-detail isolation.
  • Final committed codex review --base origin/main: no actionable correctness issue.
  • Final local ClawSweeper range review: complete, high confidence, patch correct (0.93), security cleared, no comments or functional findings. Its offline proof status is expected because the external receipt is not committed; the exact-head receipt and limits are attached above for current-body hosted review.

Risks, rollout, and rollback

The merge/deploy state is dark. CLAWSWEEPER_REPOSITORY_POOL_COORDINATOR_ENABLED defaults to false, and unsafe coordinator configuration fails validation before serving permits. No activation is authorized by this PR.

Later activation must be a separate maintainer decision after Phase 0/0.5 stabilization on a fixed deployment SHA and complete quota windows. A canary must demonstrate avoided-before-start growth after throttles, zero retry/DLQ charge for coordinator deferrals, exactly one probe per reset epoch, bounded on-wire remainder, healthy other-pool progress, and no publication SLO regression.

Rollback is configuration-only: set CLAWSWEEPER_REPOSITORY_POOL_COORDINATOR_ENABLED=false. The legacy execution path remains present, persisted coordinator state is ignored while disabled, and rollback requires no queue, DLQ, or state rewrite. Revert is not recommended for backlog alone; use rollback only for coordinator-attributable correctness, isolation, or accounting regressions.

Bay impact

None while disabled. The public surface is sanitized operator telemetry and does not alter lifecycle projection. The real-boundary receipt records openclaw_bay_affected=false.

Non-goals

  • no merge, deployment, activation, schedule/gate/credential/queue/DLQ mutation;
  • no hot-review cadence or admission change;
  • no generic item batching, GraphQL/cache, or request-elimination work;
  • no target-App/public-read circuit redesign;
  • no Cloudflare Workflow/R2 rearchitecture.

@clawsweeper

clawsweeper Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper review

@clawsweeper

clawsweeper Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 12, 2026
@clawsweeper

clawsweeper Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 14, 2026, 7:50 AM ET / 11:50 UTC.

ClawSweeper review

What this changes

This PR adds a default-disabled shared coordinator that stops repository-token publication egress after a GitHub throttle and recovers it through bounded probe cohorts.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep open for maintainer review: the coordinator is not implemented on current main, but this draft is dirty against substantial newer publication work and needs a rebase plus fresh merge-result proof before landing.

Priority: P2
Reviewed head: 6f78f960950c822c923f7cf7f94a5211ce20f95b
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The exact-head proof is strong, but current-main integration needs a refresh before this broad reliability change is merge-ready.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body supplies a detailed exact-head Crabbox proof using a real Worker, SQLite Durable Object, and GitHub CLI against loopback TLS; it needs refresh only after a rebase.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body supplies a detailed exact-head Crabbox proof using a real Worker, SQLite Durable Object, and GitHub CLI against loopback TLS; it needs refresh only after a rebase.
Evidence reviewed 6 items Coordinator behavior on PR head: The proposed Durable Object admits permits, rejects new work while open, and advances the shared epoch after a 403/429 throttle.
PR-head runner boundary: The runner only enables coordination for the repository Actions pool and otherwise preserves the legacy command path.
Current-main retry contract: Current main separately preserves unattempted GitHub-rate-limit completions without consuming the publication-failure budget.
Findings None None.
Security None None.

How this fits together

Exact-review publication sends review results and lifecycle actions through GitHub credential pools. The proposed coordinator sits before repository-token egress and returns deferred work to the durable queue when that shared pool is throttled.

flowchart LR
  A[Queued exact reviews] --> B[Publication runner]
  B --> C[Repository credential pool]
  C --> D{Throttle observed?}
  D -->|No| E[GitHub publication]
  D -->|Yes| F[Coordinator deferral]
  F --> G[Durable queue retry]
  E --> H[Lifecycle completion]
Loading

Decision needed

Question Recommendation
Should the repository-token egress coordinator be rebased and re-proven against current main before it is considered for merge? Rebase and re-prove: Update the branch onto current main and rerun the documented controlled publication proof against that merge result.

Why: Current main changed adjacent publication retry behavior after the PR base, and the current merge state is dirty, so accepting the combined runtime behavior requires maintainer approval.

Before merge

  • Resolve merge risk (P1) - The dirty merge result must preserve current main’s 403/429 retryable classification and unattempted-deferral accounting; otherwise publication work can be dead-lettered or deferred incorrectly.
  • Resolve merge risk (P1) - If enabled later, a false or overlong shared pool state can pause repository-token publication and lifecycle routing until its retry boundary.
  • Complete next step (P2) - A maintainer must choose whether this cross-cutting, default-disabled coordinator should be rebased and re-proven before landing.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 32 files; +7,443/-154 lines The proposal crosses workflows, Worker storage, queue completion, runtime execution, documentation, and proof harnesses.

Merge-risk options

Maintainer options:

  1. Rebase and prove the merged behavior (recommended)
    Rebase onto current main and show that attempted and unattempted throttle completions preserve current retry and queue semantics.
  2. Hold the inactive coordinator
    Pause the draft until maintainers want to revisit a shared pre-wire publication fence.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Rebase onto current main and attach current-head Crabbox proof covering attempted and unattempted throttle completion paths.

Technical review

Best possible solution:

If maintainers want the shared pool fence, rebase it onto current main, retain the current unattempted completion contract, and attach fresh exact-head Crabbox proof for the merged workflow, queue, and runtime paths.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a feature PR; the contributor provides a detailed controlled runtime scenario, but it has not been rerun against the current-main merge result.

Is this the best way to solve the issue?

Unclear: the shared fence is a coherent design, but rebasing and proving its interaction with the newer publication path is the safer solution before merge.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e17e09425b60.

Labels

Label justifications:

  • P2: This is a bounded reliability improvement that is default-disabled rather than an active user-facing outage.
  • merge-risk: 🚨 compatibility: The branch changes publication completion and retry accounting that current main has also recently repaired.
  • merge-risk: 🚨 availability: An enabled shared circuit can defer repository-token publication work for its bounded recovery interval.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body supplies a detailed exact-head Crabbox proof using a real Worker, SQLite Durable Object, and GitHub CLI against loopback TLS; it needs refresh only after a rebase.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a detailed exact-head Crabbox proof using a real Worker, SQLite Durable Object, and GitHub CLI against loopback TLS; it needs refresh only after a rebase.

Evidence

What I checked:

Likely related people:

  • steipete: Authored the current-main repair for throttled publication classification in the same result-publication path. (role: recent publication-path contributor; confidence: high; commits: e17e09425b60; files: src/repair/publish-event-result.ts, dashboard/exact-review-queue.ts)
  • brokemac79: Authored the reviewed coordinator and the merged Phase 0.5 quota-handoff work that this PR explicitly consumes. (role: adjacent quota-recovery contributor; confidence: medium; commits: 6f78f960950c, 56cb78d60734; files: dashboard/github-egress-pool-coordinator.ts, src/repair/github-egress-pool-runner.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Rebase onto current main and rerun the documented exact-head Crabbox proof.
  • Request a fresh review for the rebased head and updated PR body.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)
  • reviewed 2026-08-12T19:15:19.962Z sha aa63d48 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-12T19:20:32.955Z sha aa63d48 :: needs real behavior proof before merge. :: [P1] Fence the apply-decisions GitHub calls
  • reviewed 2026-08-12T20:30:40.105Z sha aa63d48 :: needs real behavior proof before merge. :: [P1] Fence the direct publication and apply paths
  • reviewed 2026-08-12T23:32:46.459Z sha 3765859 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-13T03:40:09.880Z sha 6f78f96 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. proof: sufficient Contributor real behavior proof is sufficient. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 12, 2026
@brokemac79
brokemac79 force-pushed the codex/csw-123-repository-pool-circuit branch from aa63d48 to 3765859 Compare August 12, 2026 23:27
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 12, 2026
@brokemac79
brokemac79 force-pushed the codex/csw-123-repository-pool-circuit branch from 3765859 to 6f78f96 Compare August 13, 2026 03:35
@brokemac79

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant