fix(gateway): report draining state in readiness#94915
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 22, 2026, 4:24 AM ET / 08:24 UTC. Summary PR surface: Source +8, Tests +31. Total +39 across 3 files. Reproducibility: yes. from source inspection: current main rejects enqueues while Review metrics: none identified. Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this narrow readiness wiring after maintainer review and clean or explained exact-head checks, while keeping Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main rejects enqueues while Is this the best way to solve the issue? Yes. Reusing the existing command-queue drain predicate inside readiness before the cached channel snapshot is the narrowest owner-boundary fix; clearing stale drain state and shutdown health behavior stay in separate existing paths. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f2eca9439189. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +8, Tests +31. Total +39 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
|
286b1d8 to
eb4cb57
Compare
8639e9b to
a1741a3
Compare
a1741a3 to
0e8d189
Compare
|
Merged via squash.
Thanks @markoub! |
Merged via squash. Prepared head SHA: 0e8d189 Co-authored-by: markoub <[email protected]> Co-authored-by: vincentkoc <[email protected]> Reviewed-by: @vincentkoc
Summary
/readyzreport not ready while the gateway command queue is draining for restart.isGatewayDraining()queue state;/healthz, queue admission, restart reset, and channel readiness behavior stay unchanged.mainalready clears stale drain state on in-process restart viaresetAllLanes().Linked context
Which issue does this close?
Closes #78136
Which issues, PRs, or discussions are related?
Related #78144
Was this requested by a maintainer or owner?
ClawSweeper marked #78136 as source-reproducible and queueable on June 18, 2026; #78144 was closed for active-PR queue policy, not a technical rejection.
Real behavior proof (required for external PRs)
/readyzcould report ready while the command queue was rejecting new work withGatewayDrainingError.HOME/OPENCLAW_STATE_DIR, real gateway server in minimal test-gateway mode (VITEST=1 OPENCLAW_TEST_MINIMAL_GATEWAY=1) with channels, providers, cron, browser-control, Gmail watcher, canvas host, and bundled plugins disabled.startGatewayServer(), fetched/readyz, calledmarkGatewayDraining(), fetched/readyz, calledresetAllLanes(), fetched/readyz, then closed the server cleanly.{ "before": { "status": 200, "body": { "ready": true, "failing": [] } }, "draining": { "status": 503, "body": { "ready": false, "failing": ["gateway-draining"] } }, "recovered": { "status": 200, "body": { "ready": true, "failing": [] } } }/readyzflips to HTTP 503 withgateway-drainingduring drain and recovers to HTTP 200 afterresetAllLanes().check:changedpassed with Corepack bin prepended.mainalready hasisGatewayDraining()and rejects enqueues while draining, butcreateReadinessChecker()had nogetGatewayDraininginput and only reported startup/channel/event-loop readiness; ClawSweeper's source review on Docker in-process gateway restart can leave command queue draining while healthz/readyz report OK #78136 reported the same source-level mismatch.Tests and validation
pnpm install --frozen-lockfilenode scripts/run-vitest.mjs src/gateway/server/readiness.test.ts src/gateway/server-http.probe.test.ts src/process/command-queue.test.tsgit diff --check origin/main...HEADCI=1 PATH="/opt/homebrew/Cellar/corepack/0.35.0/bin:$PATH" pnpm check:changed/readyzproof described abovecodex review --base origin/main: no regression found in the changed gateway readiness pathRisk checklist
Did user-visible behavior change? Yes
Did config, environment, or migration behavior change? No
Did security, auth, secrets, network, or tool execution behavior change? No
Highest-risk area: readiness semantics during restart drain.
Mitigation: check is transient, uncached, uses existing
isGatewayDraining()state;/healthzremains shallow liveness and reset behavior remains inresetAllLanes().Current review state
Next action: maintainer review and CI.
Waiting on: CI and maintainer review.
Bot/reviewer comments addressed: ClawSweeper's #78136 source-repro/fix-shape-clear guidance; prior #78144 shape resubmitted narrowly after queue-policy closure.
AI-assisted: Implemented with AI assistance; reviewed locally by Marko with focused tests, live gateway proof, changed gate, and Codex review.