Skip to content

fix: route mobile exec approvals to reviewer device#95175

Merged
joshavant merged 5 commits into
mainfrom
fix/mobile-exec-approval-targets
Jun 21, 2026
Merged

fix: route mobile exec approvals to reviewer device#95175
joshavant merged 5 commits into
mainfrom
fix/mobile-exec-approval-targets

Conversation

@joshavant

@joshavant joshavant commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Propagate the initiating Gateway device id through chat-triggered agent turns as approval reviewer context.
  • Bind reviewer device ids on trusted internal exec approval registration so mobile clients with operator.approvals can see and resolve their own chat-triggered exec approvals.
  • Surface Gateway exec.approval.requested and exec.approval.resolved operator events in the iOS foreground app, so mobile approval UI does not depend solely on APNs delivery/notification taps.
  • Add protocol, gateway, exec request, and iOS model coverage for reviewer-device visibility, resolver authorization, exec request propagation, and foreground approval-event handling.

Evidence

  • node scripts/run-vitest.mjs src/gateway/server-methods/approval-shared.test.ts src/gateway/server-methods/server-methods.test.ts src/agents/bash-tools.exec-approval-request.test.ts passed: 221 tests.
  • xcodebuild test -project apps/ios/OpenClaw.xcodeproj -scheme OpenClaw -destination 'id=D13D9552-08B1-42B1-A444-D429D4BA655E' -only-testing:OpenClawTests/NodeAppModelInvokeTests passed: 71 tests.
  • .agents/skills/autoreview/scripts/autoreview --mode local reported no accepted/actionable findings before commit.
  • Azure review VM redeployed with OpenClaw image built from PR SHA cd2ad47743f59af624433340c658e2055eca3f8b.
  • Review VM validation passed: pnpm validate:host-health, pnpm validate:security-controls -- --public-url https://openclaw-foundation-ios-review.westus2.cloudapp.azure.com, and pnpm validate:reviewer-setup -- --setup-payload-file ....
  • Clean iPhone 17 Pro simulator validation passed: erased simulator, installed this branch, applied setup code, connected to the review host, sent Run the approval demo., saw the foreground Exec approval required prompt with Allow Once, approved it, and Chat replied The approval demo completed.
  • Azure broad changed gate was attempted twice after AWS did not bootstrap. Both broad runs failed only in untouched src/gateway/server-channels.test.ts lifecycle timeout cases; a focused Azure rerun of that file/project passed 40/40 tests.

Known unrelated failure

The broad Azure changed gate currently appears blocked by an unrelated src/gateway/server-channels.test.ts timing flake in the gateway-server project. The branch does not modify that file or the channel lifecycle code, and the focused rerun passed on Azure.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Jun 20, 2026
@clawsweeper

clawsweeper Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 9:05 AM ET / 13:05 UTC.

Summary
The PR propagates the initiating Gateway device ID through chat-triggered agent and Codex exec approval registration, then handles foreground iOS exec approval requested/resolved events.

PR surface: Source +80, Tests +459, Other +64. Total +603 across 30 files.

Reproducibility: yes. Source inspection on current main shows no reviewer-device binding and foreground iOS server events ignore exec approval events; the PR body adds after-fix review-host and clean iPhone simulator proof.

Review metrics: 1 noteworthy metric.

  • Gateway approval protocol field: 1 optional field added. The reviewer-device field is protocol-visible and changes approval visibility semantics, so maintainers should notice it before merge.

Stored data model
Persistent data-model change detected: vector/embedding metadata: packages/gateway-protocol/src/schema/exec-approvals.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

  • Get maintainer confirmation for the reviewer-device approval boundary.

Risk before merge

  • [P1] Reviewer-device IDs widen which operator device can observe and resolve a pending exec approval, so the approval-runtime plus operator.approvals boundary needs maintainer confirmation.
  • [P1] The Gateway protocol gains one optional exec approval request field with trusted-runtime-only semantics, which is additive but still compatibility-sensitive for schema consumers and Gateway clients.
  • [P1] The current head is mergeable but UNSTABLE because required CI/security checks are still queued; this should wait for normal required checks before merge.

Maintainer options:

  1. Confirm Reviewer-Device Boundary (recommended)
    Have a Gateway or approval owner confirm that trusted approval-runtime registration may bind the initiating reviewer device and that operator.approvals remains the resolver gate.
  2. Strengthen The Trusted-Runtime Contract
    If maintainers want a stronger pre-merge contract, add focused docs or tests naming approvalReviewerDeviceIds as trusted-runtime-only exec approval metadata.
  3. Pause For Approval-Model Design
    If named reviewer devices are not the intended ownership model, pause this PR and decide the approval visibility model before merging.

Next step before merge

  • [P2] The protected maintainer label and approval-boundary change require human maintainer review; there is no narrow automated repair target.

Security
Cleared: No concrete auth bypass or supply-chain issue was found; reviewer binding is trusted-runtime-only and resolving still requires operator.approvals, but the boundary change needs maintainer confirmation.

Review details

Best possible solution:

Land after a Gateway/approval owner confirms the reviewer-device model, keeping reviewer binding limited to trusted approval-runtime registration and operator.approvals clients.

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

Yes. Source inspection on current main shows no reviewer-device binding and foreground iOS server events ignore exec approval events; the PR body adds after-fix review-host and clean iPhone simulator proof.

Is this the best way to solve the issue?

Yes, with maintainer sign-off. Threading the initiating Gateway device through chat run context into trusted approval-runtime registration is narrower than exposing backend approvals to all approval-scoped operators.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority mobile/Gateway exec approval workflow fix with real user impact and limited approval-path blast radius.
  • merge-risk: 🚨 compatibility: The PR adds an optional Gateway protocol request field and changes exec approval visibility behavior for existing clients and schema consumers.
  • merge-risk: 🚨 security-boundary: The PR changes which operator device can observe and resolve a pending exec approval, which is an authorization boundary.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body reports after-fix review-host and clean iPhone simulator validation showing the foreground exec approval prompt, Allow Once resolution, and completed chat reply.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports after-fix review-host and clean iPhone simulator validation showing the foreground exec approval prompt, Allow Once resolution, and completed chat reply.
Evidence reviewed

PR surface:

Source +80, Tests +459, Other +64. Total +603 across 30 files.

View PR surface stats
Area Files Added Removed Net
Source 24 87 7 +80
Tests 4 465 6 +459
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 2 80 16 +64
Total 30 632 29 +603

What I checked:

Likely related people:

  • vincentkoc: Live commit history shows recent shared approval lookup/registration refactors and Codex remote-node dynamic-tool work in the files this PR extends. (role: recent approval visibility and Codex dynamic-tool contributor; confidence: high; commits: 6464f8d1d924, c9e481ac48e5, 56eadf36d019; files: src/gateway/server-methods/approval-shared.ts, src/gateway/server-methods/exec-approval.ts, extensions/codex/src/app-server/dynamic-tool-build.ts)
  • ngutman: Merged history shows the iOS exec approval notification flow and watch approval hardening that this PR extends into foreground Gateway events. (role: iOS exec approval flow contributor; confidence: high; commits: 28955a36e7b0, 6f566585d84f; files: apps/ios/Sources/Model/NodeAppModel.swift, apps/ios/Sources/Push/ExecApprovalNotificationBridge.swift)
  • joshavant: Beyond authoring this PR, recent merged history shows adjacent exec normalization and iOS configuration work on affected surfaces. (role: adjacent exec and iOS contributor; confidence: medium; commits: 80227005a016, 377f6181a91b, c932bf377b54; files: src/agents/bash-tools.exec.ts, src/gateway/server-methods/approval-shared.ts, apps/ios/Sources/Model/NodeAppModel.swift)
  • jesse-merhi: Recent command authorization work touched exec approval planning paths adjacent to the host approval propagation this PR changes. (role: recent exec approval planner contributor; confidence: medium; commits: c9707ab635b9; files: src/gateway/server-methods/exec-approval.ts, src/agents/bash-tools.exec.ts)
  • pgondhi987: Merged PR history shows the requester-metadata approval binding feature that current reviewer-device visibility builds around. (role: approval requester metadata feature contributor; confidence: medium; commits: feaa1afd6db1, a153d988afc4, d6111b42792a; files: src/gateway/server-methods/approval-shared.ts, src/gateway/server-methods/exec-approval.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: 🐚 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 20, 2026
@openclaw-barnacle openclaw-barnacle Bot added the app: ios App: ios label Jun 20, 2026
@joshavant
joshavant marked this pull request as ready for review June 20, 2026 04:45
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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. labels Jun 20, 2026
@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. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 20, 2026
@joshavant
joshavant force-pushed the fix/mobile-exec-approval-targets branch from 85afa3b to ed761da Compare June 21, 2026 11:34
@joshavant
joshavant merged commit 5d1e649 into main Jun 21, 2026
168 of 169 checks passed
@joshavant
joshavant deleted the fix/mobile-exec-approval-targets branch June 21, 2026 13:47
@github-actions github-actions Bot mentioned this pull request Jun 21, 2026
Open
This was referenced Jun 21, 2026
Merged
Open
Merged
Open
Open
Open
Open
Closed
Closed
Closed
Open
Closed
Closed
Open
Closed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 22, 2026
@joshavant
fix: route mobile exec approvals to reviewer device (openclaw#95175)
674b6c7 
* fix: route mobile exec approvals to reviewer device

* fix: surface iOS approval events in foreground

* fix: forward codex approval reviewer device

* test: harden approval reviewer device contract

* test: cover reviewer approval fallback resolvers
@clawsweeper clawsweeper Bot mentioned this pull request Jun 27, 2026
Closed
@github-actions github-actions Bot mentioned this pull request Jul 1, 2026
Merged
This was referenced Jul 5, 2026
Merged
Closed
Merged
@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
Open
@evandrobarra evandrobarra mentioned this pull request Jul 17, 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

agents Agent runtime and tooling app: ios App: ios app: web-ui App: web-ui extensions: codex gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L 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.

1 participant

@joshavant