fix(repair): cut over durable cluster intake publication - #873
Conversation
7b54925 to
7ecbf77
Compare
|
Codex review: found issues before merge. Reviewed July 27, 2026, 5:16 AM ET / 09:16 UTC. ClawSweeper reviewWhat this changesThis PR moves cluster-repair intake and result publication to durable state intents, exact-path materialization, scoped GitHub App credentials, retry handling, and updated operator documentation. Merge readiness⛔ Blocked by patch quality or review findings - 10 items remain This PR remains necessary, but the revised head still has two P1 workflow failures and one P2 documentation overstatement. The durable-intake direction is maintainer-supported, yet the production cutover should stay open until target identity reaches result publication, publisher reruns cannot halt later recovery, and live recovery proof is provided. Priority: P1 Review scores
Verification
How this fits togetherCluster repair intake converts a selected GitCrawl cluster into durable repair state, then materializes that state and dispatches a planning worker. This PR changes the boundary between intake, state publication, worker-result publication, and self-heal recovery. flowchart LR
A[GitCrawl cluster snapshot] --> B[Cluster intake workflow]
B --> C[Durable intake queue]
C --> D[State materializer]
D --> E[Repair planning worker]
E --> F[Result publication]
F --> G[Durable repair state]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Carry the validated target repository through the durable worker/result artifact into result publication, handle each publisher-rerun failure without stopping later recovery, document at-least-once workflow creation with intended deduplicated worker execution, and attach redacted live recovery evidence. Do we have a high-confidence way to reproduce the issue? Yes, from source: configure an allowed Is this the best way to solve the issue? No. The intended durable cutover is appropriate, but the publisher must use the validated target identity and must isolate rerun errors before it can safely replace the existing recovery path. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 7f1696ce8874. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
943b32a to
5f52f30
Compare
5f52f30 to
b17912e
Compare
e65c5bd to
363d5df
Compare
b17912e to
210fa53
Compare
|
Thanks for this stack — the durable-intake direction is exactly where the codebase is going, and the bottom three PRs are being landed now (#881 as-is, #882/#883 with bounded maintainer fixes: append-window priority starvation guard, accepted-intent provenance in the durable contract, strict v2 ledger validation). #884 and this PR need one structural rework before they can land, plus a few scoped fixes: Dispatch authority (blocking, #884): after the queue row is acked, mutable git ledger/job files become the recovery source, and recovery uses the materializer secret to bless that state into a fresh dispatch. That means a state writer (or plain state corruption) can manufacture a pending ledger + policy-valid job. Recovery should derive from canonical DO state, or verify an immutable accepted-intent receipt (what #883 will now carry) before dispatching. This mirrors the records cutover we just completed in #879/#885 — git is projection, never authority. Also for #884: isolate malformed cluster_intake rows (one poison row currently re-fails the whole drain — same pattern as the record-tuple dead-lettering), persist the claim before the side effect, and align dispatch-receipt-owner.sh with the TS observer (they disagree on what counts as a successful prior run, which permits a duplicate planning pass). For this PR: mint target-read tokens for the validated target rather than hard-coding Both will also want a rebase once #881–#883 land. Happy to pair on the recovery model — the canonical tuples endpoint from #879 is probably the right substrate for the accepted-intent receipt. |
363d5df to
d83b6c1
Compare
210fa53 to
06c463b
Compare
d83b6c1 to
555e25d
Compare
06c463b to
69dea88
Compare
555e25d to
8b0733d
Compare
69dea88 to
011fdee
Compare
Rework of the #873 cutover layer on top of the #900 dispatch rework: - Result publication resolves the validated target repositories from the downloaded worker artifacts (result.repo, fail-closed against the allowed owner) and mints its target-read token for those repositories instead of hard-coding openclaw/openclaw. - Intake hydrates state with a read-only, non-persisted credential and drops its contents-write grants; it never publishes from its checkout. - Result publication narrows the ClawSweeper app token and workflow permissions to read (state writes use the state credential). - A failed publisher rerun in self-heal is reported per run and never aborts subsequent publisher retries or cluster self-heal work. - Docs state the accurate guarantee: at-least-once workflow creation with exactly-once worker execution intent, receipt-gated worker-side dedupe, and job-path-only worker concurrency.
|
Superseded by #901 (merged) — thank you @RomneyDa for the whole stack! Everything landed: #881 as-is, #882/#883 with maintainer fixes, #884 via #900, and this cutover via #901 with a live proof that also caught and fixed a latent owner-list bug on main. Deferred proof: the first naturally selected cluster dispatch will exercise the claim->dispatch->recovery window live (watch results/cluster-repair-intake/*.json for dispatch_claimed->dispatched and exactly one planning run). |
) (#901) * fix(repair): cut over durable cluster intake publication * fix(repair): rework cluster intake cutover per maintainer review Rework of the #873 cutover layer on top of the #900 dispatch rework: - Result publication resolves the validated target repositories from the downloaded worker artifacts (result.repo, fail-closed against the allowed owner) and mints its target-read token for those repositories instead of hard-coding openclaw/openclaw. - Intake hydrates state with a read-only, non-persisted credential and drops its contents-write grants; it never publishes from its checkout. - Result publication narrows the ClawSweeper app token and workflow permissions to read (state writes use the state credential). - A failed publisher rerun in self-heal is reported per run and never aborts subsequent publisher retries or cluster self-heal work. - Docs state the accurate guarantee: at-least-once workflow creation with exactly-once worker execution intent, receipt-gated worker-side dedupe, and job-path-only worker concurrency. * fix(repair): honor the CLAWSWEEPER_ALLOWED_OWNER owner-list contract across the cluster cutover lanes The production CLAWSWEEPER_ALLOWED_OWNER variable is a comma-separated owner list (openclaw,steipete; issue #604 / allowedRepairOwners), but three cutover gates treated it as a single owner and failed closed on every dispatch: - repair-cluster-intake.yml 'Resolve target repository' rejected the list outright (live proof run 30302458881 failed here). - resolve-result-targets rejected the list before minting the reader token; it now validates every result owner against the parsed list and fails closed when results span multiple owners, since one publication mints one reader token. - restore-cluster-intake-job compared the job-path owner against the raw list string, which could never match. Proof enablement, no-op on main: the intake wake and worker dispatch now run on the invoking revision (GITHUB_REF_NAME / CLAWSWEEPER_DISPATCH_REF), and repair-publish-results gains a validated workflow_dispatch lane for a completed worker run while workflow_run events stay pinned to trusted default-branch code. * fix(repair): accept the pnpm-forwarded -- separator in publish-cluster-intake The hosted runner's pnpm forwards the literal -- from 'pnpm run repair:publish-cluster-intake -- <intent>', so the CLI read '--' as the intent path and durable acceptance failed with ENOENT '.../--' (live proof run 30303202343). Use the first real positional and cover the separator in a CLI regression test. * fix(repair): only offer clusters that satisfy the durable acceptance contract The cluster-intake acceptance policy (#900) requires at least two candidate references, but the importer offered single-open-member clusters and the selector could choose one, failing the whole intake run at durable acceptance with 'cluster intake job reference policy mismatch' (live proof run 30304188033). Filter selection to clusters with >= 2 open members and skip single-candidate clusters defensively in the job loop. * fix(repair): publish results from the event's trusted ref instead of a hard main pin The mid-job 'git checkout -B main origin/main' pin mixed main's package.json with a branch-built dist/ on the manual publication lane, so repair:state-delta-paths was missing at 'Commit result ledger' (live proof run 30308450188). workflow_run events keep publishing default-branch code; a workflow_dispatch publishes its own write-gated ref. --------- Co-authored-by: Dallin Romney <dallinromney@gmail.com>
Summary
This is 5/5 in the cluster-fixer reliability stack, based on PR 884. This existing PR remains the top layer so its audit and review history are preserved.
repair:publish-mainpublication to the durable cluster-intake append/materializer contract.allow_merge: false, and all existing production gate settings.Stack
Audit evidence
repair:publish-main.Validation
pnpm run build:all,pnpm run check:static, lint, formatting,git diff --check, and focused concurrency/stale-checkout/lease-loss/duplicate/recovery/preservation/dispatch/security/ranking suites pass.463e53e2ac3fa016b582a8a7ee637028ba815817, exactly matching the rebased unsplit implementation.provider=aws, but broker/AWS authentication is unavailable on this host; allocation failed before creating a lease. No provider override was used. Hosted Linux checks remain required on every PR.timeoutabsent, Linux containment symbols,/private/varaliasing, and load-sensitive timing). Stack-specific suites passed.Post-merge production acceptance
Do not call the fixer healthy until production verifies two distinct clusters with no duplicate dispatch or state loss, and at least one eligible cluster produces a substantive review-ready PR. If a fresh batch has no eligible candidate, retain the selector rejection report as the correct outcome.
After all five PRs land, preserve current gates and run:
Record append-to-publication and intake-to-dispatch latency, verify unrelated job/ledger hashes are unchanged, verify one planning worker per stable dispatch key, require explicit terminal outcomes, and replay the same store to prove idempotency. Do not enable automerge or merge generated production PRs as part of that proof.