fix(setup): approve only the pairing request setup just opened

[karkarl]

What problem

The setup fallback approved a pending pairing request by queue position or identity alone when the socket omitted a request ID. A newer foreign request could be selected, and a stale request from an earlier setup attempt could share the same device identity.

This maintainer-owned replacement supersedes #1480 without rewriting SebTardif/openclaw-windows-node. It is rebased onto current origin/main at 5a59535216eea603192f2df13d4eddf1e1a20267 and preserves the contributor's narrow pairing fix.

What changed

• Socket-provided request IDs remain authoritative and are approved exactly.
• Before each operator, node, wizard, or finalization socket opens, setup snapshots pending request IDs.
• Missing-ID fallback requires exactly one newly appearing request with the full setup device identity.
• Stale same-identity, foreign, unsafe, missing, and ambiguous requests fail closed.
• Baseline and drain list commands check exit status, parse structured JSON first, and use only an exact legacy no-pending message as compatibility fallback.
• Late cleanup excludes requests that predate the setup socket and never uses approve --latest.
• Approval request IDs are passed through the existing validated environment-variable path.

Original contributor head: edb08328e5e051c30d4604ab5527e765a0ce16ad
Maintainer replacement head: 511b17680afb33de2544e9d985a7e4feb434fcf8
Rebase conflicts: none.

Required proof pools

• windows-wsl-gateway-e2e: the unchanged-head GitHub setup/connect E2E retry passed, but no current-head adversarial final-effect trace was captured for intended, foreign, and stale pairing requests. Partially verified / blocked for the security-boundary claim.
• windows-wsl-mxc: repository-required setup/connect MXC closeout. Not verified / blocked because this host reports appcontainer-dacl, and Crabbox is not installed for remote no-skip validation.

Validation

OPENCLAW_REPO_ROOT was set to this isolated worktree. Tray tests used a unique temporary OPENCLAW_TRAY_DATA_DIR. Fresh test projects received one restore run before the required --no-restore reruns.

Command	Final result
.\build.ps1	Passed: Shared, CLI, WinNode CLI, SetupEngine, WinUI, documentation gates
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore	Passed: 4,107; skipped: 32; failed: 0
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore	Passed: 3,115; skipped: 0; failed: 0
dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore	Passed: 1,228; skipped: 1; failed: 0
Focused approval helper/operator/node/baseline/drain filter	Passed: 50; skipped: 0; failed: 0
.\scripts\validate-mxc-e2e.ps1	Blocked as required: 11 adjacent tests passed, 6 MXC proofs skipped, script exited 1 because this PC requires host DACL augmentation
GitHub Build and Test run 36205272379	Passed on unchanged-head failed-job retry. The first setup/connect attempt hit Gateway SQLite restart-intent contention and cascaded one fixture failure into 23 failures; retry passed.

One Shared rerun hit the unrelated McpHttpServerTests.Dispose_DuringInFlightHandler_DoesNotSurfaceObjectDisposedException disposal race. Its isolated rerun passed, then the complete required Shared and Tray sequence passed with the counts above.

ClawSweeper found a valid P1 on 897ed058: raw No pending substring matching could erase a valid JSON baseline when metadata contained that text. The fix checks command status first, parses JSON before fallback, and covers both valid metadata and nonzero output. Rubber-duck review then found the same bug class in both changed drain paths. Those paths and device/node regressions were fixed on 511b1768. Final rubber-duck review found no concrete source blockers.

The current-head ClawSweeper re-review at 511b1768 recognized the parser and drain fixes and reported no additional source correction. It remains needs-human because current-head adversarial final-effect Gateway evidence and the no-skip MXC pool are unavailable. Its same-identity timing concern is not widened into a protocol change in this narrow PR: exact socket IDs remain authoritative, while the compatibility fallback fails closed unless one post-baseline request matches the full identity.

Structured autoreview command:

python .agents\skills\autoreview\scripts\autoreview --mode branch --base origin/main --engine codex --model gpt-5.6-sol --thinking high

The final 65,187-character bundle passed fail-closed bundle construction, but the Codex engine could not run because the local CLI is not authenticated (codex login status: not logged in). No structured result was produced. Not verified / blocked rather than reported clean.

Real behavior proof

Current-head regression proof exercises the production selector and approval paths with stale same-identity, current same-identity, foreign, ambiguous, unsafe, missing-ID, exact socket-ID, plugin failure, misleading list metadata, nonzero list failures, and late device/node drains. The focused set passes all 50 cases and confirms stale and foreign request IDs are never sent to an approval command.

The unchanged-head GitHub setup/connect E2E retry passed the real WSL Gateway setup lane. It does not replace the requested adversarial final-effect trace. Not verified / blocked: no current-head live trace shows intended approval plus foreign/stale rejection at Gateway approval I/O, and no no-skip Gateway-to-node MXC proof was available. The live official-Gateway evidence on #1480 belongs to the old contributor head and is intentionally not reused as current-head proof.

[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 real behavior proof before merge. Reviewed September 25, 2026, 9:08 PM ET / September 26, 2026, 01:08 UTC (Revision 3).

ClawSweeper review

What this changes

Windows setup snapshots pending Gateway requests before pairing and approves either the socket-provided ID or one new request matching the full setup identity; it also changes final cleanup, documentation, and tests.

Merge readiness

⛔ Blocked before merge - 7 items remain

Keep this PR open. Current main still has the unsafe approval fallback, and this branch is a focused repair with regression coverage. No definite current-head code defect emerged from review, but the required real Gateway authority proof is incomplete.

Priority: P0
Reviewed head: 511b17680afb33de2544e9d985a7e4feb434fcf8
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The source repair and focused tests are useful, but the authority effect and required MXC pool remain unproved on this head.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: Authority-chain proof required: Current-head fake-runner tests exercise selection and approval, and setup/connect CI passed, but no current-head official Gateway trace shows intended approval plus foreign and stale rejection at final approval I/O. The added baseline fields are runtime state, so no stored-data migration applies. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Authority-chain proof required: Current-head fake-runner tests exercise selection and approval, and setup/connect CI passed, but no current-head official Gateway trace shows intended approval plus foreign and stale rejection at final approval I/O. The added baseline fields are runtime state, so no stored-data migration applies. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	9 items	Current main retains unsafe fallback: At the pinned main revision, missing-ID operator pairing invokes devices approve --latest before selecting an ID; the final device drain also uses approve --latest. Approval authority path: The introduced helper obtains pending IDs through the Gateway CLI and selects a post-baseline request by full device identity. This establishes the official Gateway CLI contract as a dependency of the changed behavior. Final approval effect: Operator fallback passes the selected request ID through the validated environment-variable path to the Gateway approval command.
Findings	None	None.
Security	Needs attention	Unproved final Gateway authorization effect: The fallback can approve one new request matching the setup identity. Current-head tests use a fake command runner, while no real Gateway trace establishes rejection of a foreign or stale candidate before approval I/O.

How this fits together

Windows setup connects operator and node clients to a local Gateway. When pairing is required, setup obtains a request ID from the socket or the Gateway CLI, approves it, and continues toward a working tray connection.

flowchart LR
 A[Gateway pending requests] --> B[Baseline snapshot]
 B --> C[Setup pairing socket]
 C --> D{Socket provides an ID?}
 D -->|Yes| E[Exact request ID]
 D -->|No| F[New full identity match]
 E --> G[Gateway approval]
 F -->|One match| G
 F -->|Unclear| H[Reject approval]

Loading

Decision needed

Question	Recommendation
Should missing-ID pairing require a usable pending-list baseline on every supported Gateway installation, even if an upgrade setup stops when listing is unavailable?	Keep fail-closed with recovery: Retain the strict baseline requirement, prove supported fresh and upgrade setups, and provide an in-product retry or Fix path for listing failures.

Why: The safe failure behavior is deliberate, but the supported-version and recovery impact has not been established by current-head upgrade proof; that compatibility contract needs owner intent.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: Current-head fake-runner tests exercise selection and approval, and setup/connect CI passed, but no current-head official Gateway trace shows intended approval plus foreign and stale rejection at final approval I/O. The added baseline fields are runtime state, so no stored-data migration applies. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve security concern: Unproved final Gateway authorization effect - The fallback can approve one new request matching the setup identity. Current-head tests use a fake command runner, while no real Gateway trace establishes rejection of a foreign or stale candidate before approval I/O.
• Resolve merge risk (P1) - Current-head evidence does not show that intended approval succeeds while foreign and stale requests are rejected before the official Gateway approval effect. A new same-identity request appearing after the snapshot remains an unproved authority case.
• Resolve merge risk (P1) - Missing-ID pairing now requires a usable pending-list baseline. Fresh and upgrade behavior on supported Gateway installations is not proven, so an existing setup could stop at pairing without a clear in-product recovery path.
• Resolve merge risk (P1) - The declared no-skip windows-wsl-mxc proof pool remains incomplete; the reported local run exited 1 after MXC proofs skipped.
• Complete next step (P2) - Capture current-head official-Gateway intended, foreign, and stale approval outcomes; complete the no-skip MXC pool and establish fresh and upgrade setup behavior. Update the PR body with redacted proof for re-review, or ask a maintainer to comment @clawsweeper re-review if the update does not trigger it.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [high] Unproved final Gateway authorization effect — src/OpenClaw.SetupEngine/ApprovalRequestHelper.cs:123

Agent review details

Security

Needs attention: The patch narrows pairing approval, but the inferred missing-ID authority path lacks current-head adversarial proof at the Gateway approval effect.

Review metrics

Metric	Value	Why it matters
Code and test line delta	production +301 net, tests +465 net	The production growth implements per-socket baselines and scoped cleanup, with larger focused regression coverage.

Merge-risk options

Maintainer options:

1. Capture Gateway final-effect proof (recommended)
   On the current head, show redacted intended approval and foreign and stale rejection at real Gateway approval I/O, then complete the declared no-skip MXC pool.
2. Pause landing
   Keep the security repair open while a suitable Windows and WSL host is arranged for the missing proof and compatibility check.

Technical review

Best possible solution:

Keep exact socket IDs authoritative and the missing-ID path fail closed, provide a visible setup retry or Fix action when listing fails, and establish allowed, foreign, and stale outcomes through the real Gateway before landing.

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

Yes. Current main calls devices approve --latest when the socket supplies no ID, and the branch's focused fake-runner test models a newer foreign request; this read-only review did not execute the test.

Is this the best way to solve the issue?

Unclear pending final-effect proof. The source change is a narrow repair, but the real Gateway must confirm that the fallback rejects forbidden requests and that supported upgrades retain a usable setup path.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

No label changes.

Label justifications:

• P0: Current main can select a foreign pending pairing request during default setup, crossing a device-approval security boundary.
• merge-risk: 🚨 security-boundary: The new identity-and-baseline fallback reaches Gateway approval, while nearest-forbidden and stale final-effect cases lack current-head Gateway proof.
• merge-risk: 🚨 compatibility: Missing-ID pairing now fails when a structured pending-list baseline is unavailable, which may stop an existing setup during upgrade.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: Current-head fake-runner tests exercise selection and approval, and setup/connect CI passed, but no current-head official Gateway trace shows intended approval plus foreign and stale rejection at final approval I/O. The added baseline fields are runtime state, so no stored-data migration applies. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

• [high] Unproved final Gateway authorization effect — src/OpenClaw.SetupEngine/ApprovalRequestHelper.cs:123
  The fallback can approve one new request matching the setup identity. Current-head tests use a fake command runner, while no real Gateway trace establishes rejection of a foreign or stale candidate before approval I/O.
  Confidence: 0.91

What I checked:

• Current main retains unsafe fallback: At the pinned main revision, missing-ID operator pairing invokes devices approve --latest before selecting an ID; the final device drain also uses approve --latest. (src/OpenClaw.SetupEngine/PairOperatorStep.cs:344, 5a59535216ee)
• Approval authority path: The introduced helper obtains pending IDs through the Gateway CLI and selects a post-baseline request by full device identity. This establishes the official Gateway CLI contract as a dependency of the changed behavior. (src/OpenClaw.SetupEngine/ApprovalRequestHelper.cs:123, 511b17680afb)
• Final approval effect: Operator fallback passes the selected request ID through the validated environment-variable path to the Gateway approval command. (src/OpenClaw.SetupEngine/PairOperatorStep.cs:382, 511b17680afb)
• Prior parser finding resolved in current source: Baseline capture checks command status and parses JSON before accepting the exact legacy no-pending message; both drain paths likewise check status before selection. (src/OpenClaw.SetupEngine/ApprovalRequestHelper.cs:34, 511b17680afb)
• Focused regression coverage: Tests exercise stale and foreign requests, exact socket IDs, ambiguous matches, misleading valid JSON metadata, failed list commands, and approval command selection through a fake command runner. (tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs:5310, 511b17680afb)
• Current-head proof gap: The PR body reports passing local tests and a successful unchanged-head setup/connect CI retry, but explicitly records no current-head adversarial Gateway approval-I/O trace and a skipped, failing MXC proof run. The official Gateway trace on fix(setup): approve only the pairing request setup just opened #1480 is for that PR's different head.

Likely related people:

• Scott Hanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Karen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Barbara Kudiess: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

• Add final-effect proof for the nearest unauthorized request and show that stale or reassigned authority is rejected before Gateway approval I/O; redact credentials, IP addresses, and private endpoints.
• Complete the declared current-head WSL pairing and no-skip MXC pools, including fresh and upgrade setup behavior.

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 (2 earlier review cycles)

• reviewed 2026-09-25T23:52:08.621Z sha 897ed05 :: needs real behavior proof before merge. :: [P1] Parse list JSON before declaring the baseline empty
• reviewed 2026-09-26T00:36:47.589Z sha 511b176 :: needs real behavior proof before merge. :: none

[karkarl]

@clawsweeper re-review

The P1 JSON-baseline finding and the same bug class in both changed drain paths are fixed at 511b176. Current-head regression tests cover misleading valid metadata and nonzero list failures for baseline, device drain, and node drain. The PR body now contains final validation and the remaining live-proof blockers.

[clawsweeper]

🦞👀
Exact review queued.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/36205569416
• Updated: 2026-09-26T00:38:25.360Z

[karkarl]

Closeout at 511b17680afb33de2544e9d985a7e4feb434fcf8:

• Current-head GitHub Build and Test run 36205272379 passed on unchanged-head failed-job retry. The first setup/connect attempt hit the known Gateway SQLite restart-intent contention and cascaded through the shared fixture; the retry passed.
• ClawSweeper re-reviewed the current head, recognized the parser/drain corrections, and remains needs-human for missing adversarial Gateway final-effect proof and no-skip MXC proof.
• Local required build, Shared, Tray, SetupEngine, and 50 focused pairing tests pass. Rubber-duck review found no remaining source blocker.
• Codex autoreview remains blocked because the local Codex CLI is not authenticated; no clean structured result is claimed.
• No merge was performed. Active maintainer ownership is ending with proof blockers recorded in the PR body.