feat(gateway): persist operator approvals#103579
Conversation
openclaw-barnacle
Bot
added
docs
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Codex review: found issues before merge. Reviewed July 11, 2026, 7:46 PM ET / 23:46 UTC. Summary PR surface: Source +3523, Tests +3406, Docs +431, Generated +147, Other +502. Total +8009 across 39 files. Reproducibility: yes. for the patch defect: the exact-head implementation and included regression directly conflict on the return value of a successful persisted timeout. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Fix persisted timeout success reporting without changing its terminal state, then land the durable Gateway contract only after approval-path owners accept its authorization, restart, malformed-verdict, retention, and legacy-adapter semantics. Do we have a high-confidence way to reproduce the issue? Yes for the patch defect: the exact-head implementation and included regression directly conflict on the return value of a successful persisted timeout. Is this the best way to solve the issue? The Gateway-owned SQLite compare-and-set lifecycle with process-local executable bindings is the correct ownership boundary. The patch is not merge-ready until timeout success reporting is fixed and the permanent security contract is owner-approved. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2adaf8b3b5d0. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +3523, Tests +3406, Docs +431, Generated +147, Other +502. Total +8009 across 39 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
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
Review history (6 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
ff3afc4 to
8a86227
Compare
|
Land-ready update: rebased onto current main with the #103515 exec-revocation hardening deliberately reconciled, review findings fixed, and revalidated end to end. #103515 reconciliation (this PR's key delta): Review fixes since the last head: reviewer-binding E2E repaired (pre-rebase), audience walker cap now derived from the store cap, a resolve losing the CAS race reports Validation (fresh, on this head's stack):
Known gap: the two-device live E2E from the original evidence was not re-run on this exact head; the same suites run in CI. |
clawsweeper
Bot
removed
rating: 🦪 silver shellfish
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0770f80328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| <Accordion title="Approval families"> | ||
| - `exec.approval.request`, `exec.approval.get`, `exec.approval.list`, and `exec.approval.resolve` cover one-shot exec approval requests plus pending approval lookup/replay. | ||
| - `approval.get` and `approval.resolve` are the kind-agnostic durable approval methods (scope `operator.approvals`). `approval.get` returns a sanitized pending or retained terminal projection with a stable `urlPath`; `approval.resolve` accepts the canonical approval id, an explicit `kind`, and a decision, applies first-answer-wins resolution, and always returns the recorded canonical result. |
There was a problem hiding this comment.
For clients implementing the new durable approval API from this docs entry, including the documented kind field will make the request fail schema validation because ApprovalResolveParamsSchema only permits id and decision with additionalProperties: false (packages/gateway-protocol/src/schema/approvals.ts:211). The handler has already loaded the pending approval by then and treats invalid params as a malformed verdict (src/gateway/server-methods/approval.ts:503), so a docs-following client can accidentally deny the approval instead of resolving it. Please either document the actual two-field payload or add kind to the schema/handler contract.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
status: 👀 ready for maintainer look
Map #103515 semantics onto the durable lifecycle: resolutionSource and one-shot consumeAskFallback stay process-local record facts, trusted auto-review resolves through the durable CAS as a runtime resolver, and lost races settle with the winner's operator source. Also: derive the audience walker cap from the store cap, report APPROVAL_ALREADY_RESOLVED for a resolve that loses the CAS race, document approval.get/resolve in the protocol docs, and regenerate Swift models on the new base.
f36dbd8 to
b53f59b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b53f59be41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Method | Contract | | ||
| | ----------------------------------- | ------------------------------------------------------------------------------------------- | | ||
| | `approval.get { id }` | Returns a visible pending or retained terminal projection. | | ||
| | `approval.resolve { id, decision }` | Runs authorization, allowed-decision validation, deadline reconciliation, and terminal CAS. | |
There was a problem hiding this comment.
For clients using this public /refactor/operator-approvals page, the documented payload omits kind, but ApprovalResolveParamsSchema requires { id, kind, decision } (packages/gateway-protocol/src/schema/approvals.ts:212) and the handler treats invalid params for a known pending approval as forceMalformedDeny (src/gateway/server-methods/approval.ts:514). A docs-following approval.resolve call with only { id, decision } will consume the first-answer slot as a malformed-verdict denial instead of applying the intended decision, so include kind in this contract table.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…-ups Follow-up delta to the merged #103579 head, rebased onto current main: - gateway-protocol wire types derive from owner-module schema consts (types.ts tombstone) and ProtocolSchemas leaves the package index so the public plugin-sdk d.ts graph tree-shakes the registry declaration - approval access authority follows the operator.approvals scope tier with reviewerDeviceIds as the opt-in restriction (cross-surface first-answer-wins; requester identity gates only legacy adapters) - plugin node.invoke approvals register directly so unrenderable presentations fail closed before request routing - exec-approval manager reconciliation with #103515 revocation hardening (resolution source attribution, one-shot ask-fallback consumption) - surface-report pins and plugin-sdk API baseline refreshed; Swift models regenerated
…-ups Follow-up delta to the merged #103579 head, rebased onto current main: - gateway-protocol wire types derive from owner-module schema consts (types.ts tombstone) and ProtocolSchemas leaves the package index so the public plugin-sdk d.ts graph tree-shakes the registry declaration - approval access authority follows the operator.approvals scope tier with reviewerDeviceIds as the opt-in restriction (cross-surface first-answer-wins; requester identity gates only legacy adapters) - plugin node.invoke approvals register directly so unrenderable presentations fail closed before request routing - exec-approval manager reconciliation with #103515 revocation hardening (resolution source attribution, one-shot ask-fallback consumption) - surface-report pins and plugin-sdk API baseline refreshed; Swift models regenerated
…-ups Follow-up delta to the merged #103579 head, rebased onto current main: - gateway-protocol wire types derive from owner-module schema consts (types.ts tombstone) and ProtocolSchemas leaves the package index so the public plugin-sdk d.ts graph tree-shakes the registry declaration - approval access authority follows the operator.approvals scope tier with reviewerDeviceIds as the opt-in restriction (cross-surface first-answer-wins; requester identity gates only legacy adapters) - plugin node.invoke approvals register directly so unrenderable presentations fail closed before request routing - exec-approval manager reconciliation with #103515 revocation hardening (resolution source attribution, one-shot ask-fallback consumption) - surface-report pins and plugin-sdk API baseline refreshed; Swift models regenerated
* fix(gateway): approval registry hardening and protocol-surface follow-ups Follow-up delta to the merged #103579 head, rebased onto current main: - gateway-protocol wire types derive from owner-module schema consts (types.ts tombstone) and ProtocolSchemas leaves the package index so the public plugin-sdk d.ts graph tree-shakes the registry declaration - approval access authority follows the operator.approvals scope tier with reviewerDeviceIds as the opt-in restriction (cross-surface first-answer-wins; requester identity gates only legacy adapters) - plugin node.invoke approvals register directly so unrenderable presentations fail closed before request routing - exec-approval manager reconciliation with #103515 revocation hardening (resolution source attribution, one-shot ask-fallback consumption) - surface-report pins and plugin-sdk API baseline refreshed; Swift models regenerated * feat(channels): add typed operator approval actions Squash-rebased #103679 segment onto the durable-approval-registry tip on current main. Typed approval/command/select presentation actions replace raw-string inference across slack/telegram/discord/matrix/imessage/whatsapp, approval.resolve carries an explicit kind, and channel adapters map native callback envelopes through the typed action registry. Drift reconciliation: deprecated buildExecApprovalInteractiveReply assertions dropped (#104650 removed the shims); worker_environments bootstrap-column migration kept alongside the approval resolution_ref backfill; plugin-sdk API baseline regenerated. (cherry picked from commit 68765a5d39d2118c88a7a54d00387337912d4494) (cherry picked from commit 8642ac12af142e4b751f4f30d4b114615e7e5f66) (cherry picked from commit 036c4bc39499925fc03de16ec9302e346769350a) (cherry picked from commit 19dc350d6bc34e29a5169c6bc80971b0ad12adde) (cherry picked from commit fc978b0bad86aef421c79f6a211b25cc1b743c01) (cherry picked from commit 10de4d1ed5071f9be6ad1ee5d1e32c0fa8c9d11c) (cherry picked from commit 9a664ced1b1fa740172b258f355f1a82925ae41c) (cherry picked from commit c5ff69abbf444139e9e007bfa45beb0f00ffea54) (cherry picked from commit d466a80) (cherry picked from commit f5b4fe40dd5c961322f8553cc80b2fdfb3f6503e) (cherry picked from commit 7340b4749a4cc4c72f7a41cce1bc9cb550cae038) (cherry picked from commit a151f41808f23ae60b10305ccd2bc959b9169a86) * fix(approvals): preserve typed transport ownership * test(imessage): narrow chunked approval text * refactor(protocol): remove retired type tombstone * fix(plugin-sdk): align surface budgets after rebase * docs(changelog): note typed operator approvals * docs(changelog): defer typed approval release note
* feat(gateway): persist operator approvals * fix(gateway): preserve approval ids exactly * fix(gateway): enforce approval reviewer bindings * fix(gateway): reconcile durable approvals with exec revocation hardening Map openclaw#103515 semantics onto the durable lifecycle: resolutionSource and one-shot consumeAskFallback stay process-local record facts, trusted auto-review resolves through the durable CAS as a runtime resolver, and lost races settle with the winner's operator source. Also: derive the audience walker cap from the store cap, report APPROVAL_ALREADY_RESOLVED for a resolve that loses the CAS race, document approval.get/resolve in the protocol docs, and regenerate Swift models on the new base. * fix(approvals): validate resolver kind * docs(approvals): explain malformed verdict denial * fix(protocol): regenerate approval models after rebase * chore: leave changelog entry to release generation * fix(gateway): preserve approval registration boundaries * test(gateway): prove multi-device approval races * test(gateway): keep approval order assertion stable * docs: index operator approval architecture * fix(protocol): bound approval declaration exports
…3679) * fix(gateway): approval registry hardening and protocol-surface follow-ups Follow-up delta to the merged openclaw#103579 head, rebased onto current main: - gateway-protocol wire types derive from owner-module schema consts (types.ts tombstone) and ProtocolSchemas leaves the package index so the public plugin-sdk d.ts graph tree-shakes the registry declaration - approval access authority follows the operator.approvals scope tier with reviewerDeviceIds as the opt-in restriction (cross-surface first-answer-wins; requester identity gates only legacy adapters) - plugin node.invoke approvals register directly so unrenderable presentations fail closed before request routing - exec-approval manager reconciliation with openclaw#103515 revocation hardening (resolution source attribution, one-shot ask-fallback consumption) - surface-report pins and plugin-sdk API baseline refreshed; Swift models regenerated * feat(channels): add typed operator approval actions Squash-rebased openclaw#103679 segment onto the durable-approval-registry tip on current main. Typed approval/command/select presentation actions replace raw-string inference across slack/telegram/discord/matrix/imessage/whatsapp, approval.resolve carries an explicit kind, and channel adapters map native callback envelopes through the typed action registry. Drift reconciliation: deprecated buildExecApprovalInteractiveReply assertions dropped (openclaw#104650 removed the shims); worker_environments bootstrap-column migration kept alongside the approval resolution_ref backfill; plugin-sdk API baseline regenerated. (cherry picked from commit 68765a5d39d2118c88a7a54d00387337912d4494) (cherry picked from commit 8642ac12af142e4b751f4f30d4b114615e7e5f66) (cherry picked from commit 036c4bc39499925fc03de16ec9302e346769350a) (cherry picked from commit 19dc350d6bc34e29a5169c6bc80971b0ad12adde) (cherry picked from commit fc978b0bad86aef421c79f6a211b25cc1b743c01) (cherry picked from commit 10de4d1ed5071f9be6ad1ee5d1e32c0fa8c9d11c) (cherry picked from commit 9a664ced1b1fa740172b258f355f1a82925ae41c) (cherry picked from commit c5ff69abbf444139e9e007bfa45beb0f00ffea54) (cherry picked from commit d466a80) (cherry picked from commit f5b4fe40dd5c961322f8553cc80b2fdfb3f6503e) (cherry picked from commit 7340b4749a4cc4c72f7a41cce1bc9cb550cae038) (cherry picked from commit a151f41808f23ae60b10305ccd2bc959b9169a86) * fix(approvals): preserve typed transport ownership * test(imessage): narrow chunked approval text * refactor(protocol): remove retired type tombstone * fix(plugin-sdk): align surface budgets after rebase * docs(changelog): note typed operator approvals * docs(changelog): defer typed approval release note
1 participant
Related: #103505
What Problem This Solves
Operator approvals currently depend on process-local waiter state. Authorized surfaces cannot reliably reopen the same pending request, race to one canonical decision, or recover the recorded winner after reconnects and Gateway restarts.
Why This Change Was Made
This first increment establishes the Gateway-owned approval lifecycle: sanitized reviewer presentations in the shared SQLite state database, atomic first-answer-wins resolution, retained terminal records, generic
approval.getandapproval.resolvemethods, and a persisted source/ancestor audience snapshot. Raw exec/plugin requests and executable bindings remain process-local and are never rehydrated from SQLite.The accompanying design note maps the current Gateway, agent, channel, Control UI, Telegram, native, and plugin-SDK paths and defines the stacked UI/actions and ancestor-event follow-ups.
User Impact
This is the durable foundation; it does not yet add the standalone Control UI page or change channel rendering. Existing exec/plugin approval request flows keep their current compatibility behavior while gaining one canonical durable lifecycle behind them.
Evidence
tbx_01kx5exst4xv8cdjg8nhzthykn(violet-barnacle): full changed gate passed, including core/test typechecks, core lint, app lane, macOS app CI test shards, generated-state checks, import-cycle checks, and sidecar/storage policy checks.tsgotypechecks plus targetedoxlint --deny-warningspassed.AnyCodable.swift; generated approval models compiled before that point. Authoritative macOS CI remains the platform gate.