Skip to content

fix: device pairing floods stacked approval alerts for a retrying device and stale approvals no-op#100976

Merged
steipete merged 2 commits into
mainfrom
claude/mystifying-rubin-ff6dc4
Jul 6, 2026
Merged

fix: device pairing floods stacked approval alerts for a retrying device and stale approvals no-op#100976
steipete merged 2 commits into
mainfrom
claude/mystifying-rubin-ff6dc4

Conversation

@steipete

@steipete steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Closes #100974

What Problem This Solves

Fixes an issue where users with a device stuck in pairing repair (for example a Mac node whose stored token no longer verifies) would wake up to an unbounded stack of "New Mac wants to connect" approval alerts in the Mac app — roughly one new alert per 5 minutes — and where clicking Approve on each stacked alert silently did nothing except reveal the next stale alert.

Why This Change Was Made

The gateway expired pending device-pairing requests 5 minutes after creation even while the device kept retrying (refresh preserves ts for anti-queue-jumping, and the TTL also keyed on ts), so every TTL window it silently pruned the old request and minted a fresh requestId plus a device.pair.requested broadcast. Pending requests now carry a store-internal refreshedAtMs keepalive so an actively retrying device keeps one live request (and one approvable alert); ts still owns approval ordering and the implicit --latest race protection, and the field never crosses the protocol boundary (stripped like node pairing's internal revision). Superseded device requests now broadcast device.pair.resolved, mirroring the existing node-pairing behavior. The Mac app additionally keeps at most one queued alert per device, dedupes by requestId, closes a visible alert that got superseded, and resyncs its queue from the gateway when approving a stale request fails — so an already-accumulated alert pile collapses after a single click.

User Impact

  • One approval alert per requesting device instead of hundreds; approving it works on the first click.
  • Devices retrying overnight no longer flood device.pair.requested broadcasts to every operator-scoped client (Mac app, control UI).
  • Existing stacked alerts self-heal: the first stale Approve resyncs the queue with gateway truth.
  • No protocol or config surface change; pending-state files are transient (5-minute TTL) so no migration is needed.

Evidence

  • New regression test: re-requests keep one pending request alive past the TTL without churning requestIds, keepalive never leaks into public payloads, stale keepalive still expires (src/infra/device-pairing.test.ts).
  • New assertions for superseded requestId reporting (src/infra/device-pairing.test.ts, src/infra/device-pairing-churn.test.ts).
  • New Swift test: a newer device pairing request supersedes queued requests for the same device (apps/macos/Tests/OpenClawIPCTests/NodePairingApprovalPrompterTests.swift).
  • Testbox: pnpm test src/infra/device-pairing.test.ts src/infra/device-pairing-churn.test.ts → 62/62 passed.
  • Testbox: full pnpm check:changed gate green (typecheck lanes, lint, import cycles, pairing guards) — run https://github.com/openclaw/openclaw/actions/runs/28801209780.
  • Local macOS: full swift test --filter PairingApprovalPrompterTests build + 6/6 tests passed (arm64e, macOS 26.5).
  • Codex autoreview (gpt-5.5, --mode local): clean, no actionable findings.

@steipete
fix(gateway): stop device pairing approval alert floods from retrying…
b63b50c 
… devices

A device retrying with a broken token minted a new pending requestId (and
approval alert broadcast) every 5-minute TTL window because refresh preserved
ts and expiry keyed on ts. Pending requests now stay alive via an internal
refreshedAtMs keepalive (ts still owns ordering/--latest), superseded device
requests broadcast device.pair.resolved like node pairing already did, and the
Mac app keeps one alert per device, closes superseded visible alerts, and
resyncs its queue when approving a stale request.

Closes #100974
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: macos App: macos gateway Gateway runtime plugin: file-transfer size: M maintainer Maintainer-authored PR labels Jul 6, 2026
@clawsweeper

clawsweeper Bot commented Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 12:02 PM ET / 16:02 UTC.

Summary
The branch adds a store-internal device-pairing keepalive, broadcasts superseded device-pairing resolutions, coalesces stale macOS pairing alerts, and updates focused TypeScript/Swift tests plus docs.

PR surface: Source +57, Tests +49, Docs +2, Other +61. Total +169 across 11 files.

Reproducibility: yes. Source inspection on current main shows active retries preserve the original ts while TTL pruning keys on ts, so retries can churn request IDs and the macOS queue keeps stale alert objects; I did not run the hours-long live macOS scenario.

Review metrics: 1 noteworthy metric.

  • Pending-state contract: 1 store-internal field added; 0 protocol/config fields added. The pairing TTL behavior changes transient state, so maintainers should notice that the new keepalive is stripped before public list/get responses.

Stored data model
Persistent data-model change detected: persistent cache schema: src/infra/device-pairing.test.ts, serialized state: src/infra/device-pairing.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100974
Summary: This PR is the direct open fix candidate for the canonical macOS device-pairing alert flood issue.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Next step before merge

  • [P1] Protected maintainer-labeled PR with no narrow automated repair finding; the next action is human review/landing rather than a ClawSweeper fix PR.

Maintainer decision needed

  • Question: Should maintainers proceed with the protected-label review and landing path for this gateway plus macOS pairing fix after exact-head checks settle?
  • Rationale: The PR has a protected maintainer label, and the remaining choice is human acceptance of a high-priority auth/pairing fix rather than a mechanical repair.
  • Likely owner: steipete — He owns the current PR branch and has prior merged history on the macOS pairing prompt surface.
  • Options:
    • Proceed with maintainer review (recommended): Review/land the PR if exact-head required checks remain acceptable because the patch matches the source root cause and has focused regression coverage.
    • Request live macOS proof first: Ask for a short live macOS pairing alert click-through if maintainers want runtime UI proof beyond Swift/gateway tests before landing.
    • Pause for replacement approach: Pause this branch only if maintainers prefer a different product contract for pending-request expiry during active retries.

Security
Cleared: No concrete security or supply-chain regression was found; the auth-sensitive change keeps the new TTL keepalive internal and preserves existing approval scope checks.

Review details

Best possible solution:

Land this gateway/macOS pairing fix after maintainer review and exact-head gates, keeping refreshedAtMs internal and preserving ts as the approval-ordering contract.

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

Yes. Source inspection on current main shows active retries preserve the original ts while TTL pruning keys on ts, so retries can churn request IDs and the macOS queue keeps stale alert objects; I did not run the hours-long live macOS scenario.

Is this the best way to solve the issue?

Yes. The best fix boundary is the combination used here: keep one live gateway request for active retries, broadcast superseded request IDs, and make the macOS queue recover from stale local alerts.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P0: The PR fixes a Mac app device-pairing recovery/setup blocker where a retrying device can create unbounded stale approval alerts whose Approve action no-ops.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is protected by the maintainer label, so the external-contributor real-behavior proof gate is not applied; the body provides focused Testbox/CI and local macOS Swift test evidence rather than a live alert recording.

Label justifications:

  • P0: The PR fixes a Mac app device-pairing recovery/setup blocker where a retrying device can create unbounded stale approval alerts whose Approve action no-ops.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is protected by the maintainer label, so the external-contributor real-behavior proof gate is not applied; the body provides focused Testbox/CI and local macOS Swift test evidence rather than a live alert recording.
Evidence reviewed

PR surface:

Source +57, Tests +49, Docs +2, Other +61. Total +169 across 11 files.

View PR surface stats
Area Files Added Removed Net
Source 3 71 14 +57
Tests 2 50 1 +49
Docs 2 4 2 +2
Config 0 0 0 0
Generated 0 0 0 0
Other 4 75 14 +61
Total 11 200 31 +169

What I checked:

  • Repository policy read: Read root AGENTS.md plus the relevant scoped docs and gateway guides; their review-depth, gateway hot-path, docs-link, and protected-label guidance were applied. (AGENTS.md:1, dc600d4a77fd)
  • Live PR state: Live PR data shows this PR is open, non-draft, carries the protected maintainer label, and closes the linked device-pairing alert flood issue. (b9d43b4ea96b)
  • Current-main source reproduction: Current main prunes pending device requests by ts while refresh intentionally preserves ts, so an actively retrying repair request can age out and mint fresh request IDs. (src/infra/device-pairing.ts:198, dc600d4a77fd)
  • Pairing keepalive fix: The PR stores refreshedAtMs only on the internal pending record, keeps approval ordering on ts, and strips the field from public pending payloads. (src/infra/device-pairing.ts:417, b9d43b4ea96b)
  • Superseded request broadcast: The gateway now emits device.pair.resolved for superseded device request IDs before handling the live request, matching the existing node-pairing resolution pattern. (src/gateway/server/ws-connection/message-handler.ts:1445, b9d43b4ea96b)
  • macOS alert recovery: The macOS device prompter resyncs after stale approval failure and coalesces queued requests by device/request ID so stale alerts collapse instead of stacking. (apps/macos/Sources/OpenClaw/DevicePairingApprovalPrompter.swift:147, b9d43b4ea96b)

Likely related people:

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.

@clawsweeper clawsweeper Bot mentioned this pull request Jul 6, 2026
Closed
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. labels Jul 6, 2026
@steipete
steipete merged commit 0a488bd into main Jul 6, 2026
173 of 176 checks passed
@steipete
steipete deleted the claude/mystifying-rubin-ff6dc4 branch July 6, 2026 16:20
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 7, 2026
@steipete
fix: device pairing floods stacked approval alerts for a retrying dev…
f40f0ec 
…ice and stale approvals no-op (openclaw#100976)

* fix(gateway): stop device pairing approval alert floods from retrying devices

A device retrying with a broken token minted a new pending requestId (and
approval alert broadcast) every 5-minute TTL window because refresh preserved
ts and expiry keyed on ts. Pending requests now stay alive via an internal
refreshedAtMs keepalive (ts still owns ordering/--latest), superseded device
requests broadcast device.pair.resolved like node pairing already did, and the
Mac app keeps one alert per device, closes superseded visible alerts, and
resyncs its queue when approving a stale request.

Closes openclaw#100974

* chore: satisfy swiftformat and refresh native i18n inventory for pairing prompter changes
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
@steipete @giodl73-repo
fix: device pairing floods stacked approval alerts for a retrying dev…
6797e8e 
…ice and stale approvals no-op (openclaw#100976)

* fix(gateway): stop device pairing approval alert floods from retrying devices

A device retrying with a broken token minted a new pending requestId (and
approval alert broadcast) every 5-minute TTL window because refresh preserved
ts and expiry keyed on ts. Pending requests now stay alive via an internal
refreshedAtMs keepalive (ts still owns ordering/--latest), superseded device
requests broadcast device.pair.resolved like node pairing already did, and the
Mac app keeps one alert per device, closes superseded visible alerts, and
resyncs its queue when approving a stale request.

Closes openclaw#100974

* chore: satisfy swiftformat and refresh native i18n inventory for pairing prompter changes
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app: macos App: macos docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. plugin: file-transfer rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

1 participant

@steipete