fix: keep a native-admin upgrade reset from exiting the API

[SebTardif]

What Problem This Solves

A native-admin WebSocket upgrade waits for admission before the proxy attaches a socket error listener. If the client resets the TCP connection during that wait, Node reports ECONNRESET on the socket. With nobody listening, that event exits the API process.

Evidence

The command imports createNativeAdminAccess from this tree, sends an upgrade to agent-a.agents.example.test, and resets the socket while admission is still waiting.

Before the listener:

uncaught ECONNRESET read ECONNRESET

After the listener:

stayed-up

Real behavior proof

• Behavior or issue addressed: A client reset during native-admin upgrade admission exits the API process.
• Real environment tested: macOS, Node v26.10.0, this branch of openclaw-enterprise.
• Exact steps or command run after this patch: node /tmp/ent-upgrade-proof.mjs, which calls createNativeAdminAccess, writes the upgrade, then resetAndDestroy while admissionVerifier.verify is still pending.
• Evidence after fix: terminal output from that command:

stayed-up

• Observed result after fix: The process printed stayed-up and exited 0. The same command on the previous source printed uncaught ECONNRESET read ECONNRESET and exited 2.
• What was not tested: A signed-in console session that finishes admission and proxies to a gateway.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed October 8, 2026, 12:35 AM ET / 04:35 UTC (Revision 4).

ClawSweeper review

What this changes

The branch handles native-admin client resets during WebSocket admission, preserves denial audits, and prevents disconnected clients from opening gateway connections.

Example: A client resets an upgrade to agent-a.agents.example.test while admission waits

• Before: The reported run prints uncaught ECONNRESET read ECONNRESET and exits 2.
• After: The reported run prints stayed-up and exits 0.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused patch is useful and correct, with sufficient fault-path proof whose scope remains a production-owner harness.
Proof confidence	🦐 gold shrimp (3/6)	Sufficient (terminal): The reported macOS run drives the actual native-admin production owner with a real TCP client reset during pending admission and records recovery from ECONNRESET/exit 2 to stayed-up/exit 0. This is sufficient scoped internal-reliability proof; synthetic admission does not establish a full signed-in gateway workflow. No stored-data contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A browser connection reset while native-admin access is being checked can take down the API.
Reason: This restores process availability with a narrow repair to the existing native-admin handler. The area's exact-head review confirms the direction and audit ordering.

Merge readiness

✅ Ready for maintainer review

This PR remains useful: current main still has the admission-time socket-error gap. The earlier audit and CI-registration findings are fixed, and no actionable patch defect remains.

Likely related people: kevinlin-openai and freeqaz are relevant routing candidates from native-admin history and review context.

Priority: P1
Reviewed head: 82fbec878dd0c4103f625f323316a873b0234c48

Before merge

None.

Findings

None.

Agent review details

How this fits together

OpenClaw Control Plane proxies an authorized operator’s browser connection to an Agent’s private OpenClaw gateway. The changed admission handler checks the session and exact-Agent permission before starting that connection.

flowchart TD
 A[Browser WebSocket upgrade] --> B[Guard client socket errors]
 B --> C[Check session and exact-Agent permission]
 C -->|Denied| D[Record attributable denial audit]
 C -->|Allowed| E{Client still connected?}
 E -->|No| F[Stop without gateway connection]
 E -->|Yes| G[Resolve private transport and recheck client]
 G --> H[Proxy to Agent gateway]

Loading

Technical review

Best possible solution:

Keep socket-error handling at the production admission boundary while preserving exact-Agent denial audits and existing gateway authorization.

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

Yes: current-main source exposes an unhandled socket-error interval during admission, and the contributor reports a real TCP reset producing ECONNRESET before the fix. This read-only review did not execute it.

Is this the best way to solve the issue?

Yes: attaching the listener before the await repairs the owning boundary without changing authentication, settings, or persisted state; the disconnected-client guards retain denial auditing.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against ccb76e6a89ae.

Provenance checked

• apps/controller/src/http/native-admin.ts:handleNativeAdminUpgrade keeps the original intent (refactor(http): extract Agent and native-admin handlers #873: Extract native-admin admission, sockets, and shutdown auditing while preserving the existing resource authorization and lifecycle behavior.)
• docs/flows/agent-native-admin.md keeps the original intent (feat: add trusted-operator Agent native admin UI pilot #256: Document shared-session access for trusted operators, exact-Agent authorization, private routing, and attributable connection auditing.)
• scripts/ci/test-suites/checks-baseline-2.json keeps the original intent (ci(tests): run audited Checks and Conformance files concurrently #1398: Run audited, isolated test files concurrently while keeping unlisted files serial.)

Testing

Proof path: in-process harness. Added test files: 1.

Security

None.

Evidence

What I checked:

• Pinned production fix and preserved audit ordering: The introduced delta adds the error listener before admission awaits, retains denial auditing before either disconnected-client return, and checks socket destruction after transport resolution. (apps/controller/src/http/native-admin.ts:858, 82fbec878dd0)
• Still necessary on current main: Current main retains the unguarded admission await; the source and flow files have no changes from the pinned merge base. The merged relay-drain repairs address established connections rather than this admission gap. (apps/controller/src/http/native-admin.ts:860, ccb76e6a89ae)
• Real TCP reset proof: The complete captured PR body, source revision 27fd64098ca0a4af46e7cdeeabae16633e808f60672eddec9023e765b2ffa87e, reports macOS with Node v26.10.0 exercising createNativeAdminAccess through a real TCP upgrade and resetAndDestroy during pending admission: before-fix ECONNRESET and exit 2; after-fix stayed-up and exit 0. The signed-in gateway workflow is explicitly untested.
• Meaningful regression coverage: The new file uses real Fastify and TCP sockets for reset handling, then checks that a pending synthetic authorization denial still produces one attributable audit event. Authentication and controller collaborators are synthetic; this is not installed IAM or database proof. (tests/conformance/native-admin-upgrade-reset.test.mjs:10, 82fbec878dd0)
• Re-review continuity: Production code and the regression file are unchanged since the prior reviewed head; the latest commit updates only the source-backed flow. Both prior blocking findings remain resolved. (docs/flows/agent-native-admin.md:139, 82fbec878dd0)
• Area review approval: kevinlin-openai approved the exact head, explicitly confirming reset handling, attributable denial auditing, disconnected-client ordering, and the flow update. (82fbec878dd0)

Likely related people:

• kevinlin-openai: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• freeqaz: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Review metrics

Metric	Value	Why it matters
Production and test growth	production +11 lines; tests +204 lines	The small runtime repair is supported by two distinct TCP reset and denial-audit regressions.

Labels

Label changes:

No label changes.

Label justifications:

• P1: A client reset during native-admin admission can terminate the API process, making this an urgent availability repair.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/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.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

History

Review history (3 earlier review cycles)

• reviewed 2026-10-07T21:59:45.796Z sha 16b5adb :: blocked before merge. :: [P2] Preserve denial auditing when the client disconnects | [P2] Register the regression test with a CI suite
• reviewed 2026-10-07T22:05:42.573Z sha 9c90a0d :: blocked before merge. :: [P2] Preserve denial auditing when the client disconnects
• reviewed 2026-10-08T01:24:45.556Z sha 002d0f7 :: needs maintainer review before merge. :: none

[freeqaz]

Free's clanker here (an AI agent reviewing on Free's behalf).

Review of 9c90a0da9. The crash fix is right and needed. The early return after the first admission needs one change.

The crash. Confirmed. With the socket.on("error") line removed, the new test fails with an uncaught read ECONNRESET. With it, the test passed 20/20 at 2 CPUs on a trial merge with main f17e9f84d. A remote client could otherwise exit the API by resetting during admission, so this matters.

Agreeing with the bot's denial-audit point. The if (socket.destroyed) return; right after nativeAdminProxyContext runs before appendNativeAdminProxyDenialAudit. A signed-in caller who is denied for the exact Agent and resets while IAM is pending leaves no authorization_denial record. The HTTP path still writes one. Suggested fix: drop the first check, or move it below the denial branch, and keep the second one. nativeAdminProxyTransportContext only reads the gateway API key, so letting it run for a dead socket costs nothing. A test for a disconnect while a denial is pending would pin the order.

The second check also fixes an older leak. On main, if the client goes away during admission and admission then succeeds, proxyNativeAdminWebSocket gets a socket that has already closed. Its once("close") never fires, so the upstream upgrade goes ahead. That writes a connect audit for a client that is gone and keeps the gateway socket open. Returning before the proxy avoids all of that.

Interaction with the recent relay changes (#1659, #1666). Those changed how native-admin-proxy.ts closes the browser socket: it lingers to drain after the gateway's clean EOF and only cuts on timeout. The listener added here stays attached for the whole connection, but it only calls destroy() on a socket that is already erroring, so drain and cut behave the same. The proxy's own once("error") handler still runs after it and records client_disconnect as before. On the trial merge, native-admin-websocket-relay.test.mjs (3/3) and native-admin-access.test.mjs (8/8) pass. There are no textual conflicts with main or with #906, the other open PR that edits this file.

The suite JSON change only registers the new local-TCP test. I found no CI-surface concerns. This is ready once the denial ordering is fixed; it's for a maintainer to merge.

[SebTardif]

@freeqaz

Suggested fix: drop the first check, or move it below the denial branch, and keep the second one.

Yes. The denial branch now runs before the destroyed-socket return.

A reset while exact-Agent authorization is still pending still appends one authorization_denial row, with the principal and the Agent. The return before the gateway proxy stays, so a disconnected caller does not open that socket. Commit 002d0f7.

[kevinlin-openai]

Reviewed the native-admin upgrade reset path at 82fbec8. The listener covers admission-time socket errors, denial auditing remains attributable, and disconnected clients stop before gateway proxying. The required flow update is included. Merge remains subject to the current-head CI gates.