Skip to content

fix(devices): refresh paired device last-seen metadata#88607

Merged
steipete merged 2 commits into
mainfrom
fix/paired-device-last-seen-refactor
May 31, 2026
Merged

fix(devices): refresh paired device last-seen metadata#88607
steipete merged 2 commits into
mainfrom
fix/paired-device-last-seen-refactor

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

  • Builds on and credits fix(devices): refresh paired device last-seen metadata #81189 from @vyctorbrzezowski.
  • Refreshes paired-device last-seen metadata on accepted device-token auth, paired reconnect, and first silent auto-approved connect.
  • Centralizes approved paired-device record construction so normal and bootstrap approvals preserve existing last-seen state unless the gateway passes explicit access metadata.
  • Does not change pairing policy, token verification, roles, scopes, config, or schema shape.

Fixes #81169.
Supersedes #81189.

Real behavior proof

Behavior addressed: Accepted paired-device auth and connect paths now refresh durable device-level last-seen metadata so stale paired clients can be audited.

Real environment tested: Local OpenClaw source checkout on macOS after rebasing the branch onto current origin/main.

Exact steps or command run after this patch: Ran focused paired-device auth and gateway reconnect tests, plus changed-file checks and autoreview.

Evidence after fix: node scripts/run-vitest.mjs src/infra/device-pairing.test.ts --reporter=verbose passed with 57 tests after the final rebase. node scripts/run-vitest.mjs src/gateway/server.auth.control-ui.test.ts --reporter=verbose passed with 31 tests after the final rebase. pnpm check:changed passed before the final rebase; the post-rebase rerun delegated to Testbox and stopped before checks because local Crabbox is 0.21.0 while the current Testbox wrapper requires >=0.22.0.

Observed result after fix: Device-token verification writes lastSeenReason: "device-token-auth"; paired reconnects write lastSeenReason: "connect"; first silent auto-approved local pairing also creates the paired record with lastSeenReason: "connect" in the approval write.

What was not tested: No physical mobile client or installed user pairing state was used; proof uses OpenClaw's isolated pairing and gateway harnesses.

Verification

  • node scripts/run-vitest.mjs src/infra/device-pairing.test.ts --reporter=verbose
  • node scripts/run-vitest.mjs src/gateway/server.auth.control-ui.test.ts --reporter=verbose
  • git diff --check
  • pnpm exec oxfmt --check --threads=1 src/infra/device-pairing.ts src/infra/device-pairing.test.ts src/gateway/server/ws-connection/message-handler.ts src/gateway/server.auth.control-ui.suite.ts
  • pnpm check:changed (passed before final rebase; post-rebase rerun blocked before checks by local Crabbox version 0.21.0 needing >=0.22.0)
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main (clean: no accepted/actionable findings)

@steipete steipete requested a review from a team as a code owner May 31, 2026 12:05
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M maintainer Maintainer-authored PR labels May 31, 2026
@clawsweeper

clawsweeper Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 31, 2026, 8:11 AM ET / 12:11 UTC.

Summary
Adds optional pairing access metadata so silent approvals, device-token verification, and paired reconnects refresh existing paired-device lastSeenAtMs/lastSeenReason, with focused infra and gateway tests.

PR surface: Source +53, Tests +127. Total +180 across 4 files.

Reproducibility: yes. from source inspection, though I did not execute a failing current-main repro. Current main updates token lastUsedAtMs and reconnect displayName/remoteIp without setting paired-device lastSeenAtMs/lastSeenReason.

Review metrics: 1 noteworthy metric.

  • Pairing metadata refresh surface: 2 existing optional fields refreshed; 1 optional accessMetadata object added. The PR writes durable paired-device audit metadata and extends approval helpers, so maintainers should notice the persisted/API-adjacent surface even without a schema change.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted real setup proof from an installed/local gateway or real paired-client reconnect showing paired.json or node.list last-seen metadata changing after this patch.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body has structured proof, but it is only focused Vitest/gateway harness output; external PR proof still needs a redacted real setup run that shows the after-fix metadata refresh. 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.

Risk before merge

  • [P1] Real behavior proof is mock-only: the PR reports isolated Vitest/gateway harness output and explicitly says no physical mobile client or installed pairing state was exercised.
  • [P1] The PR carries the protected maintainer label, so it should stay in explicit maintainer review even though the code path looks scoped.

Maintainer options:

  1. Decide the mitigation before merge
    Land a maintainer-reviewed version after redacted real setup proof shows an upgraded or installed paired client refreshing durable last-seen metadata without changing pairing policy, token verification, roles, scopes, config, or schema shape.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Keep this in human review because it is a protected-label PR with mock-only proof, not a narrow repair task for automation.

Security
Cleared: No concrete security or supply-chain concern found; the diff does not touch dependencies, workflows, permissions, secrets handling, or downloaded code execution paths.

Review details

Best possible solution:

Land a maintainer-reviewed version after redacted real setup proof shows an upgraded or installed paired client refreshing durable last-seen metadata without changing pairing policy, token verification, roles, scopes, config, or schema shape.

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

Yes from source inspection, though I did not execute a failing current-main repro. Current main updates token lastUsedAtMs and reconnect displayName/remoteIp without setting paired-device lastSeenAtMs/lastSeenReason.

Is this the best way to solve the issue?

Yes for the code shape: the PR updates the owner paths that approve pairings, verify device tokens, and refresh reconnect metadata while leaving policy/config/schema unchanged. The remaining blocker is proof from a real setup rather than a different implementation direction.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 94b1427fdfa1.

Label changes

Label changes:

  • add P2: This is a normal-priority gateway/device-pairing auditability bug with limited blast radius and focused code paths.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body has structured proof, but it is only focused Vitest/gateway harness output; external PR proof still needs a redacted real setup run that shows the after-fix metadata refresh. 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.

Label justifications:

  • P2: This is a normal-priority gateway/device-pairing auditability bug with limited blast radius and focused code paths.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body has structured proof, but it is only focused Vitest/gateway harness output; external PR proof still needs a redacted real setup run that shows the after-fix metadata refresh. 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

PR surface:

Source +53, Tests +127. Total +180 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 86 33 +53
Tests 1 127 0 +127
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 213 33 +180

What I checked:

  • Current main token-auth gap: On current main, verifyDeviceToken updates entry.lastUsedAtMs and persists the paired device but does not update device-level lastSeenAtMs or lastSeenReason. (src/infra/device-pairing.ts:979, 94b1427fdfa1)
  • Current main reconnect gap: On current main, reconnect metadata only carries displayName and remoteIp through clientAccessMetadata; the later updatePairedDeviceMetadata call therefore cannot refresh last-seen metadata on normal paired reconnects. (src/gateway/server/ws-connection/message-handler.ts:1114, 94b1427fdfa1)
  • PR implementation: The PR head adds lastSeenAtMs and lastSeenReason to connection access metadata, passes it into normal/bootstrap approvals, and updates device-level last-seen metadata when device-token verification succeeds. (src/infra/device-pairing.ts:1017, 89ea073f0c20)
  • Regression coverage in branch: The branch adds focused tests for approval access metadata, repair preservation without access metadata, device-token verification refresh, bootstrap approval metadata, and gateway auth matrix last-seen assertions. (src/infra/device-pairing.test.ts:792, 89ea073f0c20)
  • Sibling surface check: node-catalog already consumes paired-device lastSeenAtMs/lastSeenReason as a durable source for offline nodes, so refreshing the stored paired-device metadata feeds an existing display/query surface rather than adding a new schema shape. (src/gateway/node-catalog.ts:90, 94b1427fdfa1)
  • Related PR provenance: The PR body credits the earlier related implementation, and local commit objects show vyctorbrzezowski authored the first branch commit before the current follow-up commit. (935b2f126eca)

Likely related people:

  • David: Shallow current-main blame attributes the central pairing approval, token verification, and gateway reconnect metadata lines to commit 778c4f9; this is a routing hint, but the checkout history is grafted so deeper ownership is ambiguous. (role: recent current-main area contributor; confidence: low; commits: 778c4f90b9b3; files: src/infra/device-pairing.ts, src/gateway/server/ws-connection/message-handler.ts)
  • vyctorbrzezowski: Authored the earlier related PR and the first commit in this branch for the same paired-device last-seen behavior, so they are relevant for intent and regression context. (role: related fix proposer; confidence: medium; commits: 935b2f126eca; files: src/infra/device-pairing.ts, src/gateway/server/ws-connection/message-handler.ts, src/infra/device-pairing.test.ts)
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 added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels May 31, 2026
@steipete steipete merged commit 703fae1 into main May 31, 2026
180 of 194 checks passed
@steipete steipete deleted the fix/paired-device-last-seen-refactor branch May 31, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: M status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

paired_devices.createdAt / lastSeenAt are null — cannot identify stale paired clients

2 participants