fix(devices): recover unknown requestId approvals in password mode by zy84338719 · Pull Request #100546 · openclaw/openclaw

zy84338719 · 2026-07-06T01:54:55Z

What Problem This Solves

openclaw devices approve <requestId> still deadlocks for local loopback gateways in password auth mode when node-host refreshes the pending device repair request before approval lands.

PR #85342 fixed the compatible replacement-request path after local fallback has already been triggered by a connection-level pairing required error. Password auth mode is different: the CLI can connect successfully, then the gateway can fail at method time instead of reaching the connection-level fallback. In real loopback password auth, the CLI can also connect device-less and lose operator.pairing before requestId validation.

Repro path:

Local loopback gateway uses password auth.
Node-host reconnect refreshes a pending device repair request before approval lands, or the password-auth CLI reaches a device-less method authorization failure.
openclaw devices approve <requestId> connects to the local gateway.
Gateway returns method-level unknown requestId or missing scope: operator.pairing rather than a connection-level pairing required error.
Current main returns null/prints an approval failure; it never attempts local file fallback.

Changes

Extract isLocalLoopbackGateway() so fallback eligibility can be reused without requiring a pairing-required connect error.
Add tryLocalApproveForLoopback() for method-level password auth failures.
Recover both method-level unknown requestId and real loopback password-auth missing scope: operator.pairing denials.
Reuse the existing strict findSameDeviceReplacementRequest() compatibility gate for refreshed requests.
Surface forbidden approval results instead of silently returning null.
Keep fallback loopback-only and disabled for explicit --url remote gateways.

Evidence

Local validation on this branch:

npx pnpm@latest exec vitest run src/cli/devices-cli.test.ts
Test Files  1 passed (1)
Tests  55 passed (55)

npx pnpm@latest check:test-types
exit 0

Focused tests cover:

password loopback missing scope: operator.pairing falling back to local list
password loopback missing scope: operator.pairing falling back to local approve
unknown requestId approving the original local pending request
unknown requestId approving a compatible refreshed replacement
forbidden approval surfacing clearly
explicit remote --url not using local fallback

Redacted live proof against a running local password-mode gateway:

OpenClaw source branch: 37babcd0 + f8c65b74
Gateway auth mode: password (running gateway)
Temporary HOME/OPENCLAW_STATE_DIR: [redacted temp dir]
Temporary config has no gateway.remote.url, so CLI target is implicit local loopback.
Created local pending request: 1781c97f-82d1-47a9-98ba-c78d329cfaa7

$ HOME=[temp] OPENCLAW_STATE_DIR=[temp]/state OPENCLAW_CONFIG_PATH=[temp]/openclaw.json node dist/index.js devices approve 1781c97f-82d1-47a9-98ba-c78d329cfaa7 --password [redacted]
Direct scope access failed; using local fallback.
Direct scope access failed; using local fallback.
Approved proof-device-pr100546 (1781c97f-82d1-47a9-98ba-c78d329cfaa7)

$ HOME=[temp] OPENCLAW_STATE_DIR=[temp]/state node --import tsx -e "listDevicePairing()"
{
  "pendingCount": 0,
  "paired": [
    {
      "deviceId": "proof-device-pr100546",
      "roles": ["operator"],
      "scopes": ["operator.pairing"],
      "tokenRoles": ["operator"]
    }
  ]
}

Fixes #59889.
Supersedes the incomplete coverage in #100455.
Related #85342, #31041, #37749.

Copilot

Pull request overview

This PR extends the devices approval fallback logic to recover from method-level unknown requestId errors that can occur in password auth mode when targeting an implicit local loopback gateway, avoiding a local-node approval deadlock.

Changes:

Extracts reusable isLocalLoopbackGateway() eligibility logic from the pairing-required fallback path.
Adds tryLocalApproveForLoopback() to locally approve the original pending request (or a compatible same-device replacement) when the gateway returns unknown requestId.
Adds/extends CLI tests to cover password-mode unknown-request recovery and fail-closed behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/cli/devices-cli.runtime.ts	Adds loopback eligibility helper + local approval recovery path for method-level `unknown requestId` errors in password auth mode.
src/cli/devices-cli.test.ts	Adds tests for password-mode fallback behavior and explicit `--url` non-fallback behavior.

  it("does not use local fallback when an explicit --url is provided", async () => {
    rejectGatewayForLocalFallback();

+  it("does not fall back locally when --url points at a remote gateway (password mode)", async () => {
+    // Simulate password-mode unknown-requestId error with explicit --url
+    callGateway.mockRejectedValueOnce(
+      Object.assign(new Error("unknown requestId"), { gatewayCode: "INVALID_REQUEST" }),
+    );
+
    await expect(
-      runDevicesCommand(["list", "--json", "--url", "ws://127.0.0.1:18789"]),
-    ).rejects.toThrow("pairing required");
+      runDevicesApprove(["req-1", "--url", "ws://10.0.0.5:18789"]),
+    ).rejects.toThrow("unknown requestId");
    expect(listDevicePairing).not.toHaveBeenCalled();
+    expect(approveDevicePairing).not.toHaveBeenCalled();
+  });
+});


+  it("approves a same-device replacement when original requestId is gone (password mode)", async () => {
+    // list for resolveApprovePairingGatewayContext
+    rejectGatewayWithUnknownRequestId();
+    // approve call fails
+    rejectGatewayWithUnknownRequestId();
+    // Local state: original request is gone, replacement exists
+    listDevicePairing.mockResolvedValueOnce({
+      pending: [
+        {


+ *
+ * Returns the approval result on success, `null` if the request is genuinely
+ * absent from local state, or `undefined` if local fallback is not applicable
+ * (non-loopback or no local pending entries).


clawsweeper · 2026-07-06T02:14:49Z

Codex review: found issues before merge. Reviewed July 8, 2026, 8:23 PM ET / 00:23 UTC.

Summary
The PR adds loopback-local device approval/list fallback for password-mode method-level unknown requestId and missing scope: operator.pairing errors, with focused CLI tests.

PR surface: Source +151, Tests +272. Total +423 across 2 files.

Reproducibility: yes. Source inspection shows current main returns null on method-level unknown requestId and the gateway emits missing scope: operator.pairing for unauthorized device-pair methods; the PR body also includes a redacted live password-mode loopback transcript.

Review metrics: 1 noteworthy metric.

Method-level approval failures handled: 2 added: unknown requestId and missing scope: operator.pairing. Both are auth-sensitive gateway failures, so maintainers should review the fallback contract separately from ordinary CLI output changes.

Root-cause cluster
Relationship: canonical
Canonical: #100546
Summary: This PR is the current canonical candidate for the password-mode method-level approval fallback; earlier and adjacent items cover replacement churn, UI alert flooding, or an incomplete version of the same fix.

Members:

fixed_by_candidate: devices approve deadlock after gateway restart — node-host repair request cannot be approved via CLI #59889 - The PR body links this deadlock report as the issue being fixed by the proposed password-mode fallback.
superseded: fix(devices): local fallback for unknown requestId in password auth mode #100455 - The same author opened an earlier incomplete version of the password-mode unknown-requestId fallback, and this PR adds tests plus live proof.
partial_overlap: fix(cli): recover replaced device approvals #85342 - The merged PR added same-device replacement fallback after connection-level pairing-required errors, but it does not cover method-level password-mode failures.
partial_overlap: [Bug]: Mac app floods unbounded stacked device pairing approval alerts for one retrying device; approving stale alerts is a no-op #100974 - The closed issue shares stale device-pair request symptoms, but its canonical merged fix addressed request churn and native alert flooding rather than this CLI password-mode fallback.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Preserve non-loopback missing scope: operator.pairing errors instead of returning the generic no-request result.
[P2] Add the explicit --url missing-scope regression test.
[P2] Get auth/CLI owner acceptance for the loopback-only local approval contract before merge.

Risk before merge

[P1] Explicit remote or otherwise non-loopback missing scope: operator.pairing approval failures are downgraded to a misleading no-request result instead of preserving the gateway authorization error.
[P1] Even after the narrow code fix, the PR expands an auth-sensitive local file approval path after method-level gateway rejection, so an auth/CLI owner should accept the loopback-only security boundary before merge.
[P1] The branch is behind the current base and should be refreshed after the blocker is fixed, but the observed risk is in the patch behavior rather than ordinary base drift.

Maintainer options:

Preserve auth errors first (recommended)
Make the missing-scope approve branch rethrow or preserve the original gateway error for explicit --url and other non-loopback targets, with a focused regression test.
Accept the bounded fallback contract
After the regression is fixed, maintainers may intentionally accept the loopback-only local file approval path as the password-mode deadlock recovery contract.
Pause for a wider recovery contract
If local file approval should not grow further, pause this PR in favor of a broader doctor, UI, or gateway recovery design.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
In `src/cli/devices-cli.runtime.ts`, change the `missing scope: operator.pairing` approve branch so it only returns `null` after an applicable loopback local fallback attempt; rethrow the original error for explicit `--url` or non-loopback targets, and add a `devices approve --url ...` missing-scope regression test in `src/cli/devices-cli.test.ts`.

Next step before merge

[P2] A narrow automated repair can preserve non-loopback missing-scope auth errors and add the missing CLI regression test; owner acceptance remains needed before merge.

Maintainer decision needed

Question: After the non-loopback missing-scope bug is fixed, should method-level unknown requestId and loopback missing scope: operator.pairing failures be allowed to approve local pairing files with the existing admin-equivalent local fallback contract?
Rationale: The source review can verify the loopback gates, but the PR changes device-pairing authorization recovery behavior after gateway rejection, so maintainer intent is needed for the security-boundary acceptance.
Likely owner: hxy91819 — They authored the merged same-device replacement fallback contract that this PR extends.
Options:
- Accept bounded fallback after fix (recommended): Fix the non-loopback auth-error regression, keep fallback loopback-only, and allow an auth/CLI owner to approve the recovery contract before merge.
- Require gateway-only recovery: Reject local file approval for method-level password-mode failures and require a gateway, doctor, or UI recovery design instead.
- Pause for broader recovery design: Hold this PR if approval, doctor, Control UI, and native prompt recovery need one coordinated contract rather than a CLI-only fallback.

Security
Cleared: No supply-chain changes or concrete auth bypass were found; the auth-sensitive fallback is loopback-gated but still needs maintainer security-boundary acceptance before merge.

Review findings

[P2] Preserve non-loopback missing-scope errors — src/cli/devices-cli.runtime.ts:583

Review details

Best possible solution:

Close this PR as superseded by #85342.

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

Yes. Source inspection shows current main returns null on method-level unknown requestId and the gateway emits missing scope: operator.pairing for unauthorized device-pair methods; the PR body also includes a redacted live password-mode loopback transcript.

Is this the best way to solve the issue?

No, not as submitted. The loopback fallback is a reasonable narrow fix, but the missing-scope branch must preserve remote/non-loopback authorization failures instead of converting them into no-request output.

Full review comments:

[P2] Preserve non-loopback missing-scope errors — src/cli/devices-cli.runtime.ts:583
This remains the same blocker from the prior review. When device.pair.approve fails with missing scope: operator.pairing, this branch calls the loopback fallback helper, but for explicit --url or other non-loopback targets the helper returns undefined and this line returns null; runDevicesApproveCommand then reports “No pending device request” even though the gateway rejected authorization. Keep the local fallback loopback-only, but rethrow or otherwise preserve the original missing-scope error when fallback is not applicable.
Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P0: The PR targets a device-approval deadlock that can leave local gateway/node recovery blocked with no supported CLI/UI approval path.
merge-risk: 🚨 compatibility: The current diff changes the user-visible result for non-loopback missing scope: operator.pairing failures from an auth error to a no-request message.
merge-risk: 🚨 auth-provider: The diff changes device-pair approval recovery behavior and the local scopes used to mint or preserve device auth tokens.
merge-risk: 🚨 security-boundary: The new fallback can approve local pairing files after the gateway rejects the method-level approval request, so the authorization boundary needs owner acceptance.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes redacted terminal output from a running local password-mode gateway showing the fallback approval and paired-state result after the fix.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted terminal output from a running local password-mode gateway showing the fallback approval and paired-state result after the fix.

Evidence reviewed

PR surface:

Source +151, Tests +272. Total +423 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	162	11	+151
Tests	1	272	0	+272
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	434	11	+423

Acceptance criteria:

[P1] pnpm test src/cli/devices-cli.test.ts.
[P1] pnpm check:test-types.
[P1] git diff --check.

What I checked:

linked superseding PR: fix(cli): recover replaced device approvals #85342 (fix(cli): recover replaced device approvals) is merged at 2026-05-22T13:44:16Z.
cluster evidence: the durable review links that PR in the work cluster or recommended risk path.
no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

hxy91819: Authored the merged same-device replacement approval fallback that this PR extends and reuses. (role: prior replacement-fallback author; confidence: high; commits: 6ea907cec1b3; files: src/cli/devices-cli.runtime.ts, src/cli/devices-cli.test.ts, src/infra/device-pairing-churn.test.ts)
vincentkoc: Recently changed the local CLI shared-token/password scope boundary that controls device-less password-mode calls and loopback-local behavior. (role: recent shared-auth contributor; confidence: high; commits: 16865d58ca1f, 039f8fb16d; files: src/gateway/call.ts, src/gateway/server/ws-connection/handshake-auth-helpers.ts, src/cli/devices-cli.runtime.ts)
RomneyDa: Recently worked on device-pairing requestId churn and devices CLI recovery guidance on the same approval failure surface. (role: recent requestId churn contributor; confidence: medium; commits: f749de4f0a6a; files: src/infra/device-pairing.ts, src/cli/devices-cli.runtime.ts, src/cli/devices-cli.test.ts)
steipete: Recently landed the adjacent Mac/gateway pairing flood fix that preserves a live requestId and broadcasts superseded device-pair resolutions. (role: recent adjacent pairing contributor; confidence: medium; commits: 0a488bd30b26; files: src/infra/device-pairing.ts, src/infra/device-pairing.test.ts, apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

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.

Review history (2 earlier review cycles)

reviewed 2026-07-06T02:27:19.806Z sha 37babcd :: needs real behavior proof before merge. :: none
reviewed 2026-07-09T00:17:33.885Z sha f8c65b7 :: found issues before merge. :: [P2] Preserve non-loopback missing-scope errors

In password auth mode, the CLI connects to the gateway successfully but the pending pairing request may have been refreshed by a node-host reconnect before approval lands. This produces a method-level 'unknown requestId' error instead of the connection-level 'pairing required' error that resolveLocalPairingFallback expects. The existing fallback in approvePairingWithFallback only triggers on 'pairing required' connection errors. In password auth mode it never fires, leaving the user in an unresolvable deadlock. While openclaw#85342 added same-device replacement logic, that code only runs *after* the fallback triggers — it does not help when the fallback itself is never reached. Changes: - Extract isLocalLoopbackGateway() from resolveLocalPairingFallback - Add tryLocalApproveForLoopback(): when gateway returns unknown requestId on a loopback gateway, attempt file-based approval - Case 1: original requestId still in local pending.json → approve - Case 2: node-host refreshed it → find compatible replacement via existing findSameDeviceReplacementRequest (strict publicKey/roles/ client-metadata/scope checks) - Both forbidden results surface via formatDevicePairingForbiddenMessage - Wire into both isUnknownRequestIdError exit paths in approvePairingWithFallback (admin-retry catch and main catch) - Add focused tests: unknown requestId → approve locally, unknown requestId → approve compatible replacement, forbidden surfacing Fixes openclaw#59889

zy84338719 · 2026-07-09T00:11:18Z

@clawsweeper re-review\n\nAdded live password-mode loopback proof and patched the real method-level scope-denial path observed during proof. Validation: npx pnpm@latest exec vitest run src/cli/devices-cli.test.ts (55 passed) and npx pnpm@latest check:test-types (exit 0).

knoal · 2026-07-13T15:32:35Z

A/B verified locally against base SHA. Ran vitest with the PR's test file against the PR's fix (B) and against the base's fix (A).

Result: CLEAN A/B

A (PR test + base fix): FAIL - 1 test failed
B (PR test + PR fix):   PASS

The new test falls back to local list when password loopback auth lands fails on base, passes with the fix. Failing-first signature is correct. Looks ready to merge.

Copilot AI review requested due to automatic review settings July 6, 2026 01:54

openclaw-barnacle Bot added cli CLI command changes size: M labels Jul 6, 2026

Copilot started reviewing on behalf of zy84338719 July 6, 2026 01:55 View session

openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 6, 2026

Copilot AI reviewed Jul 6, 2026

View reviewed changes

zy84338719 force-pushed the fix/devices-approve-password-mode-v2 branch from 479c601 to b964f5d Compare July 6, 2026 02:12

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 6, 2026

zy84338719 force-pushed the fix/devices-approve-password-mode-v2 branch from b964f5d to 37babcd Compare July 6, 2026 02:18

clawsweeper Bot mentioned this pull request Jul 6, 2026

[Bug]: Mac app floods unbounded stacked device pairing approval alerts for one retrying device; approving stale alerts is a no-op #100974

Closed

fix(devices): recover password loopback scope denials

f8c65b7

steipete mentioned this pull request Jul 9, 2026

fix(devices): local fallback for unknown requestId in password auth mode #100455

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(devices): recover unknown requestId approvals in password mode#100546

fix(devices): recover unknown requestId approvals in password mode#100546
zy84338719 wants to merge 2 commits into
openclaw:mainfrom
zy84338719:fix/devices-approve-password-mode-v2

zy84338719 commented Jul 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

zy84338719 commented Jul 9, 2026

Uh oh!

knoal commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

zy84338719 commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Changes

Evidence

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

clawsweeper Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zy84338719 commented Jul 9, 2026

Uh oh!

knoal commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zy84338719 commented Jul 6, 2026 •

edited

Loading

clawsweeper Bot commented Jul 6, 2026 •

edited

Loading