Repository navigation
fix(gateway): prevent workspace icon failures after startup - #167697
Conversation
Bind HTTP requests and cancellation cleanup to the live Gateway instead of retired startup work. Preserve acknowledged background hooks and export bounded client/shutdown cancellation counts without unhandled-error noise. Observe response completion before transport teardown so Node 24 buffered disconnects are counted without treating normal connection closes as aborts.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesThe Gateway gives HTTP requests their own live execution lifetime so workspace icons load after startup, disconnected requests cancel cleanly, and accepted background hooks continue. Example: A user loads a workspace icon after Gateway startup completes.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review This PR addresses a distinct, source-backed Gateway defect that remains on current main. No actionable patch defect was identified; the latest commit resolves the prior stability-recorder omission. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe Gateway HTTP server receives browser resource requests and hook calls, checks their existing authorization, and dispatches them to resource or agent owners. Request cleanup must finish before the Gateway releases its runtime resources. flowchart TD
A[Browser or hook client] --> B[Gateway HTTP server]
B --> C[Live Gateway execution owner]
C --> D[Request lifetime and existing authorization]
D --> E[Resource response]
D --> F[Accepted independent hook run]
G[Client disconnect or Gateway shutdown] --> D
D --> H[Cancellation cleanup and bounded metrics]
Technical reviewBest possible solution: Keep HTTP execution and cleanup under the existing live Gateway owner while accepted hooks retain their independent lifetime. Do we have a high-confidence way to reproduce the issue? Yes. The startup-retirement mechanism is clear in source and the recorded public Gateway comparison demonstrates HTTP 500 before the repair and HTTP 200 afterward; this read-only review did not execute target code. Is this the best way to solve the issue? Yes. The patch repairs execution custody at the shared HTTP boundary, reuses existing transport and detached-work owners, and leaves resource authorization and stored settings intact. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 3223e13a514c. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
PR surfaceSource +121, Tests +260, Docs +11, Other -1. Total +391 across 16 files. Added test files: 0. View PR surface stats
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (3 earlier review cycles)Reviewed October 9, 2026, 5:51 AM ET / 09:51 UTC (Revision 4). |
Keep normal outcomes explicitly undefined and retain the failure outcome used by cancellation accounting. Remove the obsolete Prometheus line-limit suppression and its retired baseline entry.
|
Pull request was converted to draft
|
Landing with the authorized inherited-CI exception for head CI run 37912289175 completed with 65 jobs passed and 22 skipped. Its only root failure is Scheduled main run 37912611958, job 113761463036 independently fails identically on the exact tested main baseline The final head passed independent review and ClawSweeper with zero findings. Focused remote proof, types, lint, formatting, and ratchets are recorded in the PR body. No CI rerun or branch refresh was used to chase green. |
Bind HTTP request lifetime directly to the completed Gateway kernel instead of resolving the full route context before request routing. Preserve live execution tracking, disconnect and shutdown cancellation, and joined cleanup. Fix the eager lookup introduced by 41589bb (#167697). Keep all existing cold-route and authority assertions, and wire HTTP fixtures to the same lifetime input used by production.
…#167697) Give HTTP requests a live Gateway-owned, disconnect-aware work scope instead of retaining the retired startup scope. Join request cleanup, keep acknowledged hooks on their detached lifetime, and retain diagnostics for unexpected errors. Count expected client/shutdown cancellations in Prometheus without filling the stability event buffer. Real Gateway proof changed workspace icons from 4/4 HTTP 500 to 4/4 HTTP 200 and matching error records from 4 to 0. Focused remote regressions, lint, types, ratchets, and all Node/UI CI tests passed. Land with the verified inherited-CI exception for the Windows signal-listener TS2554 reproduced on main run 37912611958; main already fixes it in 84898cc.
Bind HTTP request lifetime directly to the completed Gateway kernel instead of resolving the full route context before request routing. Preserve live execution tracking, disconnect and shutdown cancellation, and joined cleanup. Fix the eager lookup introduced by 731adfa (openclaw#167697). Keep all existing cold-route and authority assertions, and wire HTTP fixtures to the same lifetime input used by production.
What Problem This Solves
Fixes workspace icons returning HTTP 500 after successful Gateway startup, with repeated AbortError stacks in the Gateway log.
User Impact
HTTP reads use a live request lifetime. Disconnects cancel cooperating work quietly, shutdown closes unfinished responses, and acknowledged hooks continue under their independent Gateway lifetime. Unexpected errors retain their diagnostics. Operators can observe cancellations through
openclaw_gateway_http_cancelled_total{source="client"|"shutdown"}. Normal cancellations stay out of the stability event buffer.Why This Change Was Made
HTTP listeners inherited the startup AsyncWorkScope after the run loop retired it. Requests now enter the live Gateway owner and get a disconnect-aware child scope. Cancellation and draining restore that request context so abort listeners can schedule cleanup and the Gateway joins it before releasing resources.
The transport owner records successful response completion before Node's normal connection-close handling. This matters on Node 24, which can emit
finishwithwritableFinished=trueafter a buffered write loses its socket. NormalConnection: closeresponses and server-selected rejections remain excluded from cancellation counts.Current UI already deduplicates these image loads. The tested virtual-remount flow issued one metadata request and one image request, retained the image across an actual row unmount/remount, and produced zero failed/cancelled media requests. No UI patch was justified by this evidence.
Non-test/non-doc LOC: +120, using the requested diff filter. The growth supplies the missing cancellation/cleanup lifetime and bounded metric. Existing metric cases move to their current timing owner, and the now-unneeded line-limit suppression is removed. No user configuration, schema, or dependency change.
Evidence
Full public Gateway with synthetic state and a paired-device token; startup retired before the first request:
Document, JS, agent-avatar, user-avatar, and assistant-media controls remained 200. The comparison used real routes and public Gateway startup, with no injected route or minimal Gateway. The final rebased candidate also passed all six route families on Node 24, logged zero HTTP errors, and shut down cleanly. The UI proof used current source served by the existing Vite E2E helper and a mocked Gateway; it is not a packaging test.
blacksmith-testboxleasetbx_01m4fcnm2krqcahzgh6qjjm3y4.tbx_01m4fppr74xnj09jgfva92r0pq: 16 HTTP lifetime cases passed (30.70s cold one-worker wall time), 92 lifecycle/diagnostics/Prometheus cases passed, and 358 Gateway siblings passed with one independently reproduced baseline failure. Export collision and eager-import/PR-wrapper closure guards passed (20 closure tests).blacksmith-testboxleasetbx_01m4ft94fhkbp987fnshcbxz84: exact staged tree verified; core, extension, and scripts lint passed; 123 affected tests passed throughnode scripts/run-vitest.mjs … --maxWorkers=1in 53.80s. The fullnode scripts/check-changed.mjs --staged --base f8e594e461efa21db360d345e986a2d0a543d5a4gate passed all typechecks and early guards. Its only failure is the dead-export scan; unchanged main source at that exact base reproduces the same four findings. Every remaining check from the owning plan was run separately and passed, including lint, schema, import-cycle, webhook, and pairing guards. The complete integration batch took 39m58s.cbx_a4477a7324caafter Testboxtbx_01m4fy9mykahqexnx71ky785j9stalled in admission: the old recorder failed withexpected 4 to be 3;node scripts/run-vitest.mjs src/logging/diagnostic-stability.test.ts src/logging/diagnostic-stability-bundle.test.ts src/infra/diagnostic-events.test.ts extensions/diagnostics-prometheus/src/service.test.ts --maxWorkers=1passed 84 tests in 40.98s. Core and logging-test typechecks passed. After simplifying the callback for the executable-line ratchet, the final source passed 37 logging/bundle tests in 33.65s, targeted lint, both size ratchets, formatting, and diff checks. That final run took 4m29s end to end. All leases are stopped.Known old-base failure:
server.hooks-admission.test.ts/returns typed admission failures and leaves the idempotency key retryablereturned 502 instead of 200 on both candidate and unchanged15305ccd53dedd1663a89f92a402480d7b71bbf8. The dead-export findings also reproduce on main basef8e594e461efa21db360d345e986a2d0a543d5a4:bindCliQuestionAnswerAuthority,prepareSessionEventTargetForHost,readSessionEntryCreationTransition, andRESTART_CONTINUATION_CONTEXT_PREFIX. These scans remain failed; no baseline or ignore exception was added.Historical logs did not include URLs, so the original production route distribution remains unmeasured. The reproduced AbortError originated in server startup retirement; this proof found no reduction in browser aborts. The previous-head CI run (
37906971427, head7441d36e6ea54a0c96e6ef8aec637b6e067b1d44) took 12m51s wall time and 14,012 summed executed-job seconds. All Node tests and all nine UI E2E shards passed; its sole root failure was the newly added event missing from the stability recorder's exhaustive switch. That omission is repaired here and covered by the existing bounded-projection regression. Updated-head CI (37912289175, headec1d1a4a42202ec639df3db8e404e5e5f4cfdc23) passes the repaired lint shard. It has one independently confirmed inherited type failure:src/cli/update-cli.windows-lifecycle.test.ts(545,11), TS2554 from calling a signal listener without its argument. Scheduled main run37912611958, job113761463036, fails identically on the exact tested baselinecff1f4fcb9015c99af0c3b104d88ed13fa08b309; the test, type configuration, package manifest, and lockfile are byte-identical in the PR merge. Main already fixes it in3223e13a514c6134e10ee7f585627434d245fd20. Final CI completed in 15m38s wall time, including 2m17s queued, and 15,172 summed job seconds: 65 passed, 22 skipped, and only the inherited root plus aggregate failed. All nine UI E2E shards passed. ClawSweeper reviewed the exact final head with zero findings and no pre-merge actions. This PR uses the authorized inherited-CI landing exception; no CI rerun or branch refresh was requested.