diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 81082ce057ab..5e0d3601e916 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -107,6 +107,26 @@ public enum ApprovalAllowDecision: String, Codable, Sendable { case allowAlways = "allow-always" } +public enum ApprovalAllowedReason: String, Codable, Sendable { + case user = "user" +} + +public enum ApprovalDeniedReason: String, Codable, Sendable { + case user = "user" + case malformedVerdict = "malformed-verdict" + case noRoute = "no-route" + case storageCorrupt = "storage-corrupt" +} + +public enum ApprovalExpiredReason: String, Codable, Sendable { + case timeout = "timeout" +} + +public enum ApprovalCancelledReason: String, Codable, Sendable { + case runAborted = "run-aborted" + case gatewayRestart = "gateway-restart" +} + public enum PluginApprovalSeverity: String, Codable, Sendable { case info = "info" case warning = "warning" @@ -3836,18 +3856,22 @@ public struct SessionsSendParams: Codable, Sendable { public struct SessionsMessagesSubscribeParams: Codable, Sendable { public let key: String public let agentid: String? + public let includeapprovals: Bool? public init( key: String, - agentid: String? = nil) + agentid: String? = nil, + includeapprovals: Bool? = nil) { self.key = key self.agentid = agentid + self.includeapprovals = includeapprovals } private enum CodingKeys: String, CodingKey { case key case agentid = "agentId" + case includeapprovals = "includeApprovals" } } @@ -9936,9 +9960,9 @@ public struct AllowedApprovalSnapshot: Codable, Sendable { public let expiresatms: Int public let presentation: ApprovalPresentation public let resolvedatms: Int - public let reason: ApprovalTerminalReason public let status: String public let decision: ApprovalAllowDecision + public let reason: ApprovalAllowedReason public init( id: String, @@ -9947,9 +9971,9 @@ public struct AllowedApprovalSnapshot: Codable, Sendable { expiresatms: Int, presentation: ApprovalPresentation, resolvedatms: Int, - reason: ApprovalTerminalReason, status: String, - decision: ApprovalAllowDecision) + decision: ApprovalAllowDecision, + reason: ApprovalAllowedReason) { self.id = id self.urlpath = urlpath @@ -9957,9 +9981,9 @@ public struct AllowedApprovalSnapshot: Codable, Sendable { self.expiresatms = expiresatms self.presentation = presentation self.resolvedatms = resolvedatms - self.reason = reason self.status = status self.decision = decision + self.reason = reason } private enum CodingKeys: String, CodingKey { @@ -9969,9 +9993,9 @@ public struct AllowedApprovalSnapshot: Codable, Sendable { case expiresatms = "expiresAtMs" case presentation case resolvedatms = "resolvedAtMs" - case reason case status case decision + case reason } } @@ -9982,9 +10006,9 @@ public struct DeniedApprovalSnapshot: Codable, Sendable { public let expiresatms: Int public let presentation: ApprovalPresentation public let resolvedatms: Int - public let reason: ApprovalTerminalReason public let status: String public let decision: String + public let reason: ApprovalDeniedReason public init( id: String, @@ -9993,9 +10017,9 @@ public struct DeniedApprovalSnapshot: Codable, Sendable { expiresatms: Int, presentation: ApprovalPresentation, resolvedatms: Int, - reason: ApprovalTerminalReason, status: String, - decision: String) + decision: String, + reason: ApprovalDeniedReason) { self.id = id self.urlpath = urlpath @@ -10003,9 +10027,9 @@ public struct DeniedApprovalSnapshot: Codable, Sendable { self.expiresatms = expiresatms self.presentation = presentation self.resolvedatms = resolvedatms - self.reason = reason self.status = status self.decision = decision + self.reason = reason } private enum CodingKeys: String, CodingKey { @@ -10015,9 +10039,9 @@ public struct DeniedApprovalSnapshot: Codable, Sendable { case expiresatms = "expiresAtMs" case presentation case resolvedatms = "resolvedAtMs" - case reason case status case decision + case reason } } @@ -10028,8 +10052,8 @@ public struct ExpiredApprovalSnapshot: Codable, Sendable { public let expiresatms: Int public let presentation: ApprovalPresentation public let resolvedatms: Int - public let reason: ApprovalTerminalReason public let status: String + public let reason: ApprovalExpiredReason public init( id: String, @@ -10038,8 +10062,8 @@ public struct ExpiredApprovalSnapshot: Codable, Sendable { expiresatms: Int, presentation: ApprovalPresentation, resolvedatms: Int, - reason: ApprovalTerminalReason, - status: String) + status: String, + reason: ApprovalExpiredReason) { self.id = id self.urlpath = urlpath @@ -10047,8 +10071,8 @@ public struct ExpiredApprovalSnapshot: Codable, Sendable { self.expiresatms = expiresatms self.presentation = presentation self.resolvedatms = resolvedatms - self.reason = reason self.status = status + self.reason = reason } private enum CodingKeys: String, CodingKey { @@ -10058,8 +10082,8 @@ public struct ExpiredApprovalSnapshot: Codable, Sendable { case expiresatms = "expiresAtMs" case presentation case resolvedatms = "resolvedAtMs" - case reason case status + case reason } } @@ -10070,8 +10094,8 @@ public struct CancelledApprovalSnapshot: Codable, Sendable { public let expiresatms: Int public let presentation: ApprovalPresentation public let resolvedatms: Int - public let reason: ApprovalTerminalReason public let status: String + public let reason: ApprovalCancelledReason public init( id: String, @@ -10080,8 +10104,8 @@ public struct CancelledApprovalSnapshot: Codable, Sendable { expiresatms: Int, presentation: ApprovalPresentation, resolvedatms: Int, - reason: ApprovalTerminalReason, - status: String) + status: String, + reason: ApprovalCancelledReason) { self.id = id self.urlpath = urlpath @@ -10089,8 +10113,8 @@ public struct CancelledApprovalSnapshot: Codable, Sendable { self.expiresatms = expiresatms self.presentation = presentation self.resolvedatms = resolvedatms - self.reason = reason self.status = status + self.reason = reason } private enum CodingKeys: String, CodingKey { @@ -10100,8 +10124,8 @@ public struct CancelledApprovalSnapshot: Codable, Sendable { case expiresatms = "expiresAtMs" case presentation case resolvedatms = "resolvedAtMs" - case reason case status + case reason } } @@ -10173,6 +10197,92 @@ public struct ApprovalResolveResult: Codable, Sendable { } } +public struct PendingSessionApprovalEvent: Codable, Sendable { + public let sessionkey: String + public let sourcesessionkey: String? + public let updatedatms: Int + public let phase: String + public let approval: PendingApprovalSnapshot + + public init( + sessionkey: String, + sourcesessionkey: String? = nil, + updatedatms: Int, + phase: String, + approval: PendingApprovalSnapshot) + { + self.sessionkey = sessionkey + self.sourcesessionkey = sourcesessionkey + self.updatedatms = updatedatms + self.phase = phase + self.approval = approval + } + + private enum CodingKeys: String, CodingKey { + case sessionkey = "sessionKey" + case sourcesessionkey = "sourceSessionKey" + case updatedatms = "updatedAtMs" + case phase + case approval + } +} + +public struct TerminalSessionApprovalEvent: Codable, Sendable { + public let sessionkey: String + public let sourcesessionkey: String? + public let updatedatms: Int + public let phase: String + public let approval: TerminalApprovalSnapshot + + public init( + sessionkey: String, + sourcesessionkey: String? = nil, + updatedatms: Int, + phase: String, + approval: TerminalApprovalSnapshot) + { + self.sessionkey = sessionkey + self.sourcesessionkey = sourcesessionkey + self.updatedatms = updatedatms + self.phase = phase + self.approval = approval + } + + private enum CodingKeys: String, CodingKey { + case sessionkey = "sessionKey" + case sourcesessionkey = "sourceSessionKey" + case updatedatms = "updatedAtMs" + case phase + case approval + } +} + +public struct SessionApprovalReplay: Codable, Sendable { + public let sessionkey: String + public let updatedatms: Int + public let approvals: [PendingApprovalSnapshot] + public let truncated: Bool + + public init( + sessionkey: String, + updatedatms: Int, + approvals: [PendingApprovalSnapshot], + truncated: Bool) + { + self.sessionkey = sessionkey + self.updatedatms = updatedatms + self.approvals = approvals + self.truncated = truncated + } + + private enum CodingKeys: String, CodingKey { + case sessionkey = "sessionKey" + case updatedatms = "updatedAtMs" + case approvals + case truncated + } +} + public struct ExecApprovalsGetParams: Codable, Sendable {} public struct ExecApprovalsSetParams: Codable, Sendable { @@ -12153,6 +12263,37 @@ public enum TerminalApprovalSnapshot: Codable, Sendable { } } +public enum SessionApprovalEvent: Codable, Sendable { + case pending(PendingSessionApprovalEvent) + case terminal(TerminalSessionApprovalEvent) + + private enum CodingKeys: String, CodingKey { + case discriminator = "phase" + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + let discriminator = try container.decode(String.self, forKey: .discriminator) + switch discriminator { + case "pending": self = try .pending(PendingSessionApprovalEvent(from: decoder)) + case "terminal": self = try .terminal(TerminalSessionApprovalEvent(from: decoder)) + default: + throw DecodingError.dataCorruptedError( + forKey: .discriminator, + in: container, + debugDescription: "Unknown SessionApprovalEvent discriminator value" + ) + } + } + + public func encode(to encoder: Encoder) throws { + switch self { + case .pending(let value): try value.encode(to: encoder) + case .terminal(let value): try value.encode(to: encoder) + } + } +} + public enum PluginCatalogInstallAction: Codable, Sendable { case clawhub(PluginCatalogClawHubInstall) case official(PluginCatalogOfficialInstall) diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index a9872caace54..d8d4ab046bbd 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -28669b8725f8641f4e3e1d00f1c120a78e20d8f3c7ed2055af77a58d2bd21c7d plugin-sdk-api-baseline.json -d8bba86cc315f64d977d1b0f770e203b4e3531bbd2bc72aa7a834be154e5fd94 plugin-sdk-api-baseline.jsonl +634a2cda48d53c9a234d59200b4666c105615da4d64612ea30facb42df9c4d93 plugin-sdk-api-baseline.json +f5bc2ec06dbdc26736f8d810d75f72f378ffd2111c2a143ed153a284b6d89d25 plugin-sdk-api-baseline.jsonl diff --git a/docs/gateway/index.md b/docs/gateway/index.md index 2948500a8fdd..2ecf2b8e2f39 100644 --- a/docs/gateway/index.md +++ b/docs/gateway/index.md @@ -304,9 +304,9 @@ Defaults include isolated state/config and base gateway port `19001`. a generated dump of every callable helper route. - Requests: `req(method, params)` → `res(ok/payload|error)`. - Common events include `connect.challenge`, `agent`, `chat`, - `session.message`, `session.operation`, `session.tool`, `sessions.changed`, - `presence`, `tick`, `health`, `heartbeat`, pairing/approval lifecycle events, - and `shutdown`. + `session.message`, `session.operation`, `session.tool`, opt-in + `session.approval`, `sessions.changed`, `presence`, `tick`, `health`, + `heartbeat`, pairing/approval lifecycle events, and `shutdown`. Agent runs are two-stage: diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md index 215be4d52efc..b8e33dd07c04 100644 --- a/docs/gateway/protocol.md +++ b/docs/gateway/protocol.md @@ -507,7 +507,7 @@ methods. Treat this as feature discovery, not a full enumeration of - `sessions.list` returns the current session index, including per-row `agentRuntime` metadata when an agent runtime backend is configured. - `sessions.subscribe` and `sessions.unsubscribe` toggle session change event subscriptions for the current WS client. - - `sessions.messages.subscribe` and `sessions.messages.unsubscribe` toggle transcript/message event subscriptions for one session. + - `sessions.messages.subscribe` and `sessions.messages.unsubscribe` toggle transcript/message event subscriptions for one session. Pass `includeApprovals: true` to also receive sanitized `session.approval` lifecycle events for approvals whose persisted audience includes that exact session and whose reviewer binding authorizes the subscribing client. The subscribe response then includes a bounded pending `approvalReplay`; it is authoritative when `truncated` is false. The opt-in is per subscribe call, not sticky: re-subscribing to the same session without `includeApprovals: true` removes an existing approval subscription. In addition to normal session-read authority, this opt-in requires `operator.admin`, or `operator.approvals` on a paired device. - `sessions.preview` returns bounded transcript previews for specific session keys. - `sessions.describe` returns one gateway session row for an exact session key. - `sessions.resolve` resolves or canonicalizes a session target. @@ -580,6 +580,9 @@ methods. Treat this as feature discovery, not a full enumeration of `replace=true` and use `deltaText` as the replacement text. - `session.message`, `session.operation`, `session.tool`: transcript, in-flight session operation, and event-stream updates for a subscribed session. +- `session.approval`: sanitized pending and terminal approval truth for an + explicitly opted-in exact-session subscriber. Child approvals use the + persisted ancestor audience; events never mutate transcripts or wake agents. - `sessions.changed`: session index or metadata changed. - `presence`: system presence snapshot updates. - `tick`: periodic keepalive/liveness event. diff --git a/docs/refactor/operator-approvals.md b/docs/refactor/operator-approvals.md index 52ca5f6901a0..b4fa92e07454 100644 --- a/docs/refactor/operator-approvals.md +++ b/docs/refactor/operator-approvals.md @@ -35,16 +35,18 @@ Inline actions and deep links coexist. There is no approval-mode toggle. - Redesigning exec allowlists, plugin policy composition, or `allow-always` persistence except where required to make terminal outcomes unambiguous. - Making a gatewayless embedded TUI remotely reachable in the first increment. It remains local-only and must fail closed when no reviewer exists. -## Existing system and evidence map +## Pre-rollout baseline and evidence map -| Surface | Current entry point and owner | Current behavior and gap | +This table records the implementation state when #103505 was opened. The rollout sections below track the durable registry, typed actions, deep-link page, and native-client increments built on top of that baseline. + +| Surface | Baseline entry point and owner | Baseline behavior and gap | | ----------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Agent exec | `src/agents/bash-tools.exec-approval-request.ts`, `src/agents/bash-tools.exec-host-shared.ts` | Two-phase `exec.approval.*` registration prevents an early `/approve` race, but timeout can still become allow through `askFallback`. | | Plugin tool gate | `src/agents/agent-tools.before-tool-call.ts` | Requests `plugin.approval.*`; `timeoutBehavior: "allow"` can approve a timed-out gate. Embedded mode has separate process-local authority in `src/infra/embedded-plugin-approval-broker.ts`. | | Plugin node gate | `src/gateway/node-invoke-plugin-policy.ts` | Creates and broadcasts directly through the plugin manager, duplicating part of the server-method lifecycle. | | Gateway authority | `src/gateway/server-aux-handlers.ts`, `src/gateway/exec-approval-manager.ts`, `src/gateway/server-methods/approval-shared.ts` | Separate exec and plugin managers use process-local maps. Terminal entries survive for 15 seconds. First-answer-wins holds only inside one process. | | Gateway protocol | `packages/gateway-protocol/src/schema/exec-approvals.ts`, `packages/gateway-protocol/src/schema/plugin-approvals.ts`, `src/gateway/methods/core-descriptors.ts` | Exec has pending-only `get`; plugin has no `get`; no kind-agnostic terminal lookup exists for a deep link. | -| Delivery | `src/infra/exec-approval-channel-runtime.ts`, `src/infra/approval-native-runtime.ts`, `src/infra/approval-handler-runtime.ts` | Supports origin routing, approver DMs, pending replay, native handlers, and in-process terminal cleanup. PR 5 adds durable terminal reconciliation. | +| Delivery | `src/infra/exec-approval-channel-runtime.ts`, `src/infra/approval-native-runtime.ts`, `src/infra/approval-handler-runtime.ts` | Supports origin routing, approver DMs, pending replay, native handlers, and in-process terminal cleanup. A separate follow-up adds durable terminal reconciliation. | | Portable actions | `src/interactive/payload.ts`, `src/plugin-sdk/interactive-runtime.ts`, `src/plugin-sdk/approval-reply-runtime.ts` | Approval buttons are command actions containing `/approve ...`; URL and Web App targets are untyped button fields. | | Telegram | `extensions/telegram/src/approval-handler.runtime.ts`, `extensions/telegram/src/button-types.ts` | The renderer parses command text to recognize approval semantics before producing private callback data. | | Control UI | `ui/src/app/exec-approval.ts`, `ui/src/app/overlays.ts`, `ui/src/components/exec-approval.ts` | Approval UI is a global modal. `ui/src/app-route-paths.ts` and `ui/src/app-routes.ts` use exact routes and rewrite unknown paths to Chat. | @@ -200,10 +202,12 @@ Add an approval-scoped `session.approval` projection event. Publish the canonica - `sessionKey`: stream receiving the projection. - `sourceSessionKey`: child/source that raised the gate. -- `phase`: `requested \| terminal`. +- `phase`: `pending \| terminal`, discriminated against the approval status. - one safe `OperatorApproval` projection. -Register the event under `operator.approvals` in `src/gateway/server-broadcast.ts`. Session subscription alone never grants approval visibility. +Clients opt in with `sessions.messages.subscribe { key, agentId?, includeApprovals: true }`. The successful response adds an `approvalReplay` containing up to 1,000 current pending approvals for that exact stream key that the subscribing client is also record-authorized to review. `truncated: false` makes the filtered replay authoritative and reconnecting clients replace their local pending set with it; `truncated: true` is an overload signal and clients must keep unseen local entries until canonical lookup or later lifecycle events settle them. A later durable timeout discovered during replay emits terminal tombstones only to subscribed, record-authorized audiences before the new snapshot is returned. `operator.admin` may opt in directly; narrower clients require both a paired device identity and `operator.approvals`. Session subscription alone never grants approval visibility. + +Register the event under `operator.approvals` in `src/gateway/server-broadcast.ts`. The projection is observational: it never appends transcript rows, emits `sessions.changed`, or wakes an agent. Extend `MessagePresentationAction` in `src/interactive/payload.ts`: @@ -290,13 +294,13 @@ Use a deterministic breadth-first walk: The registry source is `src/agents/subagent-registry-read.ts`; ownership fields are defined in `src/agents/subagent-registry.types.ts`. Session fallback fields are defined in `src/config/sessions/types.ts`. -Requested and terminal projections use the same persisted audience even if focus/controller ownership changes while the approval is pending. This guarantees that every surface that displayed the request receives terminal cleanup. Resolution always targets the source approval ID; audience sessions never receive cloned approval state. +Requested and terminal projections use the same persisted audience even if focus/controller ownership changes while the approval is pending. This guarantees terminal cleanup for every audience session stream that received the request projection. Resolution always targets the source approval ID; audience sessions never receive cloned approval state. Forwarded channel-message cleanup remains the separate delivery-locator follow-up below. Do not write transcript messages, inject system prompts, start owner turns, or emit `sessions.changed` solely for an approval. ## Delivered-surface convergence -Native approval handlers already retain their delivered message entries long enough to replace or retire active controls. Generic forwarded approval messages currently discard the `MessageReceipt`, so a decision on another surface can leave their old controls looking pending. PR 5 closes that gap with an `operator_approval_deliveries` child table in the shared state database. +Native approval handlers already retain their delivered message entries long enough to replace or retire active controls. Generic forwarded approval messages currently discard the `MessageReceipt`, so a decision on another surface can leave their old controls looking pending. A separate follow-up closes that gap with an `operator_approval_deliveries` child table in the shared state database. Each row stores the approval ID, a unique delivery ID, channel/account/exact route, a bounded JSON-validated channel-private message locator, delivery timestamps, and terminalization state. It never stores callback data, decision tokens, or raw approval requests. The channel owns locator encoding and message mutation; core owns canonical status, target selection, retry policy, and fallback terminal text. @@ -369,7 +373,7 @@ Do not silently change them in the storage PR. The strict-semantics PR must upda - Transport-private callback encoding with explicit owner kind. - Durable fixed-size callback references for canonical IDs beyond transport limits. - Bundled channel migration away from command-text and approval-ID inference. -- Canonical first-answer truth on the clicked surface and best-effort active-native terminal updates; durable reconciliation stays in PR 5. +- Canonical first-answer truth on the clicked surface and best-effort active-native terminal updates; durable channel-message terminalization remains a follow-up. - SDK and bundled-channel tests. ### PR 3: Control UI deep link @@ -382,17 +386,32 @@ Do not silently change them in the storage PR. The strict-semantics PR must upda ### PR 4: native clients -- iOS, watchOS, and Android review surfaces use kind-aware `approval.get/resolve`. +- iOS and Android review surfaces use kind-aware `approval.get/resolve`; watchOS relays reviewer-safe prompts and decisions through the paired iPhone. +- Watch offers the exec decisions supported by its compact relay contract: allow once and deny. - Canonical first-answer terminal truth replaces local attempted-decision state. +- Lost or ambiguous resolve acknowledgements freeze controls until canonical readback. +- Previous shipped Gateway v4 instances retain exec review through a narrow legacy-method fallback; retained cross-surface terminal state requires the unified methods. +- Reviewer warnings and owner context remain visible across iPhone, Watch, and Android. - Native unit, build, and platform proof. -### PR 5: propagation and fail-closed behavior +### PR 5: ancestor lifecycle propagation + +- `session.approval` pending/terminal delivery from the audience snapshot persisted in PR 1. +- Exact-session subscription, reconnect replay, and terminal tombstones without transcript mutation or agent wake. +- Lifecycle callbacks run after durable insert/CAS and never become approval authority. +- Nested-subagent and reconnect proof. + +### PR 6: fail-closed behavior -- `session.approval` request/terminal delivery from the audience snapshot persisted in PR 1. -- Durable forwarded-delivery locators and canonical terminal cleanup across every delivered surface, including restart replay. - Migrate `node-invoke-plugin-policy.ts` and the embedded plugin broker away from duplicate authority. -- Strict timeout/malformed/no-route semantics and compatibility docs. -- Multi-surface and nested-subagent end-to-end proof. +- Strict timeout, malformed, no-route, binding, and allow-once consumption semantics. +- Deprecate shipped permissive timeout settings without honoring them after an ask is pending. +- Multi-surface contention and failure-injection proof. + +### Follow-up: durable remote-message cleanup + +- Persist forwarded-delivery locators and terminalize every delivered channel message after restart. +- Keep this transport lifecycle separate from canonical approval authority and typed presentation actions. ## Tests @@ -415,7 +434,11 @@ Required focused coverage: - Owner projections cause no transcript mutation or agent wake. - Control UI route works at `/` and a configured base path; refresh shows pending or terminal truth. - Simultaneous Control UI and Telegram answers show one winner and "resolved elsewhere" on the loser. -- User-path proof through Testbox/Crabbox, including a mobile-width approval page and Telegram action cleanup. +- Native approval identifiers and Gateway owner identifiers preserve exact UTF-8 bytes across routing and reconciliation. +- Native RPC-family negotiation pins one canonical or legacy family per admitted Gateway route and never silently downgrades after use. +- Lost native resolve acknowledgements freeze actions until canonical readback; failed readback cannot fabricate a winner or acknowledge a Watch refresh. +- Watch snapshot request correlation is accepted only for the exact paired Gateway owner and a completed canonical iPhone readback. +- User-path proof through Testbox/Crabbox, including a mobile-width approval page, Telegram action cleanup, and one pending/resolve/late-loser round trip across Android, iPhone, and Watch. ## Observability @@ -432,10 +455,10 @@ Track: - startup-orphan cancellations; - audience size. -A committed transition is success even if later event delivery fails. Delivery failure is logged separately; PR 5 repairs remote state through durable delivery replay and canonical lookup. +A committed transition is success even if later event delivery fails. Lifecycle subscribers recover through PR 5 replay and canonical lookup. Durable channel-message terminalization remains the separate follow-up above. ## Open decisions 1. **Externally reachable Control UI origin.** Every snapshot carries the stable relative `urlPath`. An absolute URL may be advertised only from a cached Tailscale Serve/Funnel location after Gateway exposure succeeds; `allowedOrigins`, request Host headers, `gateway.remote.url`, and display-only loopback/LAN candidates are not canonical origins. Telegram can use its authenticated Mini App wrapper to retain the approval path through bootstrap. Arbitrary reverse proxies remain relative-only until a separately reviewed explicit public-URL contract exists. Never let a channel guess the origin. -2. **Strict timeout compatibility cutover.** The target is fail-closed, but `askFallback` and plugin `timeoutBehavior: "allow"` are shipped contracts. Recommended: make the behavior change in PR 5 with explicit owner/security approval, changelog, docs, and a migration/deprecation decision rather than hiding it in PR 1. +2. **Strict timeout compatibility cutover.** The target is fail-closed, but `askFallback` and plugin `timeoutBehavior: "allow"` are shipped contracts. Recommended: make the behavior change in PR 6 with explicit owner/security approval, changelog, docs, and a migration/deprecation decision rather than hiding it in PR 1. 3. **Gatewayless embedded mode.** Recommended: keep it local-only initially, then make it a client of the canonical service when a Gateway exists. Do not advertise a deep link that no server can resolve. diff --git a/packages/gateway-protocol/src/index.ts b/packages/gateway-protocol/src/index.ts index 3d4c2b69884d..7c8887f57e41 100644 --- a/packages/gateway-protocol/src/index.ts +++ b/packages/gateway-protocol/src/index.ts @@ -302,6 +302,10 @@ import { ApprovalResolveResultSchema, type ApprovalSnapshot, ApprovalSnapshotSchema, + type SessionApprovalEvent, + SessionApprovalEventSchema, + type SessionApprovalReplay, + SessionApprovalReplaySchema, type ApprovalTerminalReason, ApprovalTerminalReasonSchema, type CancelledApprovalSnapshot, @@ -1934,6 +1938,8 @@ export { ApprovalGetResultSchema, ApprovalResolveParamsSchema, ApprovalResolveResultSchema, + SessionApprovalEventSchema, + SessionApprovalReplaySchema, ExecApprovalsGetParamsSchema, ExecApprovalsSetParamsSchema, ExecApprovalGetParamsSchema, @@ -2289,6 +2295,8 @@ export type { ApprovalGetResult, ApprovalResolveParams, ApprovalResolveResult, + SessionApprovalEvent, + SessionApprovalReplay, ExecApprovalsGetParams, ExecApprovalsNodeSnapshot, ExecApprovalsSetParams, diff --git a/packages/gateway-protocol/src/native-protocol-levels.guard.test.ts b/packages/gateway-protocol/src/native-protocol-levels.guard.test.ts index 5ff840ed940d..f34a8714eacc 100644 --- a/packages/gateway-protocol/src/native-protocol-levels.guard.test.ts +++ b/packages/gateway-protocol/src/native-protocol-levels.guard.test.ts @@ -79,7 +79,7 @@ function stringLiteralUnionValues(schema: unknown): string[] | undefined { } const candidate = schema as { anyOf?: unknown; oneOf?: unknown }; const branches = candidate.oneOf ?? candidate.anyOf; - if (!Array.isArray(branches) || branches.length < 2) { + if (!Array.isArray(branches) || branches.length === 0) { return undefined; } @@ -245,4 +245,29 @@ describe("native Gateway protocol levels", () => { } } }); + + it("emits the session approval event as a discriminated Swift union", async () => { + const swiftGeneratedPath = + "apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift"; + const swiftGenerated = await readRepoFile(swiftGeneratedPath); + + assertPattern( + swiftGenerated, + swiftGeneratedPath, + /public enum SessionApprovalEvent: Codable, Sendable \{/, + "missing the generated SessionApprovalEvent union.", + ); + assertPattern( + swiftGenerated, + swiftGeneratedPath, + /case pending\(PendingSessionApprovalEvent\)/, + "SessionApprovalEvent must decode pending transitions.", + ); + assertPattern( + swiftGenerated, + swiftGeneratedPath, + /case terminal\(TerminalSessionApprovalEvent\)/, + "SessionApprovalEvent must decode terminal transitions.", + ); + }); }); diff --git a/packages/gateway-protocol/src/schema/approvals.ts b/packages/gateway-protocol/src/schema/approvals.ts index 681db85b5d35..0e51641fb3d7 100644 --- a/packages/gateway-protocol/src/schema/approvals.ts +++ b/packages/gateway-protocol/src/schema/approvals.ts @@ -39,6 +39,26 @@ export const ApprovalTerminalReasonSchema = Type.Union([ Type.Literal("storage-corrupt"), ]); +/** Terminal reason accepted for an allowed approval. */ +export const ApprovalAllowedReasonSchema = Type.Union([Type.Literal("user")]); + +/** Terminal reasons accepted for a denied approval. */ +export const ApprovalDeniedReasonSchema = Type.Union([ + Type.Literal("user"), + Type.Literal("malformed-verdict"), + Type.Literal("no-route"), + Type.Literal("storage-corrupt"), +]); + +/** Terminal reason accepted for an expired approval. */ +export const ApprovalExpiredReasonSchema = Type.Union([Type.Literal("timeout")]); + +/** Terminal reasons accepted for a cancelled approval. */ +export const ApprovalCancelledReasonSchema = Type.Union([ + Type.Literal("run-aborted"), + Type.Literal("gateway-restart"), +]); + /** Reviewer-facing severity for plugin-owned approval requests. */ export const PluginApprovalSeveritySchema = Type.Union([ Type.Literal("info"), @@ -105,7 +125,6 @@ const ApprovalRecordCommonFields = { const ApprovalResolutionFields = { resolvedAtMs: Type.Integer({ minimum: 0 }), - reason: ApprovalTerminalReasonSchema, }; /** Approval that has not yet accepted a reviewer decision. */ @@ -121,6 +140,7 @@ export const AllowedApprovalSnapshotSchema = Type.Object( ...ApprovalResolutionFields, status: Type.Literal("allowed"), decision: ApprovalAllowDecisionSchema, + reason: ApprovalAllowedReasonSchema, }, { additionalProperties: false }, ); @@ -132,6 +152,7 @@ export const DeniedApprovalSnapshotSchema = Type.Object( ...ApprovalResolutionFields, status: Type.Literal("denied"), decision: Type.Literal("deny"), + reason: ApprovalDeniedReasonSchema, }, { additionalProperties: false }, ); @@ -142,6 +163,7 @@ export const ExpiredApprovalSnapshotSchema = Type.Object( ...ApprovalRecordCommonFields, ...ApprovalResolutionFields, status: Type.Literal("expired"), + reason: ApprovalExpiredReasonSchema, }, { additionalProperties: false }, ); @@ -152,6 +174,7 @@ export const CancelledApprovalSnapshotSchema = Type.Object( ...ApprovalRecordCommonFields, ...ApprovalResolutionFields, status: Type.Literal("cancelled"), + reason: ApprovalCancelledReasonSchema, }, { additionalProperties: false }, ); @@ -204,6 +227,49 @@ export const ApprovalResolveResultSchema = Type.Object( { additionalProperties: false }, ); +const SessionApprovalEventCommonFields = { + sessionKey: NonEmptyString, + sourceSessionKey: Type.Optional(NonEmptyString), + updatedAtMs: Type.Integer({ minimum: 0 }), +}; + +/** Sanitized pending transition delivered only to an opted-in session audience. */ +export const PendingSessionApprovalEventSchema = Type.Object( + { + ...SessionApprovalEventCommonFields, + phase: Type.Literal("pending"), + approval: PendingApprovalSnapshotSchema, + }, + { additionalProperties: false }, +); + +/** Sanitized terminal transition delivered only to an opted-in session audience. */ +export const TerminalSessionApprovalEventSchema = Type.Object( + { + ...SessionApprovalEventCommonFields, + phase: Type.Literal("terminal"), + approval: TerminalApprovalSnapshotSchema, + }, + { additionalProperties: false }, +); + +/** Sanitized approval transition delivered only to an opted-in session audience. */ +export const SessionApprovalEventSchema = Type.Union([ + PendingSessionApprovalEventSchema, + TerminalSessionApprovalEventSchema, +]); + +/** Authoritative pending approval set returned when a session stream subscribes. */ +export const SessionApprovalReplaySchema = Type.Object( + { + sessionKey: NonEmptyString, + updatedAtMs: Type.Integer({ minimum: 0 }), + approvals: Type.Array(PendingApprovalSnapshotSchema), + truncated: Type.Boolean(), + }, + { additionalProperties: false }, +); + // Owner-local wire types derived directly from local schema consts so the // public plugin-sdk declaration graph never pulls in the ProtocolSchemas registry. export type ApprovalKind = Static; @@ -225,3 +291,5 @@ export type DeniedApprovalSnapshot = Static export type ExpiredApprovalSnapshot = Static; export type CancelledApprovalSnapshot = Static; export type TerminalApprovalSnapshot = Static; +export type SessionApprovalEvent = Static; +export type SessionApprovalReplay = Static; diff --git a/packages/gateway-protocol/src/schema/protocol-schemas.ts b/packages/gateway-protocol/src/schema/protocol-schemas.ts index 310698520c0b..13fccde5db21 100644 --- a/packages/gateway-protocol/src/schema/protocol-schemas.ts +++ b/packages/gateway-protocol/src/schema/protocol-schemas.ts @@ -94,13 +94,19 @@ import { import { AllowedApprovalSnapshotSchema, ApprovalAllowDecisionSchema, + ApprovalAllowedReasonSchema, + ApprovalCancelledReasonSchema, ApprovalDecisionSchema, + ApprovalDeniedReasonSchema, + ApprovalExpiredReasonSchema, ApprovalGetParamsSchema, ApprovalGetResultSchema, ApprovalKindSchema, ApprovalPresentationSchema, ApprovalResolveParamsSchema, ApprovalResolveResultSchema, + SessionApprovalEventSchema, + SessionApprovalReplaySchema, ApprovalSnapshotSchema, ApprovalTerminalReasonSchema, CancelledApprovalSnapshotSchema, @@ -108,9 +114,11 @@ import { ExecApprovalPresentationSchema, ExpiredApprovalSnapshotSchema, PendingApprovalSnapshotSchema, + PendingSessionApprovalEventSchema, PluginApprovalPresentationSchema, PluginApprovalSeveritySchema, TerminalApprovalSnapshotSchema, + TerminalSessionApprovalEventSchema, } from "./approvals.js"; import { ArtifactSummarySchema, @@ -865,6 +873,10 @@ export const ProtocolSchemas = { ApprovalKind: ApprovalKindSchema, ApprovalDecision: ApprovalDecisionSchema, ApprovalAllowDecision: ApprovalAllowDecisionSchema, + ApprovalAllowedReason: ApprovalAllowedReasonSchema, + ApprovalDeniedReason: ApprovalDeniedReasonSchema, + ApprovalExpiredReason: ApprovalExpiredReasonSchema, + ApprovalCancelledReason: ApprovalCancelledReasonSchema, PluginApprovalSeverity: PluginApprovalSeveritySchema, ExecApprovalPresentation: ExecApprovalPresentationSchema, PluginApprovalPresentation: PluginApprovalPresentationSchema, @@ -881,6 +893,10 @@ export const ProtocolSchemas = { ApprovalGetResult: ApprovalGetResultSchema, ApprovalResolveParams: ApprovalResolveParamsSchema, ApprovalResolveResult: ApprovalResolveResultSchema, + PendingSessionApprovalEvent: PendingSessionApprovalEventSchema, + TerminalSessionApprovalEvent: TerminalSessionApprovalEventSchema, + SessionApprovalEvent: SessionApprovalEventSchema, + SessionApprovalReplay: SessionApprovalReplaySchema, ExecApprovalsGetParams: ExecApprovalsGetParamsSchema, ExecApprovalsSetParams: ExecApprovalsSetParamsSchema, ExecApprovalsNodeGetParams: ExecApprovalsNodeGetParamsSchema, diff --git a/packages/gateway-protocol/src/schema/sessions.ts b/packages/gateway-protocol/src/schema/sessions.ts index b42652ffb956..5b6421892ea1 100644 --- a/packages/gateway-protocol/src/schema/sessions.ts +++ b/packages/gateway-protocol/src/schema/sessions.ts @@ -382,6 +382,8 @@ export const SessionsMessagesSubscribeParamsSchema = Type.Object( { key: NonEmptyString, agentId: Type.Optional(NonEmptyString), + /** Opt in to sanitized durable approval events for this session and its descendants. */ + includeApprovals: Type.Optional(Type.Literal(true)), }, { additionalProperties: false }, ); diff --git a/packages/gateway-protocol/src/session-approval-validators.test.ts b/packages/gateway-protocol/src/session-approval-validators.test.ts new file mode 100644 index 000000000000..00f5a56718b6 --- /dev/null +++ b/packages/gateway-protocol/src/session-approval-validators.test.ts @@ -0,0 +1,147 @@ +import { Value } from "typebox/value"; +import { describe, expect, it } from "vitest"; +import { + SessionApprovalEventSchema, + SessionApprovalReplaySchema, + validateSessionsMessagesSubscribeParams, +} from "./index.js"; + +const approval = { + id: "approval:01JZ4K6M2X8YQW9N7R3T5V1C0B", + urlPath: "/approve/approval%3A01JZ4K6M2X8YQW9N7R3T5V1C0B", + presentation: { + kind: "exec", + commandText: "git status --short", + commandPreview: "git status", + warningText: null, + host: "gateway", + nodeId: null, + agentId: "main", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + createdAtMs: 1_780_000_000_000, + expiresAtMs: 1_780_001_800_000, +} as const; + +const pending = { ...approval, status: "pending" } as const; +const terminal = { + ...approval, + status: "denied", + decision: "deny", + resolvedAtMs: approval.createdAtMs + 1_000, + reason: "user", +} as const; + +describe("session approval protocol validators", () => { + it("keeps approval subscription opt-in additive and literal", () => { + expect(validateSessionsMessagesSubscribeParams({ key: "agent:main:main" })).toBe(true); + expect( + validateSessionsMessagesSubscribeParams({ + key: "agent:main:main", + includeApprovals: true, + }), + ).toBe(true); + expect( + validateSessionsMessagesSubscribeParams({ + key: "agent:main:main", + includeApprovals: false, + }), + ).toBe(false); + }); + + it("requires event phase to match the approval snapshot state", () => { + const common = { + sessionKey: "agent:main:main", + sourceSessionKey: "agent:worker:subagent:child", + updatedAtMs: terminal.resolvedAtMs, + } as const; + + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + phase: "pending", + approval: pending, + }), + ).toBe(true); + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + phase: "terminal", + approval: terminal, + }), + ).toBe(true); + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + phase: "pending", + approval: terminal, + }), + ).toBe(false); + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + phase: "terminal", + approval: pending, + }), + ).toBe(false); + }); + + it("rejects terminal reasons that contradict the terminal status", () => { + const common = { + sessionKey: "agent:main:main", + phase: "terminal", + updatedAtMs: terminal.resolvedAtMs, + } as const; + const terminalCommon = { + ...approval, + resolvedAtMs: terminal.resolvedAtMs, + } as const; + + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + approval: { ...terminal, reason: "timeout" }, + }), + ).toBe(false); + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + approval: { + ...terminalCommon, + status: "allowed", + decision: "allow-once", + reason: "timeout", + }, + }), + ).toBe(false); + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + approval: { ...terminalCommon, status: "expired", reason: "user" }, + }), + ).toBe(false); + expect( + Value.Check(SessionApprovalEventSchema, { + ...common, + approval: { ...terminalCommon, status: "cancelled", reason: "timeout" }, + }), + ).toBe(false); + }); + + it("replays only authoritative pending approval snapshots", () => { + const replay = { + sessionKey: "agent:main:main", + updatedAtMs: approval.createdAtMs, + approvals: [pending], + truncated: false, + } as const; + + expect(Value.Check(SessionApprovalReplaySchema, replay)).toBe(true); + expect( + Value.Check(SessionApprovalReplaySchema, { + ...replay, + approvals: [terminal], + }), + ).toBe(false); + }); +}); diff --git a/src/gateway/approval-session-audience.test.ts b/src/gateway/approval-session-audience.test.ts index 0487e7f66154..b8272bbad4c2 100644 --- a/src/gateway/approval-session-audience.test.ts +++ b/src/gateway/approval-session-audience.test.ts @@ -1,9 +1,23 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { + resolveApprovalFallbackAudienceSessionKey, + resolveApprovalSessionAudience, resolveApprovalSessionAudienceFromSources, + resolveApprovalSourceStreamKey, type ApprovalSessionAudienceSources, } from "./approval-session-audience.js"; +const getRuntimeConfigMock = vi.fn(() => ({}) as object); +vi.mock("../config/io.js", () => ({ + getRuntimeConfig: () => getRuntimeConfigMock(), +})); +vi.mock("../agents/subagent-registry-read.js", () => ({ + buildLatestSubagentRunReadIndex: () => ({ getLatestSubagentRun: () => undefined }), +})); +vi.mock("../config/sessions/session-accessor.js", () => ({ + loadSessionEntry: () => undefined, +})); + type GraphNode = { registry?: { controllerSessionKey?: string | null; @@ -32,6 +46,13 @@ function resolveAudience( } describe("resolveApprovalSessionAudienceFromSources", () => { + it("scopes a global source to the agent-specific stream key", () => { + expect(resolveApprovalSourceStreamKey(" global ", "Work Agent")).toBe( + "agent:work-agent:global", + ); + expect(resolveApprovalSourceStreamKey("agent:work:child", "work")).toBe("agent:work:child"); + }); + it("keeps the canonical source first when it has no ancestors", () => { expect( resolveAudience(" Child ", {}, (key) => `agent:main:${key.trim().toLowerCase()}`), @@ -139,3 +160,46 @@ describe("resolveApprovalSessionAudienceFromSources", () => { expect(audience.at(-1)).toBe("session-63"); }); }); + +describe("resolveApprovalFallbackAudienceSessionKey", () => { + it("canonicalizes configured main-key aliases when config loads", () => { + getRuntimeConfigMock.mockReturnValueOnce({ session: { mainKey: "boss" } }); + expect(resolveApprovalFallbackAudienceSessionKey("main", "work")).toBe("agent:work:boss"); + }); + + it("scopes unscoped aliases even when config loading throws", () => { + getRuntimeConfigMock.mockImplementationOnce(() => { + throw new Error("config unavailable"); + }); + expect(resolveApprovalFallbackAudienceSessionKey("child", "work")).toBe("agent:work:child"); + }); +}); + +describe("resolveApprovalSourceStreamKey fallback scoping", () => { + it("scopes raw fallback aliases to the raising agent", () => { + expect(resolveApprovalSourceStreamKey("child", "work")).toBe("agent:work:child"); + expect(resolveApprovalSourceStreamKey("GLOBAL", "work")).toBe("agent:work:global"); + }); + + it("keeps agent-scoped, unknown, and agent-less keys exact", () => { + expect(resolveApprovalSourceStreamKey("agent:other:child", "work")).toBe("agent:other:child"); + expect(resolveApprovalSourceStreamKey("unknown", "work")).toBe("unknown"); + expect(resolveApprovalSourceStreamKey("child", null)).toBe("child"); + }); +}); + +describe("resolveApprovalSessionAudience runtime scoping", () => { + it("scopes unscoped source aliases to the raising agent", () => { + expect(resolveApprovalSessionAudience("child", "work")).toEqual(["agent:work:child"]); + }); + + it("scopes a global source to the raising agent stream", () => { + expect(resolveApprovalSessionAudience("global", "work")).toEqual(["agent:work:global"]); + }); + + it("keeps explicit cross-agent source keys exact", () => { + expect(resolveApprovalSessionAudience("agent:other:child", "work")).toEqual([ + "agent:other:child", + ]); + }); +}); diff --git a/src/gateway/approval-session-audience.ts b/src/gateway/approval-session-audience.ts index 493e997d108d..efafd694c89d 100644 --- a/src/gateway/approval-session-audience.ts +++ b/src/gateway/approval-session-audience.ts @@ -1,8 +1,10 @@ +import { resolveDefaultAgentId } from "../agents/agent-scope.js"; import { buildLatestSubagentRunReadIndex } from "../agents/subagent-registry-read.js"; import { getRuntimeConfig } from "../config/io.js"; import { loadSessionEntry } from "../config/sessions/session-accessor.js"; import type { SessionEntry } from "../config/sessions/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { normalizeAgentId, parseAgentSessionKey } from "../routing/session-key.js"; import { OPERATOR_APPROVAL_MAX_AUDIENCE_SESSION_KEYS } from "./operator-approval-store.js"; import { canonicalizeSpawnedByForAgent, @@ -102,32 +104,120 @@ export function resolveApprovalSessionAudienceFromSources(params: { function createRuntimeApprovalSessionAudienceSources( cfg: OpenClawConfig, + sourceAgentId?: string | null, ): ApprovalSessionAudienceSources { const subagentRuns = buildLatestSubagentRunReadIndex(); + const resolveStorageTarget = (sessionKey: string): { agentId: string; sessionKey: string } => { + const parsed = parseAgentSessionKey(sessionKey); + if (parsed?.rest.toLowerCase() === "global") { + return { agentId: normalizeAgentId(parsed.agentId), sessionKey: "global" }; + } + return { + agentId: resolveSessionStoreAgentId(cfg, sessionKey), + sessionKey, + }; + }; return { canonicalizeSessionKey: (sessionKey, relativeToSessionKey) => { if (!relativeToSessionKey) { - return resolveSessionStoreKey({ cfg, sessionKey }); + return canonicalizeApprovalSourceStreamKey(cfg, sessionKey, sourceAgentId); } const relativeAgentId = resolveSessionStoreAgentId(cfg, relativeToSessionKey); - return canonicalizeSpawnedByForAgent(cfg, relativeAgentId, sessionKey); + const canonical = canonicalizeSpawnedByForAgent(cfg, relativeAgentId, sessionKey); + return canonical ? resolveApprovalSourceStreamKey(canonical, relativeAgentId) : canonical; }, getLatestSubagentLineage: (sessionKey) => subagentRuns.getLatestSubagentRun(sessionKey), - getStoredSessionLineage: (sessionKey) => - loadSessionEntry({ - agentId: resolveSessionStoreAgentId(cfg, sessionKey), + getStoredSessionLineage: (sessionKey) => { + const target = resolveStorageTarget(sessionKey); + return loadSessionEntry({ + agentId: target.agentId, clone: false, hydrateSkillPromptRefs: false, - sessionKey, - }), + sessionKey: target.sessionKey, + }); + }, }; } /** Resolves an approval audience from the live registry and session stores. */ -export function resolveApprovalSessionAudience(sourceSessionKey: string): string[] { +export function resolveApprovalSessionAudience( + sourceSessionKey: string, + sourceAgentId?: string | null, +): string[] { const cfg = getRuntimeConfig(); return resolveApprovalSessionAudienceFromSources({ sourceSessionKey, - sources: createRuntimeApprovalSessionAudienceSources(cfg), + sources: createRuntimeApprovalSessionAudienceSources(cfg, sourceAgentId), }); } + +/** Canonicalize one source key against config: agent scoping, main-key aliases, global sentinel. */ +function canonicalizeApprovalSourceStreamKey( + cfg: OpenClawConfig, + sessionKey: string, + sourceAgentId?: string | null, +): string { + const ownerAgentId = normalizeAgentId(sourceAgentId ?? resolveDefaultAgentId(cfg)); + // Unscoped source aliases (e.g. "child", "main") must resolve against the + // raising agent's store, not the default agent's, or multi-agent audiences + // route to the wrong session streams. + const lowered = sessionKey.trim().toLowerCase(); + const scoped = + parseAgentSessionKey(sessionKey) || lowered === "global" || lowered === "unknown" + ? sessionKey + : `agent:${ownerAgentId}:${sessionKey}`; + const canonical = resolveSessionStoreKey({ cfg, sessionKey: scoped }); + // Storage uses the bare global sentinel, while live session streams are + // agent-scoped so one agent cannot receive another's global events. + return resolveApprovalSourceStreamKey(canonical, ownerAgentId); +} + +/** + * Fallback audience key when the lineage walk fails. Config-only + * canonicalization (agent scope, configured main-key aliases) still applies + * when the config loads; the pure-string form is the true last resort. + */ +/** Non-throwing audience resolver for injection into the approval manager. + * Lineage is routing metadata, not an approval safety prerequisite; when + * session stores are unavailable this preserves the agent-scoped source. */ +export function resolveApprovalSessionAudienceWithFallback( + sourceSessionKey: string, + sourceAgentId?: string | null, +): string[] { + try { + return resolveApprovalSessionAudience(sourceSessionKey, sourceAgentId); + } catch { + return [resolveApprovalFallbackAudienceSessionKey(sourceSessionKey, sourceAgentId)]; + } +} + +export function resolveApprovalFallbackAudienceSessionKey( + sourceSessionKey: string, + sourceAgentId?: string | null, +): string { + try { + return canonicalizeApprovalSourceStreamKey(getRuntimeConfig(), sourceSessionKey, sourceAgentId); + } catch { + return resolveApprovalSourceStreamKey(sourceSessionKey, sourceAgentId); + } +} + +/** Best-effort stream key used when lineage lookup is unavailable. */ +export function resolveApprovalSourceStreamKey( + sourceSessionKey: string, + sourceAgentId?: string | null, +): string { + const normalizedSessionKey = sourceSessionKey.trim(); + const lowered = normalizedSessionKey.toLowerCase(); + // Subscribers only know agent-scoped stream keys, so raw fallback inputs + // (bare "global", "main", unscoped child aliases) must scope to the raising + // agent or the persisted audience is unreachable exactly when lineage + // lookup already failed. "unknown" has no stream and stays bare. + if (!sourceAgentId || lowered === "unknown" || parseAgentSessionKey(normalizedSessionKey)) { + return normalizedSessionKey; + } + const agentId = normalizeAgentId(sourceAgentId); + return lowered === "global" + ? `agent:${agentId}:global` + : `agent:${agentId}:${normalizedSessionKey}`; +} diff --git a/src/gateway/exec-approval-manager.test.ts b/src/gateway/exec-approval-manager.test.ts index a5d7e9e595a0..5f2707144493 100644 --- a/src/gateway/exec-approval-manager.test.ts +++ b/src/gateway/exec-approval-manager.test.ts @@ -5,14 +5,18 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import type { ExecApprovalRequestPayload } from "../infra/exec-approvals.js"; +import type { ExecApprovalDecision, ExecApprovalRequestPayload } from "../infra/exec-approvals.js"; import type { PluginApprovalRequestPayload } from "../infra/plugin-approvals.js"; import { MAX_TIMER_TIMEOUT_MS } from "../shared/number-coercion.js"; import { closeOpenClawStateDatabase, openOpenClawStateDatabase, } from "../state/openclaw-state-db.js"; -import { ExecApprovalManager, type ExecApprovalManagerOptions } from "./exec-approval-manager.js"; +import { + ExecApprovalManager, + type ExecApprovalManagerOptions, + type OperatorApprovalLifecycleEvent, +} from "./exec-approval-manager.js"; import { getOperatorApproval, resolveOperatorApproval } from "./operator-approval-store.js"; type TimeoutCallback = Parameters[0]; @@ -35,6 +39,7 @@ describe("ExecApprovalManager", () => { options: { runtimeEpoch?: string; onError?: ExecApprovalManagerOptions["onError"]; + onLifecycle?: ExecApprovalManagerOptions["onLifecycle"]; } = {}, ) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-manager-")); @@ -49,6 +54,7 @@ describe("ExecApprovalManager", () => { resolveAllowedDecisions: () => ["allow-once", "deny"], resolveAudienceSessionKeys: (sessionKey) => [sessionKey, "agent:main:parent"], onError: options.onError, + onLifecycle: options.onLifecycle, }), }; } @@ -302,6 +308,234 @@ describe("ExecApprovalManager", () => { }); }); + it("emits pending only after durable insert and live waiter registration", async () => { + let manager!: ExecApprovalManager; + let durableAtCallback: ReturnType = null; + let waiterAtCallback: Promise | null = null; + const lifecycleEvents: OperatorApprovalLifecycleEvent[] = []; + const created = createPersistentManager({ + onLifecycle: (event) => { + lifecycleEvents.push(event); + if (event.phase === "pending") { + durableAtCallback = getOperatorApproval({ + id: event.record.id, + databaseOptions: created.databaseOptions, + }); + waiterAtCallback = manager.awaitDecision(event.record.id); + } + }, + }); + manager = created.manager; + const record = manager.create( + { command: "echo ordered", sessionKey: "agent:main:child" }, + 60_000, + "approval-lifecycle-ordered", + ); + + const decisionPromise = manager.register(record, 60_000); + + expect(lifecycleEvents).toMatchObject([ + { + phase: "pending", + record: { + id: record.id, + status: "pending", + audienceSessionKeys: ["agent:main:child", "agent:main:parent"], + }, + }, + ]); + expect(durableAtCallback).toEqual(lifecycleEvents[0]?.record); + expect(waiterAtCallback).toBe(decisionPromise); + + manager.resolveDetailed(record.id, "deny", { kind: "system", id: null }); + await expect(decisionPromise).resolves.toBe("deny"); + }); + + it("passes the source agent when deriving a global-session stream audience", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-manager-")); + tempDirs.push(dir); + const databaseOptions = { path: path.join(dir, "state.sqlite") }; + const resolveAudienceSessionKeys = vi.fn((sessionKey: string, agentId?: string | null) => [ + sessionKey === "global" && agentId ? `agent:${agentId}:global` : sessionKey, + ]); + const manager = new ExecApprovalManager({ + approvalKind: "exec", + persistence: { runtimeEpoch: "runtime-a", databaseOptions }, + resolveAllowedDecisions: () => ["allow-once", "deny"], + resolveAudienceSessionKeys, + }); + const record = manager.create( + { command: "echo global", sessionKey: "global", agentId: "work" }, + 60_000, + "approval-global-audience", + ); + const decisionPromise = manager.register(record, 60_000); + + expect(resolveAudienceSessionKeys).toHaveBeenCalledWith("global", "work"); + expect(getOperatorApproval({ id: record.id, databaseOptions })).toMatchObject({ + source: { sessionKey: "global", agentId: "work" }, + audienceSessionKeys: ["agent:work:global"], + }); + + manager.resolveDetailed(record.id, "deny", { kind: "system", id: null }); + await expect(decisionPromise).resolves.toBe("deny"); + }); + + it("emits one terminal event for the winning resolution and none for later answers", async () => { + const lifecycleEvents: OperatorApprovalLifecycleEvent[] = []; + const { manager } = createPersistentManager({ + onLifecycle: (event) => lifecycleEvents.push(event), + }); + const record = manager.create({ command: "echo race" }, 60_000, "approval-lifecycle-race"); + const decisionPromise = manager.register(record, 60_000); + + expect( + manager.resolveDetailed(record.id, "allow-once", { + kind: "device", + id: "control-ui", + }), + ).toMatchObject({ outcome: "resolved" }); + expect( + manager.resolveDetailed(record.id, "deny", { + kind: "channel", + id: "telegram", + }), + ).toMatchObject({ outcome: "already-resolved", retry: "conflict" }); + await expect(decisionPromise).resolves.toBe("allow-once"); + + expect(lifecycleEvents.map((event) => event.phase)).toEqual(["pending", "terminal"]); + expect(lifecycleEvents[1]?.record).toMatchObject({ + id: record.id, + status: "allowed", + decision: "allow-once", + resolver: { kind: "device", id: "control-ui" }, + }); + }); + + it("emits a terminal event when the durable timeout wins", async () => { + const timers = installTimerMocks(); + vi.spyOn(Date, "now").mockReturnValue(1_000); + const lifecycleEvents: OperatorApprovalLifecycleEvent[] = []; + const { manager } = createPersistentManager({ + onLifecycle: (event) => lifecycleEvents.push(event), + }); + const record = manager.create( + { command: "echo timeout" }, + 60_000, + "approval-lifecycle-timeout", + ); + const decisionPromise = manager.register(record, 60_000); + vi.mocked(Date.now).mockReturnValue(record.expiresAtMs); + + runTimer(timers[0]); + + await expect(decisionPromise).resolves.toBeNull(); + expect(lifecycleEvents.map((event) => event.phase)).toEqual(["pending", "terminal"]); + expect(lifecycleEvents[1]?.record).toMatchObject({ + id: record.id, + status: "expired", + decision: "deny", + terminalReason: "timeout", + }); + }); + + it("emits a terminal event for an explicit force-deny transition", async () => { + const lifecycleEvents: OperatorApprovalLifecycleEvent[] = []; + const { manager } = createPersistentManager({ + onLifecycle: (event) => lifecycleEvents.push(event), + }); + const record = manager.create( + { command: "echo malformed" }, + 60_000, + "approval-lifecycle-force-deny", + ); + const decisionPromise = manager.register(record, 60_000); + + expect( + manager.forceDenyDetailed(record.id, "malformed-verdict", { + kind: "system", + id: "invalid-verdict", + }), + ).toMatchObject({ outcome: "denied" }); + await expect(decisionPromise).resolves.toBe("deny"); + + expect(lifecycleEvents.map((event) => event.phase)).toEqual(["pending", "terminal"]); + expect(lifecycleEvents[1]?.record).toMatchObject({ + id: record.id, + status: "denied", + decision: "deny", + terminalReason: "malformed-verdict", + }); + }); + + it("isolates lifecycle callback failures from registration and resolution", async () => { + const onLifecycle = vi.fn(() => { + throw new Error("stream unavailable"); + }); + const { manager, databaseOptions } = createPersistentManager({ onLifecycle }); + const record = manager.create( + { command: "echo isolated" }, + 60_000, + "approval-lifecycle-isolation", + ); + + let decisionPromise!: Promise; + expect(() => { + decisionPromise = manager.register(record, 60_000); + }).not.toThrow(); + expect(() => + manager.resolveDetailed(record.id, "deny", { + kind: "device", + id: "control-ui", + }), + ).not.toThrow(); + await expect(decisionPromise).resolves.toBe("deny"); + + expect(onLifecycle).toHaveBeenCalledTimes(2); + expect(getOperatorApproval({ id: record.id, databaseOptions })).toMatchObject({ + status: "denied", + decision: "deny", + }); + }); + + it("does not re-emit pending for an idempotent persisted registration", () => { + installTimerMocks(); + const { manager, databaseOptions } = createPersistentManager(); + const record = manager.create( + { command: "echo replay", sessionKey: "agent:main:child" }, + 60_000, + "approval-lifecycle-existing", + ); + const originalPromise = manager.register(record, 60_000); + const onLifecycle = vi.fn(); + const replayManager = new ExecApprovalManager({ + approvalKind: "exec", + persistence: { runtimeEpoch: "runtime-a", databaseOptions }, + resolveAllowedDecisions: () => ["allow-once", "deny"], + resolveAudienceSessionKeys: (sessionKey) => [sessionKey, "agent:main:parent"], + onLifecycle, + }); + + const replayPromise = replayManager.register( + { ...record, request: { ...record.request } }, + 60_000, + ); + + expect(replayManager.awaitDecision(record.id)).toBe(replayPromise); + expect(onLifecycle).not.toHaveBeenCalled(); + expect(getOperatorApproval({ id: record.id, databaseOptions })).toMatchObject({ + id: record.id, + status: "pending", + }); + + manager.resolveDetailed(record.id, "deny", { kind: "system", id: null }); + replayManager.resolveDetailed(record.id, "deny", { kind: "system", id: null }); + return Promise.all([ + expect(originalPromise).resolves.toBe("deny"), + expect(replayPromise).resolves.toBe("deny"), + ]); + }); + it("persists only the reviewer-safe presentation while retaining the local request", async () => { const { manager, databaseOptions } = createPersistentManager(); const request: ExecApprovalRequestPayload = { @@ -547,6 +781,47 @@ describe("ExecApprovalManager", () => { }); }); + it("publishes durable expiry when storage recovery crosses the deadline", async () => { + installTimerMocks(); + vi.spyOn(Date, "now").mockReturnValue(1_000); + const lifecycleEvents: OperatorApprovalLifecycleEvent[] = []; + const { manager, databaseOptions, dir } = createPersistentManager({ + onLifecycle: (event) => lifecycleEvents.push(event), + }); + const record = manager.create( + { command: "echo expiry" }, + 1_000, + "approval-storage-recovery-expiry", + ); + const decisionPromise = manager.register(record, 1_000); + const validDatabasePath = databaseOptions.path; + const blocker = path.join(dir, "expiry-storage-blocker"); + fs.writeFileSync(blocker, "blocked"); + databaseOptions.path = path.join(blocker, "state.sqlite"); + + expect(() => + manager.resolveDetailed(record.id, "allow-once", { + kind: "device", + id: "control-ui", + }), + ).toThrow(); + await expect(decisionPromise).resolves.toBe("deny"); + + databaseOptions.path = validDatabasePath; + vi.mocked(Date.now).mockReturnValue(record.expiresAtMs); + expect( + manager.resolveDetailed(record.id, "allow-once", { + kind: "device", + id: "control-ui", + }), + ).toMatchObject({ outcome: "expired", record: { status: "expired" } }); + expect(lifecycleEvents.map((event) => event.phase)).toEqual(["pending", "terminal"]); + expect(lifecycleEvents[1]?.record).toMatchObject({ + status: "expired", + terminalReason: "timeout", + }); + }); + it("reconciles a durable terminal row into the existing local waiter", async () => { const { manager, databaseOptions } = createPersistentManager(); const record = manager.create({ command: "echo ok" }, 60_000, "approval-reconcile"); diff --git a/src/gateway/exec-approval-manager.ts b/src/gateway/exec-approval-manager.ts index 7121d58ff5e6..17ba6ccaaa6e 100644 --- a/src/gateway/exec-approval-manager.ts +++ b/src/gateway/exec-approval-manager.ts @@ -91,14 +91,23 @@ export type ExecApprovalManagerOptions = { approvalKind?: OperatorApprovalKind; persistence?: OperatorApprovalPersistenceRuntime; resolveAllowedDecisions?: (request: TPayload) => readonly ExecApprovalDecision[]; - /** Session-lineage audience policy is gateway-owned and injected; importing - * it here would close an agents->gateway barrel cycle. Absent resolver seeds - * only the raising session. */ - resolveAudienceSessionKeys?: (sourceSessionKey: string) => string[]; + /** Session-lineage audience policy is gateway-owned and injected as a + * non-throwing resolver; importing it here would close an agents->gateway + * barrel cycle. Absent resolver (tests) seeds only the raising session. */ + resolveAudienceSessionKeys?: ( + sourceSessionKey: string, + sourceAgentId?: string | null, + ) => string[]; onError?: ( error: Error, context: { approvalId: string; approvalKind: OperatorApprovalKind; operation: "expire" }, ) => void; + onLifecycle?: (event: OperatorApprovalLifecycleEvent) => void; +}; + +export type OperatorApprovalLifecycleEvent = { + phase: "pending" | "terminal"; + record: OperatorApprovalRecord; }; type WithLiveRecord = TResult extends { record: OperatorApprovalRecord } @@ -257,19 +266,17 @@ export class ExecApprovalManager { throw new Error(`approval id '${record.id}' already resolved`); } + let insertedRecord: OperatorApprovalRecord | null = null; if (persistence) { const source = resolveApprovalSource(record.request); let audienceSessionKeys: string[] = []; if (source.sessionKey) { - try { - audienceSessionKeys = this.options.resolveAudienceSessionKeys?.(source.sessionKey) ?? [ - source.sessionKey, - ]; - } catch { - // Lineage is routing metadata, not an approval safety prerequisite. - // Preserve at least the source audience when session stores are unavailable. - audienceSessionKeys = [source.sessionKey]; - } + // The injected resolver owns lineage lookup plus its own agent-scoped + // fallback and never throws. Without one (tests), seed the raw source. + audienceSessionKeys = this.options.resolveAudienceSessionKeys?.( + source.sessionKey, + source.agentId, + ) ?? [source.sessionKey]; } const inserted = insertOperatorApproval({ approval: { @@ -293,6 +300,9 @@ export class ExecApprovalManager { if (inserted.outcome === "conflict") { throw new Error(`approval id '${record.id}' conflicts with persisted state`); } + if (inserted.outcome === "inserted") { + insertedRecord = inserted.record; + } } let resolvePromise: (decision: ExecApprovalDecision | null) => void; @@ -315,9 +325,21 @@ export class ExecApprovalManager { }; this.pending.set(record.id, entry); this.scheduleExpiryTimer(entry); + if (insertedRecord) { + this.emitLifecycle({ phase: "pending", record: insertedRecord }); + } return promise; } + private emitLifecycle(event: OperatorApprovalLifecycleEvent): void { + try { + this.options.onLifecycle?.(event); + } catch { + // Stream fanout is observational. It must never change approval truth or + // prevent the durable first-answer transition from releasing its waiter. + } + } + private projectLocalRecord(record: ExecApprovalRecord): OperatorApprovalRecord | null { const presentation = buildApprovalPresentation({ kind: this.approvalKind, @@ -537,7 +559,7 @@ export class ExecApprovalManager { localDecision?: ExecApprovalDecision | null, localResolvedBy: string | null = null, localResolutionSource: ExecApprovalResolutionSource = "operator", - ): void { + ): boolean { const persistence = this.options.persistence; if ( record.kind !== this.approvalKind || @@ -545,7 +567,7 @@ export class ExecApprovalManager { record.status === "pending" || record.resolvedAtMs === null ) { - return; + return false; } const decision = localDecision === undefined @@ -553,7 +575,7 @@ export class ExecApprovalManager { ? record.decision : null : localDecision; - this.settleLocalEntry({ + const settled = this.settleLocalEntry({ recordId: record.id, decision, resolvedAtMs: record.resolvedAtMs, @@ -565,6 +587,15 @@ export class ExecApprovalManager { consumedBy: record.consumedBy, resolutionSource: localResolutionSource, }); + if (settled) { + this.emitLifecycle({ phase: "terminal", record }); + } + return settled; + } + + /** Settle one durable terminal transition and report whether this manager published it. */ + reconcileDurableTerminal(record: OperatorApprovalRecord): boolean { + return this.settleLocalFromStore(record); } /** Reconciles durable truth with an existing waiter without rehydrating its request. */ @@ -630,6 +661,9 @@ export class ExecApprovalManager { runtimeEpoch: persistence.runtimeEpoch, databaseOptions: persistence.databaseOptions, }); + if (result.outcome === "denied" || result.outcome === "expired") { + this.emitLifecycle({ phase: "terminal", record: result.record }); + } return attachLiveRecord(result, localEntry.record) as ExecApprovalForceDenyResult; } diff --git a/src/gateway/local-request-context.ts b/src/gateway/local-request-context.ts index a249bffcf1a8..261163c7e9db 100644 --- a/src/gateway/local-request-context.ts +++ b/src/gateway/local-request-context.ts @@ -131,7 +131,7 @@ function createLocalGatewayRequestContext( unsubscribeSessionEvents: (connId) => { sessionEvents.delete(connId); }, - subscribeSessionMessageEvents: () => {}, + subscribeSessionMessageEvents: () => undefined, unsubscribeSessionMessageEvents: () => {}, unsubscribeAllSessionEvents: (connId) => { sessionEvents.delete(connId); diff --git a/src/gateway/operator-approval-session-events.test.ts b/src/gateway/operator-approval-session-events.test.ts new file mode 100644 index 000000000000..feba500e9cc3 --- /dev/null +++ b/src/gateway/operator-approval-session-events.test.ts @@ -0,0 +1,586 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { buildApprovalResolutionRef } from "../infra/approval-resolution-ref.js"; +import type { ExecApprovalRequestPayload } from "../infra/exec-approvals.js"; +import { + closeOpenClawStateDatabaseForTest, + type OpenClawStateDatabaseOptions, +} from "../state/openclaw-state-db.js"; +import { ExecApprovalManager } from "./exec-approval-manager.js"; +import { createOperatorApprovalSessionEventRuntime } from "./operator-approval-session-events.js"; +import { + insertOperatorApproval, + resolveOperatorApproval, + type NewOperatorApproval, + type OperatorApprovalRecord, +} from "./operator-approval-store.js"; +import type { GatewayBroadcastToConnIdsFn } from "./server-broadcast-types.js"; +import { createSessionMessageSubscriberRegistry } from "./server-chat-state.js"; +import type { GatewayClient } from "./server-methods/types.js"; + +const SOURCE_SESSION_KEY = "agent:main:child"; +const PARENT_SESSION_KEY = "agent:main:parent"; +const SIBLING_SESSION_KEY = "agent:main:parent:sibling"; +const tempDirs: string[] = []; + +function createDatabaseOptions(): OpenClawStateDatabaseOptions { + const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-events-")); + tempDirs.push(stateDir); + return { env: { ...process.env, OPENCLAW_STATE_DIR: stateDir } }; +} + +function createClient(params: { + connId: string; + scopes: string[]; + deviceId?: string; + invalidated?: boolean; +}): GatewayClient { + return { + connId: params.connId, + connect: { + client: { id: "approval-session-events", displayName: "Approval Session Events" }, + scopes: params.scopes, + ...(params.deviceId ? { device: { id: params.deviceId } } : {}), + }, + ...(params.invalidated ? { invalidated: true } : {}), + } as unknown as GatewayClient; +} + +function createPendingRecord( + params: { + id?: string; + audienceSessionKeys?: string[]; + sourceSessionKey?: string | null; + reviewerDeviceIds?: string[]; + createdAtMs?: number; + expiresAtMs?: number; + } = {}, +): OperatorApprovalRecord { + const id = params.id ?? "approval:child/request?1"; + const createdAtMs = params.createdAtMs ?? 1_000; + return { + id, + resolutionRef: buildApprovalResolutionRef({ approvalId: id, approvalKind: "exec" }), + kind: "exec", + status: "pending", + presentation: { + kind: "exec", + commandText: "printf session-approval", + commandPreview: "printf session-approval", + warningText: "Review this command", + host: "gateway", + nodeId: null, + agentId: "main", + allowedDecisions: ["allow-once", "allow-always", "deny"], + }, + requester: { + deviceId: "requester-device", + clientId: "requester-client", + deviceTokenAuth: true, + }, + reviewerDeviceIds: params.reviewerDeviceIds ?? ["reviewer-device"], + source: { + agentId: "main", + sessionKey: params.sourceSessionKey ?? SOURCE_SESSION_KEY, + sessionId: "private-session-id", + runId: "private-run-id", + toolCallId: "private-tool-call-id", + toolName: "exec", + }, + audienceSessionKeys: params.audienceSessionKeys ?? [SOURCE_SESSION_KEY, PARENT_SESSION_KEY], + runtimeEpoch: "private-runtime-epoch", + createdAtMs, + expiresAtMs: params.expiresAtMs ?? 10_000, + updatedAtMs: createdAtMs, + decision: null, + terminalReason: null, + resolvedAtMs: null, + resolver: null, + consumedAtMs: null, + consumedBy: null, + }; +} + +function createTerminalRecord( + pending: OperatorApprovalRecord, + resolvedAtMs = 2_000, +): OperatorApprovalRecord { + return { + ...pending, + status: "denied", + updatedAtMs: resolvedAtMs, + decision: "deny", + terminalReason: "user", + resolvedAtMs, + resolver: { kind: "device", id: "reviewer-device" }, + }; +} + +function createRuntime(params: { + clients: GatewayClient[]; + databaseOptions?: OpenClawStateDatabaseOptions; + now?: () => number; + controlUiBasePath?: string; + reconcileTerminal?: Parameters< + typeof createOperatorApprovalSessionEventRuntime + >[0]["reconcileTerminal"]; +}) { + const subscribers = createSessionMessageSubscriberRegistry(); + const broadcastToConnIds = vi.fn(); + const runtime = createOperatorApprovalSessionEventRuntime({ + clients: params.clients, + sessionMessageSubscribers: subscribers, + broadcastToConnIds, + databaseOptions: params.databaseOptions, + controlUiBasePath: params.controlUiBasePath, + now: params.now, + reconcileTerminal: params.reconcileTerminal, + }); + return { broadcastToConnIds, runtime, subscribers }; +} + +function insertPendingApproval(params: { + databaseOptions: OpenClawStateDatabaseOptions; + id: string; + audienceSessionKeys: string[]; + createdAtMs: number; + expiresAtMs: number; + reviewerDeviceIds?: string[]; +}): OperatorApprovalRecord { + const record = createPendingRecord(params); + const approval: NewOperatorApproval = { + id: record.id, + kind: record.kind, + presentation: record.presentation, + requester: record.requester, + reviewerDeviceIds: params.reviewerDeviceIds ?? record.reviewerDeviceIds, + source: record.source, + audienceSessionKeys: record.audienceSessionKeys, + runtimeEpoch: record.runtimeEpoch, + createdAtMs: record.createdAtMs, + expiresAtMs: record.expiresAtMs, + }; + const inserted = insertOperatorApproval({ approval, databaseOptions: params.databaseOptions }); + if (inserted.outcome !== "inserted") { + throw new Error(`expected approval '${params.id}' to be inserted`); + } + return inserted.record; +} + +describe("operator approval session events", () => { + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + closeOpenClawStateDatabaseForTest(); + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { force: true, recursive: true }); + } + }); + + it("targets exact opted-in source and ancestor audiences with reviewer authorization", () => { + const clients = [ + createClient({ connId: "source-admin", scopes: ["operator.admin"] }), + createClient({ + connId: "source-device", + scopes: ["operator.approvals"], + deviceId: "source-reviewer", + }), + createClient({ connId: "source-no-device", scopes: ["operator.approvals"] }), + createClient({ + connId: "source-unrelated-device", + scopes: ["operator.approvals"], + deviceId: "unrelated-device", + }), + createClient({ + connId: "source-requester-device", + scopes: ["operator.approvals"], + deviceId: "requester-device", + }), + createClient({ + connId: "source-no-scope", + scopes: ["operator.read"], + deviceId: "unprivileged-device", + }), + createClient({ connId: "source-not-opted-in", scopes: ["operator.admin"] }), + createClient({ + connId: "source-invalidated", + scopes: ["operator.admin"], + invalidated: true, + }), + createClient({ + connId: "parent-device", + scopes: ["operator.approvals"], + deviceId: "parent-reviewer", + }), + createClient({ connId: "sibling-admin", scopes: ["operator.admin"] }), + ]; + const { broadcastToConnIds, runtime, subscribers } = createRuntime({ + clients, + controlUiBasePath: "/operator/", + }); + for (const connId of [ + "source-admin", + "source-device", + "source-no-device", + "source-unrelated-device", + "source-requester-device", + "source-no-scope", + "source-invalidated", + ]) { + subscribers.subscribe(connId, SOURCE_SESSION_KEY, { includeApprovals: true }); + } + subscribers.subscribe("source-not-opted-in", SOURCE_SESSION_KEY); + subscribers.subscribe("parent-device", PARENT_SESSION_KEY, { includeApprovals: true }); + subscribers.subscribe("sibling-admin", SIBLING_SESSION_KEY, { includeApprovals: true }); + + const record = createPendingRecord({ + reviewerDeviceIds: ["source-reviewer", "parent-reviewer"], + }); + runtime.publish({ phase: "pending", record }); + + expect(broadcastToConnIds).toHaveBeenCalledTimes(2); + expect(broadcastToConnIds).toHaveBeenNthCalledWith( + 1, + "session.approval", + { + sessionKey: SOURCE_SESSION_KEY, + sourceSessionKey: SOURCE_SESSION_KEY, + phase: "pending", + updatedAtMs: 1_000, + approval: { + id: record.id, + status: "pending", + presentation: record.presentation, + urlPath: "/operator/approve/approval%3Achild%2Frequest%3F1", + createdAtMs: 1_000, + expiresAtMs: 10_000, + }, + }, + new Set(["source-admin", "source-device"]), + ); + expect(broadcastToConnIds).toHaveBeenNthCalledWith( + 2, + "session.approval", + expect.objectContaining({ + sessionKey: PARENT_SESSION_KEY, + sourceSessionKey: SOURCE_SESSION_KEY, + phase: "pending", + }), + new Set(["parent-device"]), + ); + + const payloads = broadcastToConnIds.mock.calls.map((call) => call[1]); + expect(payloads).not.toContainEqual( + expect.objectContaining({ sessionKey: SIBLING_SESSION_KEY }), + ); + const serialized = JSON.stringify(payloads); + expect(serialized).not.toContain("requester-device"); + expect(serialized).not.toContain("requester-client"); + expect(serialized).not.toContain("private-session-id"); + expect(serialized).not.toContain("private-run-id"); + expect(serialized).not.toContain("private-tool-call-id"); + expect(serialized).not.toContain("private-runtime-epoch"); + }); + + it("publishes the agent-scoped stream key for global-scope sources", () => { + const client = createClient({ connId: "admin", scopes: ["operator.admin"] }); + const { broadcastToConnIds, runtime, subscribers } = createRuntime({ clients: [client] }); + subscribers.subscribe("admin", "agent:main:global", { includeApprovals: true }); + + // Storage records the bare "global" sentinel; subscribers only know the + // agent-scoped stream key, so the published event must carry that form. + const pending = createPendingRecord({ + sourceSessionKey: "global", + audienceSessionKeys: ["agent:main:global"], + }); + runtime.publish({ phase: "pending", record: pending }); + runtime.publish({ phase: "terminal", record: createTerminalRecord(pending) }); + + expect(broadcastToConnIds).toHaveBeenCalledTimes(2); + expect(broadcastToConnIds).toHaveBeenNthCalledWith( + 1, + "session.approval", + expect.objectContaining({ + sessionKey: "agent:main:global", + sourceSessionKey: "agent:main:global", + phase: "pending", + }), + new Set(["admin"]), + ); + expect(broadcastToConnIds).toHaveBeenNthCalledWith( + 2, + "session.approval", + expect.objectContaining({ + sessionKey: "agent:main:global", + sourceSessionKey: "agent:main:global", + phase: "terminal", + }), + new Set(["admin"]), + ); + }); + + it("publishes the canonical audience source key for unscoped session aliases", () => { + const client = createClient({ connId: "admin", scopes: ["operator.admin"] }); + const { broadcastToConnIds, runtime, subscribers } = createRuntime({ clients: [client] }); + subscribers.subscribe("admin", "agent:work:child", { includeApprovals: true }); + + // The persisted source may be a raw unscoped alias; subscribers must see + // the canonical stream key the audience walk seeded first. + const pending = createPendingRecord({ + sourceSessionKey: "child", + audienceSessionKeys: ["agent:work:child", "agent:work:parent"], + }); + runtime.publish({ phase: "pending", record: pending }); + + expect(broadcastToConnIds).toHaveBeenCalledWith( + "session.approval", + expect.objectContaining({ + sessionKey: "agent:work:child", + sourceSessionKey: "agent:work:child", + phase: "pending", + }), + new Set(["admin"]), + ); + }); + + it("publishes terminal state and rejects lifecycle phases inconsistent with durable status", () => { + const client = createClient({ connId: "admin", scopes: ["operator.admin"] }); + const { broadcastToConnIds, runtime, subscribers } = createRuntime({ clients: [client] }); + subscribers.subscribe("admin", SOURCE_SESSION_KEY, { includeApprovals: true }); + + const pending = createPendingRecord({ audienceSessionKeys: [SOURCE_SESSION_KEY] }); + const terminal = createTerminalRecord(pending); + runtime.publish({ phase: "terminal", record: pending }); + runtime.publish({ phase: "pending", record: terminal }); + expect(broadcastToConnIds).not.toHaveBeenCalled(); + + runtime.publish({ phase: "terminal", record: terminal }); + + expect(broadcastToConnIds).toHaveBeenCalledOnce(); + expect(broadcastToConnIds).toHaveBeenCalledWith( + "session.approval", + { + sessionKey: SOURCE_SESSION_KEY, + sourceSessionKey: SOURCE_SESSION_KEY, + phase: "terminal", + updatedAtMs: 2_000, + approval: { + id: terminal.id, + status: "denied", + decision: "deny", + reason: "user", + presentation: terminal.presentation, + urlPath: `/approve/${encodeURIComponent(terminal.id)}`, + createdAtMs: 1_000, + expiresAtMs: 10_000, + resolvedAtMs: 2_000, + }, + }, + new Set(["admin"]), + ); + }); + + it("returns the authoritative sanitized pending set for one exact audience", () => { + const databaseOptions = createDatabaseOptions(); + insertPendingApproval({ + databaseOptions, + id: "source-and-parent", + audienceSessionKeys: [SOURCE_SESSION_KEY, PARENT_SESSION_KEY], + createdAtMs: 1_000, + expiresAtMs: 10_000, + }); + const parentOnly = insertPendingApproval({ + databaseOptions, + id: "parent-only", + audienceSessionKeys: [PARENT_SESSION_KEY], + createdAtMs: 1_001, + expiresAtMs: 10_000, + }); + insertPendingApproval({ + databaseOptions, + id: "sibling-only", + audienceSessionKeys: [SIBLING_SESSION_KEY], + createdAtMs: 1_002, + expiresAtMs: 10_000, + }); + const resolved = insertPendingApproval({ + databaseOptions, + id: "already-resolved", + audienceSessionKeys: [PARENT_SESSION_KEY], + createdAtMs: 1_003, + expiresAtMs: 10_000, + }); + expect( + resolveOperatorApproval({ + id: resolved.id, + decision: "deny", + resolver: { kind: "device", id: "reviewer-device" }, + nowMs: 2_000, + databaseOptions, + }), + ).toMatchObject({ outcome: "resolved" }); + const { runtime } = createRuntime({ + clients: [], + databaseOptions, + controlUiBasePath: "/operator", + now: () => 5_000, + }); + + const replayReviewer = createClient({ + connId: "replay-reviewer", + scopes: ["operator.approvals"], + deviceId: "reviewer-device", + }); + expect(runtime.replay(PARENT_SESSION_KEY, replayReviewer)).toEqual({ + sessionKey: PARENT_SESSION_KEY, + updatedAtMs: 5_000, + truncated: false, + approvals: [ + { + id: "source-and-parent", + status: "pending", + presentation: createPendingRecord({ id: "source-and-parent" }).presentation, + urlPath: "/operator/approve/source-and-parent", + createdAtMs: 1_000, + expiresAtMs: 10_000, + }, + { + id: parentOnly.id, + status: "pending", + presentation: parentOnly.presentation, + urlPath: "/operator/approve/parent-only", + createdAtMs: 1_001, + expiresAtMs: 10_000, + }, + ], + }); + expect( + runtime.replay( + PARENT_SESSION_KEY, + createClient({ + connId: "unrelated-replay", + scopes: ["operator.approvals"], + deviceId: "unrelated-device", + }), + ), + ).toEqual({ + sessionKey: PARENT_SESSION_KEY, + updatedAtMs: 5_000, + truncated: false, + approvals: [], + }); + }); + + it("publishes replay-triggered expiry to existing ancestor recipients before an empty replay", () => { + const databaseOptions = createDatabaseOptions(); + insertPendingApproval({ + databaseOptions, + id: "expired-child-approval", + audienceSessionKeys: [SOURCE_SESSION_KEY, PARENT_SESSION_KEY], + createdAtMs: 1_000, + expiresAtMs: 4_000, + reviewerDeviceIds: ["parent-device"], + }); + const parent = createClient({ + connId: "parent-reviewer", + scopes: ["operator.approvals"], + deviceId: "parent-device", + }); + const { broadcastToConnIds, runtime, subscribers } = createRuntime({ + clients: [parent], + databaseOptions, + now: () => 5_000, + }); + subscribers.subscribe("parent-reviewer", PARENT_SESSION_KEY, { includeApprovals: true }); + + const replay = runtime.replay(SOURCE_SESSION_KEY, parent); + + expect(broadcastToConnIds).toHaveBeenCalledOnce(); + expect(broadcastToConnIds).toHaveBeenCalledWith( + "session.approval", + expect.objectContaining({ + sessionKey: PARENT_SESSION_KEY, + sourceSessionKey: SOURCE_SESSION_KEY, + phase: "terminal", + updatedAtMs: 5_000, + approval: expect.objectContaining({ + id: "expired-child-approval", + status: "expired", + reason: "timeout", + resolvedAtMs: 5_000, + }), + }), + new Set(["parent-reviewer"]), + ); + expect(replay).toEqual({ + sessionKey: SOURCE_SESSION_KEY, + updatedAtMs: 5_000, + approvals: [], + truncated: false, + }); + }); + + it("settles the owning waiter and publishes replay-triggered expiry once", async () => { + vi.useFakeTimers(); + vi.setSystemTime(1_000); + const databaseOptions = createDatabaseOptions(); + let runtime!: ReturnType; + const manager = new ExecApprovalManager({ + approvalKind: "exec", + persistence: { runtimeEpoch: "session-events", databaseOptions }, + resolveAllowedDecisions: () => ["allow-once", "deny"], + resolveAudienceSessionKeys: () => [SOURCE_SESSION_KEY, PARENT_SESSION_KEY], + onLifecycle: (event) => runtime.publish(event), + }); + const parent = createClient({ + connId: "parent-reviewer", + scopes: ["operator.approvals"], + deviceId: "parent-device", + }); + const harness = createRuntime({ + clients: [parent], + databaseOptions, + now: () => Date.now(), + reconcileTerminal: (record) => manager.reconcileDurableTerminal(record), + }); + runtime = harness.runtime; + harness.subscribers.subscribe("parent-reviewer", PARENT_SESSION_KEY, { + includeApprovals: true, + }); + const record = manager.create( + { + command: "printf replay-expiry", + sessionKey: SOURCE_SESSION_KEY, + agentId: "main", + }, + 3_000, + "replay-expiry-with-waiter", + ); + const decisionPromise = manager.register(record, 3_000); + harness.broadcastToConnIds.mockClear(); + vi.setSystemTime(record.expiresAtMs); + + expect(runtime.replay(SOURCE_SESSION_KEY, parent)).toEqual({ + sessionKey: SOURCE_SESSION_KEY, + updatedAtMs: record.expiresAtMs, + approvals: [], + truncated: false, + }); + await expect(decisionPromise).resolves.toBeNull(); + expect(harness.broadcastToConnIds).toHaveBeenCalledOnce(); + expect(harness.broadcastToConnIds).toHaveBeenCalledWith( + "session.approval", + expect.objectContaining({ + sessionKey: PARENT_SESSION_KEY, + phase: "terminal", + approval: expect.objectContaining({ status: "expired" }), + }), + new Set(["parent-reviewer"]), + ); + + await vi.advanceTimersByTimeAsync(20_000); + expect(harness.broadcastToConnIds).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/gateway/operator-approval-session-events.ts b/src/gateway/operator-approval-session-events.ts new file mode 100644 index 000000000000..2d22460cd48f --- /dev/null +++ b/src/gateway/operator-approval-session-events.ts @@ -0,0 +1,150 @@ +import type { + PendingApprovalSnapshot, + SessionApprovalEvent, + SessionApprovalReplay, +} from "../../packages/gateway-protocol/src/index.js"; +import type { OpenClawStateDatabaseOptions } from "../state/openclaw-state-db.js"; +import { resolveApprovalSourceStreamKey } from "./approval-session-audience.js"; +import { normalizeControlUiBasePath } from "./control-ui-shared.js"; +import type { OperatorApprovalLifecycleEvent } from "./exec-approval-manager.js"; +import { canAccessOperatorApproval } from "./operator-approval-authorization.js"; +import { projectOperatorApprovalSnapshot } from "./operator-approval-snapshot.js"; +import { + expireDueOperatorApprovals, + listPendingOperatorApprovals, + type OperatorApprovalRecord, +} from "./operator-approval-store.js"; +import type { GatewayBroadcastToConnIdsFn } from "./server-broadcast-types.js"; +import type { SessionMessageSubscriberRegistry } from "./server-chat-state.js"; +import type { GatewayClient } from "./server-methods/types.js"; + +const MAX_SESSION_APPROVAL_REPLAY = 1_000; +type ApprovalSessionClient = GatewayClient & { invalidated?: boolean }; + +export type OperatorApprovalSessionEventRuntime = { + publish: (event: OperatorApprovalLifecycleEvent) => void; + replay: (sessionKey: string, client: GatewayClient | null) => SessionApprovalReplay; +}; + +/** Project durable approval truth to exact, explicitly opted-in session audiences. */ +export function createOperatorApprovalSessionEventRuntime(params: { + clients: Iterable; + sessionMessageSubscribers: Pick; + broadcastToConnIds: GatewayBroadcastToConnIdsFn; + controlUiBasePath?: string; + databaseOptions?: OpenClawStateDatabaseOptions; + now?: () => number; + reconcileTerminal?: (record: OperatorApprovalRecord) => boolean; +}): OperatorApprovalSessionEventRuntime { + const controlUiBasePath = normalizeControlUiBasePath(params.controlUiBasePath); + const now = params.now ?? Date.now; + + const canAccessRecord = (client: GatewayClient | null, record: OperatorApprovalRecord): boolean => + canAccessOperatorApproval({ + client, + binding: { reviewerDeviceIds: record.reviewerDeviceIds }, + }); + + const authorizedRecipients = ( + sessionKey: string, + record: OperatorApprovalRecord, + ): ReadonlySet => { + const subscribed = params.sessionMessageSubscribers.getApprovals(sessionKey); + if (subscribed.size === 0) { + return subscribed; + } + const recipients = new Set(); + for (const client of params.clients) { + const connId = client.connId; + if ( + !client.invalidated && + connId && + subscribed.has(connId) && + canAccessRecord(client, record) + ) { + recipients.add(connId); + } + } + return recipients; + }; + + const publish = (event: OperatorApprovalLifecycleEvent): void => { + const approval = projectOperatorApprovalSnapshot(event.record, controlUiBasePath); + if (!approval || event.record.audienceSessionKeys.length === 0) { + return; + } + // The audience walk seeds the fully canonicalized source stream key as its + // first entry; publish that exact form so parents can correlate the event + // with a stream key they subscribed to. Raw source aliases (bare "global", + // "main", unscoped child keys) never reach subscribers. + const sourceStreamKey = + event.record.audienceSessionKeys[0] ?? + (event.record.source.sessionKey + ? resolveApprovalSourceStreamKey( + event.record.source.sessionKey, + event.record.source.agentId, + ) + : null); + for (const sessionKey of event.record.audienceSessionKeys) { + const recipients = authorizedRecipients(sessionKey, event.record); + if (recipients.size === 0) { + continue; + } + const common = { + sessionKey, + ...(sourceStreamKey ? { sourceSessionKey: sourceStreamKey } : {}), + updatedAtMs: event.record.updatedAtMs, + }; + let payload: SessionApprovalEvent; + if (event.phase === "pending") { + if (approval.status !== "pending") { + continue; + } + payload = { ...common, phase: "pending", approval }; + } else { + if (approval.status === "pending") { + continue; + } + payload = { ...common, phase: "terminal", approval }; + } + params.broadcastToConnIds("session.approval", payload, recipients); + } + }; + + return { + publish, + replay: (sessionKey, client) => { + const snapshotAtMs = now(); + const expired = expireDueOperatorApprovals({ + nowMs: snapshotAtMs, + databaseOptions: params.databaseOptions, + }); + // A replay read can be the first observer after a suspended timer. Emit + // the durable timeout tombstone before returning the authoritative set. + for (const record of expired.records) { + if (params.reconcileTerminal?.(record) !== true) { + publish({ phase: "terminal", record }); + } + } + const approvals: PendingApprovalSnapshot[] = []; + const records = listPendingOperatorApprovals({ + audienceSessionKey: sessionKey, + recordFilter: (record) => canAccessRecord(client, record), + limit: MAX_SESSION_APPROVAL_REPLAY + 1, + nowMs: snapshotAtMs, + databaseOptions: params.databaseOptions, + }); + const truncated = records.length > MAX_SESSION_APPROVAL_REPLAY; + for (const record of records) { + if (approvals.length === MAX_SESSION_APPROVAL_REPLAY) { + return { sessionKey, updatedAtMs: snapshotAtMs, approvals, truncated: true }; + } + const approval = projectOperatorApprovalSnapshot(record, controlUiBasePath); + if (approval?.status === "pending") { + approvals.push(approval); + } + } + return { sessionKey, updatedAtMs: snapshotAtMs, approvals, truncated }; + }, + }; +} diff --git a/src/gateway/operator-approval-snapshot.ts b/src/gateway/operator-approval-snapshot.ts new file mode 100644 index 000000000000..32bed4559ea3 --- /dev/null +++ b/src/gateway/operator-approval-snapshot.ts @@ -0,0 +1,38 @@ +import type { ApprovalSnapshot } from "../../packages/gateway-protocol/src/index.js"; +import type { OperatorApprovalRecord } from "./operator-approval-store.js"; + +/** Project one durable row into the reviewer-safe public approval shape. */ +export function projectOperatorApprovalSnapshot( + record: OperatorApprovalRecord, + controlUiBasePath: string, +): ApprovalSnapshot | null { + const common = { + id: record.id, + status: record.status, + presentation: record.presentation, + urlPath: `${controlUiBasePath}/approve/${encodeURIComponent(record.id)}`, + createdAtMs: record.createdAtMs, + expiresAtMs: record.expiresAtMs, + }; + if (record.status === "pending") { + return common as ApprovalSnapshot; + } + if (record.resolvedAtMs === null || record.terminalReason === null) { + return null; + } + const terminal = { + ...common, + resolvedAtMs: record.resolvedAtMs, + reason: record.terminalReason, + }; + if (record.status === "allowed") { + if (record.decision !== "allow-once" && record.decision !== "allow-always") { + return null; + } + return { ...terminal, decision: record.decision } as ApprovalSnapshot; + } + if (record.status === "denied") { + return { ...terminal, decision: "deny" } as ApprovalSnapshot; + } + return terminal as ApprovalSnapshot; +} diff --git a/src/gateway/operator-approval-store.test.ts b/src/gateway/operator-approval-store.test.ts index c8647e537c75..1d42f5daaa2b 100644 --- a/src/gateway/operator-approval-store.test.ts +++ b/src/gateway/operator-approval-store.test.ts @@ -179,6 +179,74 @@ describe("operator approval store", () => { ]); }); + it("filters an audience before applying the replay limit across scan pages", () => { + const databaseOptions = createDatabaseOptions(); + for (let index = 0; index < 256; index += 1) { + const id = `unrelated-${String(index).padStart(3, "0")}`; + expect( + insertOperatorApproval({ + approval: approval(id, { + audienceSessionKeys: ["agent:main:other"], + createdAtMs: 1_000 + index, + }), + databaseOptions, + }), + ).toMatchObject({ outcome: "inserted" }); + } + expect( + insertOperatorApproval({ + approval: approval("target-after-first-page", { + audienceSessionKeys: ["agent:main:target"], + createdAtMs: 2_000, + }), + databaseOptions, + }), + ).toMatchObject({ outcome: "inserted" }); + + expect( + listPendingOperatorApprovals({ + audienceSessionKey: "agent:main:target", + limit: 1, + nowMs: 3_000, + databaseOptions, + }), + ).toMatchObject([{ id: "target-after-first-page" }]); + }); + + it("applies a record filter before the replay limit across scan pages", () => { + const databaseOptions = createDatabaseOptions(); + for (let index = 0; index < 256; index += 1) { + const id = `unrelated-reviewer-${String(index).padStart(3, "0")}`; + expect( + insertOperatorApproval({ + approval: approval(id, { + reviewerDeviceIds: ["unrelated-device"], + createdAtMs: 1_000 + index, + }), + databaseOptions, + }), + ).toMatchObject({ outcome: "inserted" }); + } + expect( + insertOperatorApproval({ + approval: approval("authorized-after-first-page", { + reviewerDeviceIds: ["authorized-device"], + createdAtMs: 2_000, + }), + databaseOptions, + }), + ).toMatchObject({ outcome: "inserted" }); + + expect( + listPendingOperatorApprovals({ + recordFilter: (record) => record.reviewerDeviceIds.includes("authorized-device"), + limit: 1, + nowMs: 3_000, + databaseOptions, + }), + ).toMatchObject([{ id: "authorized-after-first-page" }]); + }); + it("reads the default clock after waiting for the SQLite write lock", async () => { const databaseOptions = createDatabaseOptions(); const createdAtMs = Date.now(); @@ -284,6 +352,23 @@ describe("operator approval store", () => { } }); + it("preserves protocol-valid boundary whitespace as opaque approval identity", () => { + const databaseOptions = createDatabaseOptions(); + for (const [index, id] of ["\uFEFF", "\u00A0", " approval-edge "].entries()) { + const inserted = insertOperatorApproval({ + approval: approval(id, { createdAtMs: 1_000 + index }), + databaseOptions, + }); + + expect(inserted).toMatchObject({ outcome: "inserted", record: { id } }); + expect(getOperatorApproval({ id, nowMs: 2_000, databaseOptions })).toMatchObject({ + id, + status: "pending", + }); + } + expect(getOperatorApproval({ id: "approval-edge", nowMs: 2_000, databaseOptions })).toBeNull(); + }); + it("keeps canonical ids and transport references in disjoint lookup namespaces", () => { const databaseOptions = createDatabaseOptions(); const inserted = insertOperatorApproval({ diff --git a/src/gateway/operator-approval-store.ts b/src/gateway/operator-approval-store.ts index 62a9087e1119..fa7cb15b86a2 100644 --- a/src/gateway/operator-approval-store.ts +++ b/src/gateway/operator-approval-store.ts @@ -26,6 +26,8 @@ import { export const OPERATOR_APPROVAL_TERMINAL_RETENTION_MS = 30 * 24 * 60 * 60_000; export const OPERATOR_APPROVAL_MAX_AUDIENCE_SESSION_KEYS = 64; +const OPERATOR_APPROVAL_PENDING_SCAN_PAGE_SIZE = 256; +const OPERATOR_APPROVAL_MAX_LIST_LIMIT = 1_001; export type OperatorApprovalKind = "exec" | "plugin"; export type OperatorApprovalStatus = "pending" | "allowed" | "denied" | "expired" | "cancelled"; @@ -719,6 +721,8 @@ export function listPendingOperatorApprovals( params: { kind?: OperatorApprovalKind; sourceSessionKey?: string; + audienceSessionKey?: string; + recordFilter?: (record: OperatorApprovalRecord) => boolean; limit?: number; nowMs?: number; databaseOptions?: OpenClawStateDatabaseOptions; @@ -728,34 +732,73 @@ export function listPendingOperatorApprovals( return runOpenClawStateWriteTransaction((database) => { const nowMs = params.nowMs ?? Date.now(); const stateDb = getNodeSqliteKysely(database.db); - let query = stateDb - .selectFrom("operator_approvals") - .selectAll() - .where("status", "=", "pending") - .where("expires_at_ms", ">", nowMs) - .orderBy("created_at_ms", "asc") - .orderBy("approval_id", "asc") - .limit(Math.max(1, Math.min(params.limit ?? 1_000, 1_000))); - if (params.kind) { - query = query.where("kind", "=", params.kind); - } - if (params.sourceSessionKey) { - query = query.where("source_session_key", "=", params.sourceSessionKey); - } - const rows = executeSqliteQuerySync(database.db, query).rows; + const resultLimit = Math.max( + 1, + Math.min(params.limit ?? 1_000, OPERATOR_APPROVAL_MAX_LIST_LIMIT), + ); + const audienceSessionKey = + params.audienceSessionKey === undefined + ? undefined + : requireString(params.audienceSessionKey, "operator approval audience session key"); + const requiresPostFilter = + audienceSessionKey !== undefined || params.recordFilter !== undefined; const records: OperatorApprovalRecord[] = []; - for (const row of rows) { - const record = decodeOperatorApprovalRow(row); - if (record) { - records.push(record); - } else { - denyCorruptPendingRow({ - database, - id: row.approval_id, - nowMs, - createdAtMs: row.created_at_ms, - }); + let cursor: { createdAtMs: number; id: string } | undefined; + // Audience and reviewer bindings live in validated bounded JSON. Keyset-scan + // first, then apply the limit so unrelated records cannot starve replay. + while (records.length < resultLimit) { + let query = stateDb + .selectFrom("operator_approvals") + .selectAll() + .where("status", "=", "pending") + .where("expires_at_ms", ">", nowMs) + .orderBy("created_at_ms", "asc") + .orderBy("approval_id", "asc") + .limit(requiresPostFilter ? OPERATOR_APPROVAL_PENDING_SCAN_PAGE_SIZE : resultLimit); + if (params.kind) { + query = query.where("kind", "=", params.kind); } + if (params.sourceSessionKey) { + query = query.where("source_session_key", "=", params.sourceSessionKey); + } + if (cursor) { + const pageCursor = cursor; + query = query.where((eb) => + eb.or([ + eb("created_at_ms", ">", pageCursor.createdAtMs), + eb.and([ + eb("created_at_ms", "=", pageCursor.createdAtMs), + eb("approval_id", ">", pageCursor.id), + ]), + ]), + ); + } + const rows = executeSqliteQuerySync(database.db, query).rows; + for (const row of rows) { + const record = decodeOperatorApprovalRow(row); + if (!record) { + denyCorruptPendingRow({ + database, + id: row.approval_id, + nowMs, + createdAtMs: row.created_at_ms, + }); + continue; + } + const matchesAudience = + !audienceSessionKey || record.audienceSessionKeys.includes(audienceSessionKey); + if (matchesAudience && (!params.recordFilter || params.recordFilter(record))) { + records.push(record); + if (records.length === resultLimit) { + break; + } + } + } + const last = rows.at(-1); + if (!requiresPostFilter || rows.length < OPERATOR_APPROVAL_PENDING_SCAN_PAGE_SIZE || !last) { + break; + } + cursor = { createdAtMs: last.created_at_ms, id: last.approval_id }; } return records; }, params.databaseOptions); diff --git a/src/gateway/server-aux-handlers.ts b/src/gateway/server-aux-handlers.ts index 42f9f4fc9038..eb5e7e385ae8 100644 --- a/src/gateway/server-aux-handlers.ts +++ b/src/gateway/server-aux-handlers.ts @@ -8,10 +8,8 @@ import { resolveExecApprovalRequestAllowedDecisions, type ExecApprovalRequestPayload, } from "../infra/exec-approvals.js"; -import { - resolvePluginApprovalRequestAllowedDecisions, - type PluginApprovalRequestPayload, -} from "../infra/plugin-approvals.js"; +import { resolveCanonicalPluginApprovalRequestAllowedDecisions } from "../infra/plugin-approval-canonical-decisions.js"; +import type { PluginApprovalRequestPayload } from "../infra/plugin-approvals.js"; import { resolveCommandSecretsFromActiveRuntimeSnapshot, type CommandSecretAssignment, @@ -21,7 +19,7 @@ import { type PreparedSecretsRuntimeSnapshot, } from "../secrets/runtime-state.js"; import { createLazyPromise } from "../shared/lazy-runtime.js"; -import { resolveApprovalSessionAudience } from "./approval-session-audience.js"; +import { resolveApprovalSessionAudienceWithFallback } from "./approval-session-audience.js"; import { diffConfigPaths } from "./config-diff.js"; import { buildGatewayReloadPlan, @@ -29,7 +27,10 @@ import { type GatewayReloadPlan, } from "./config-reload-plan.js"; import { createExecApprovalIosPushDelivery } from "./exec-approval-ios-push.js"; -import { ExecApprovalManager } from "./exec-approval-manager.js"; +import { + ExecApprovalManager, + type OperatorApprovalLifecycleEvent, +} from "./exec-approval-manager.js"; import { closeOrphanedOperatorApprovals, pruneTerminalOperatorApprovals, @@ -88,6 +89,7 @@ export function createGatewayAuxHandlers(params: { stopChannel: (name: ChannelKind) => Promise; getChannelAutostartSuppression?: () => ChannelAutostartSuppression | null; logChannels: { info: (msg: string) => void }; + onApprovalLifecycle?: (event: OperatorApprovalLifecycleEvent) => void; }) { // Both approval kinds share one durable first-answer-wins registry and // Gateway-lifetime epoch while retaining separate in-process waiter maps. @@ -103,8 +105,9 @@ export function createGatewayAuxHandlers(params: { const execApprovalManager = new ExecApprovalManager({ approvalKind: "exec", persistence: approvalPersistence, - resolveAudienceSessionKeys: resolveApprovalSessionAudience, + resolveAudienceSessionKeys: resolveApprovalSessionAudienceWithFallback, resolveAllowedDecisions: resolveExecApprovalRequestAllowedDecisions, + onLifecycle: params.onApprovalLifecycle, onError: (error, context) => { params.log.error?.( `${context.approvalKind} approval ${context.operation} failed for ${context.approvalId}: ${String(error)}`, @@ -127,8 +130,9 @@ export function createGatewayAuxHandlers(params: { const pluginApprovalManager = new ExecApprovalManager({ approvalKind: "plugin", persistence: approvalPersistence, - resolveAudienceSessionKeys: resolveApprovalSessionAudience, - resolveAllowedDecisions: (request) => resolvePluginApprovalRequestAllowedDecisions(request), + resolveAudienceSessionKeys: resolveApprovalSessionAudienceWithFallback, + resolveAllowedDecisions: resolveCanonicalPluginApprovalRequestAllowedDecisions, + onLifecycle: params.onApprovalLifecycle, onError: (error, context) => { params.log.error?.( `${context.approvalKind} approval ${context.operation} failed for ${context.approvalId}: ${String(error)}`, diff --git a/src/gateway/server-broadcast.ts b/src/gateway/server-broadcast.ts index 6ea44805d8e3..6b9724f6004e 100644 --- a/src/gateway/server-broadcast.ts +++ b/src/gateway/server-broadcast.ts @@ -47,6 +47,7 @@ const EVENT_SCOPE_GUARDS: Record = { "node.pair.requested": [PAIRING_SCOPE], "node.pair.resolved": [PAIRING_SCOPE], "sessions.changed": [READ_SCOPE], + "session.approval": [APPROVALS_SCOPE], "session.message": [READ_SCOPE], "session.operation": [READ_SCOPE], "session.tool": [READ_SCOPE], diff --git a/src/gateway/server-chat-state.test.ts b/src/gateway/server-chat-state.test.ts new file mode 100644 index 000000000000..26510d0f02dd --- /dev/null +++ b/src/gateway/server-chat-state.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { createSessionMessageSubscriberRegistry } from "./server-chat-state.js"; + +describe("createSessionMessageSubscriberRegistry", () => { + it("keeps approval delivery opt-in and updates it on resubscribe", () => { + const subscribers = createSessionMessageSubscriberRegistry(); + + subscribers.subscribe("conn-plain", "agent:main:main"); + subscribers.subscribe("conn-reviewer", "agent:main:main", { includeApprovals: true }); + + expect([...subscribers.get("agent:main:main")]).toEqual(["conn-plain", "conn-reviewer"]); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual(["conn-reviewer"]); + + subscribers.subscribe("conn-reviewer", "agent:main:main"); + expect([...subscribers.get("agent:main:main")]).toEqual(["conn-plain", "conn-reviewer"]); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual([]); + + subscribers.subscribe("conn-reviewer", "agent:main:main", { includeApprovals: true }); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual(["conn-reviewer"]); + + subscribers.unsubscribe("conn-reviewer", "agent:main:main"); + expect([...subscribers.get("agent:main:main")]).toEqual(["conn-plain"]); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual([]); + }); + + it("removes approval subscriptions through connection cleanup and registry reset", () => { + const subscribers = createSessionMessageSubscriberRegistry(); + + subscribers.subscribe("conn-reviewer", "agent:main:main", { includeApprovals: true }); + subscribers.subscribe("conn-reviewer", "agent:main:child", { includeApprovals: true }); + subscribers.subscribe("conn-other", "agent:main:child", { includeApprovals: true }); + + subscribers.unsubscribeAll("conn-reviewer"); + expect([...subscribers.get("agent:main:main")]).toEqual([]); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual([]); + expect([...subscribers.get("agent:main:child")]).toEqual(["conn-other"]); + expect([...subscribers.getApprovals("agent:main:child")]).toEqual(["conn-other"]); + + subscribers.clear(); + expect([...subscribers.get("agent:main:child")]).toEqual([]); + expect([...subscribers.getApprovals("agent:main:child")]).toEqual([]); + }); + + it("rolls a provisional subscription back to its exact prior state", () => { + const subscribers = createSessionMessageSubscriberRegistry(); + + const removeNew = subscribers.subscribe("conn-new", "agent:main:main", { + includeApprovals: true, + }); + removeNew?.(); + expect([...subscribers.get("agent:main:main")]).toEqual([]); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual([]); + + subscribers.subscribe("conn-plain", "agent:main:main"); + const restorePlain = subscribers.subscribe("conn-plain", "agent:main:main", { + includeApprovals: true, + }); + restorePlain?.(); + expect([...subscribers.get("agent:main:main")]).toEqual(["conn-plain"]); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual([]); + + subscribers.subscribe("conn-reviewer", "agent:main:main", { includeApprovals: true }); + const restoreReviewer = subscribers.subscribe("conn-reviewer", "agent:main:main"); + restoreReviewer?.(); + expect([...subscribers.getApprovals("agent:main:main")]).toEqual(["conn-reviewer"]); + }); +}); diff --git a/src/gateway/server-chat-state.ts b/src/gateway/server-chat-state.ts index a05813e4195e..c2fbe726a3e0 100644 --- a/src/gateway/server-chat-state.ts +++ b/src/gateway/server-chat-state.ts @@ -229,10 +229,15 @@ export type SessionEventSubscriberRegistry = { }; export type SessionMessageSubscriberRegistry = { - subscribe: (connId: string, sessionKey: string) => void; + subscribe: ( + connId: string, + sessionKey: string, + opts?: { includeApprovals?: boolean }, + ) => (() => void) | undefined; unsubscribe: (connId: string, sessionKey: string) => void; unsubscribeAll: (connId: string) => void; get: (sessionKey: string) => ReadonlySet; + getApprovals: (sessionKey: string) => ReadonlySet; clear: () => void; }; @@ -276,17 +281,23 @@ export function createSessionEventSubscriberRegistry(): SessionEventSubscriberRe export function createSessionMessageSubscriberRegistry(): SessionMessageSubscriberRegistry { const sessionToConnIds = new Map>(); const connToSessionKeys = new Map>(); + const approvalSessionToConnIds = new Map>(); + const connToApprovalSessionKeys = new Map>(); const empty = new Set(); const normalize = (value: string): string => value.trim(); - return { - subscribe: (connId: string, sessionKey: string) => { + const registry: SessionMessageSubscriberRegistry = { + subscribe: (connId: string, sessionKey: string, opts) => { const normalizedConnId = normalize(connId); const normalizedSessionKey = normalize(sessionKey); if (!normalizedConnId || !normalizedSessionKey) { return; } + const hadMessages = + sessionToConnIds.get(normalizedSessionKey)?.has(normalizedConnId) ?? false; + const hadApprovals = + approvalSessionToConnIds.get(normalizedSessionKey)?.has(normalizedConnId) ?? false; const connIds = sessionToConnIds.get(normalizedSessionKey) ?? new Set(); connIds.add(normalizedConnId); sessionToConnIds.set(normalizedSessionKey, connIds); @@ -294,6 +305,42 @@ export function createSessionMessageSubscriberRegistry(): SessionMessageSubscrib const sessionKeys = connToSessionKeys.get(normalizedConnId) ?? new Set(); sessionKeys.add(normalizedSessionKey); connToSessionKeys.set(normalizedConnId, sessionKeys); + + if (opts?.includeApprovals) { + const approvalConnIds = + approvalSessionToConnIds.get(normalizedSessionKey) ?? new Set(); + approvalConnIds.add(normalizedConnId); + approvalSessionToConnIds.set(normalizedSessionKey, approvalConnIds); + + const approvalSessionKeys = + connToApprovalSessionKeys.get(normalizedConnId) ?? new Set(); + approvalSessionKeys.add(normalizedSessionKey); + connToApprovalSessionKeys.set(normalizedConnId, approvalSessionKeys); + } else { + const approvalConnIds = approvalSessionToConnIds.get(normalizedSessionKey); + approvalConnIds?.delete(normalizedConnId); + if (approvalConnIds?.size === 0) { + approvalSessionToConnIds.delete(normalizedSessionKey); + } + const approvalSessionKeys = connToApprovalSessionKeys.get(normalizedConnId); + approvalSessionKeys?.delete(normalizedSessionKey); + if (approvalSessionKeys?.size === 0) { + connToApprovalSessionKeys.delete(normalizedConnId); + } + } + // Replay setup subscribes before reading its snapshot. Preserve the exact + // prior state so a failed read cannot leave a ghost or remove a retry. + return () => { + if (!hadMessages) { + registry.unsubscribe(normalizedConnId, normalizedSessionKey); + return; + } + registry.subscribe( + normalizedConnId, + normalizedSessionKey, + hadApprovals ? { includeApprovals: true } : undefined, + ); + }; }, unsubscribe: (connId: string, sessionKey: string) => { const normalizedConnId = normalize(connId); @@ -315,6 +362,20 @@ export function createSessionMessageSubscriberRegistry(): SessionMessageSubscrib connToSessionKeys.delete(normalizedConnId); } } + const approvalConnIds = approvalSessionToConnIds.get(normalizedSessionKey); + if (approvalConnIds) { + approvalConnIds.delete(normalizedConnId); + if (approvalConnIds.size === 0) { + approvalSessionToConnIds.delete(normalizedSessionKey); + } + } + const approvalSessionKeys = connToApprovalSessionKeys.get(normalizedConnId); + if (approvalSessionKeys) { + approvalSessionKeys.delete(normalizedSessionKey); + if (approvalSessionKeys.size === 0) { + connToApprovalSessionKeys.delete(normalizedConnId); + } + } }, unsubscribeAll: (connId: string) => { const normalizedConnId = normalize(connId); @@ -336,6 +397,16 @@ export function createSessionMessageSubscriberRegistry(): SessionMessageSubscrib } } connToSessionKeys.delete(normalizedConnId); + + const approvalSessionKeys = connToApprovalSessionKeys.get(normalizedConnId); + for (const sessionKey of approvalSessionKeys ?? []) { + const connIds = approvalSessionToConnIds.get(sessionKey); + connIds?.delete(normalizedConnId); + if (connIds?.size === 0) { + approvalSessionToConnIds.delete(sessionKey); + } + } + connToApprovalSessionKeys.delete(normalizedConnId); }, get: (sessionKey: string) => { const normalizedSessionKey = normalize(sessionKey); @@ -344,11 +415,21 @@ export function createSessionMessageSubscriberRegistry(): SessionMessageSubscrib } return sessionToConnIds.get(normalizedSessionKey) ?? empty; }, + getApprovals: (sessionKey: string) => { + const normalizedSessionKey = normalize(sessionKey); + if (!normalizedSessionKey) { + return empty; + } + return approvalSessionToConnIds.get(normalizedSessionKey) ?? empty; + }, clear: () => { sessionToConnIds.clear(); connToSessionKeys.clear(); + approvalSessionToConnIds.clear(); + connToApprovalSessionKeys.clear(); }, }; + return registry; } /** Create the run-id recipient registry used for streaming tool events. */ diff --git a/src/gateway/server-methods-list.ts b/src/gateway/server-methods-list.ts index 014e4c2fb258..257312f48a65 100644 --- a/src/gateway/server-methods-list.ts +++ b/src/gateway/server-methods-list.ts @@ -40,6 +40,7 @@ export const GATEWAY_EVENTS = [ "connect.challenge", "agent", "chat", + "session.approval", "session.message", "session.operation", "session.tool", diff --git a/src/gateway/server-methods/sessions.messages-subscribe-approvals.test.ts b/src/gateway/server-methods/sessions.messages-subscribe-approvals.test.ts new file mode 100644 index 000000000000..e5977d22ff96 --- /dev/null +++ b/src/gateway/server-methods/sessions.messages-subscribe-approvals.test.ts @@ -0,0 +1,254 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { SessionApprovalReplay } from "../../../packages/gateway-protocol/src/index.js"; +import type { + GatewayClient, + GatewayRequestContext, + GatewayRequestHandlerOptions, +} from "./types.js"; + +const loadSessionEntryMock = vi.fn((sessionKey: string, _opts?: { agentId?: string }) => ({ + canonicalKey: sessionKey, +})); + +vi.mock("../session-utils.js", async () => { + const actual = await vi.importActual("../session-utils.js"); + return { + ...actual, + loadSessionEntry: (...args: unknown[]) => + loadSessionEntryMock(...(args as [string, { agentId?: string }?])), + }; +}); + +import { sessionsHandlers } from "./sessions.js"; + +function createClient(params: { + scopes: string[]; + deviceId?: string; + connId?: string; +}): GatewayClient { + return { + connId: params.connId ?? "conn-approval-reviewer", + connect: { + client: { id: "approval-subscribe-test", displayName: "Approval Subscribe Test" }, + scopes: params.scopes, + ...(params.deviceId ? { device: { id: params.deviceId } } : {}), + }, + } as unknown as GatewayClient; +} + +function createContext(params: { + replay?: SessionApprovalReplay; + replayError?: Error; + globalScope?: boolean; + agents?: Array<{ id: string; default?: boolean }>; +}) { + const rollbackSubscription = vi.fn(); + const subscribeSessionMessageEvents = vi.fn(() => rollbackSubscription); + const listSessionPendingApprovals = vi.fn(() => { + if (params.replayError) { + throw params.replayError; + } + return params.replay; + }); + const logError = vi.fn(); + const context = { + getRuntimeConfig: () => ({ + agents: { list: params.agents ?? [{ id: "main", default: true }] }, + ...(params.globalScope ? { session: { scope: "global" as const } } : {}), + }), + listSessionPendingApprovals, + logGateway: { error: logError }, + subscribeSessionMessageEvents, + } as unknown as GatewayRequestContext; + return { + context, + listSessionPendingApprovals, + logError, + rollbackSubscription, + subscribeSessionMessageEvents, + }; +} + +async function subscribe(params: { + body: Record; + client: GatewayClient; + context: GatewayRequestContext; +}) { + const respond = vi.fn(); + await sessionsHandlers["sessions.messages.subscribe"]({ + req: { id: "req-subscribe-approvals" } as never, + params: params.body, + respond, + context: params.context, + client: params.client, + isWebchatConnect: () => false, + } satisfies GatewayRequestHandlerOptions); + return respond; +} + +describe("sessions.messages.subscribe approval opt-in", () => { + beforeEach(() => { + loadSessionEntryMock.mockReset(); + loadSessionEntryMock.mockImplementation((sessionKey: string) => ({ canonicalKey: sessionKey })); + }); + + it("allows an admin without a paired device and uses the exact scoped subscription key", async () => { + loadSessionEntryMock.mockReturnValueOnce({ canonicalKey: "global" }); + const approvalReplay = { + sessionKey: "agent:work:global", + updatedAtMs: 42, + approvals: [], + truncated: false, + } satisfies SessionApprovalReplay; + const { context, listSessionPendingApprovals, subscribeSessionMessageEvents } = createContext({ + replay: approvalReplay, + globalScope: true, + agents: [{ id: "main", default: true }, { id: "work" }], + }); + + const respond = await subscribe({ + body: { key: "agent:work:main", includeApprovals: true }, + client: createClient({ scopes: ["operator.admin"], connId: " conn-admin " }), + context, + }); + + expect(listSessionPendingApprovals).toHaveBeenCalledWith( + "agent:work:global", + expect.objectContaining({ connId: " conn-admin " }), + ); + expect(subscribeSessionMessageEvents.mock.invocationCallOrder[0]).toBeLessThan( + listSessionPendingApprovals.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY, + ); + expect(subscribeSessionMessageEvents).toHaveBeenCalledWith("conn-admin", "agent:work:global", { + includeApprovals: true, + }); + expect(respond).toHaveBeenCalledWith( + true, + { subscribed: true, key: "global", approvalReplay }, + undefined, + ); + }); + + it("allows a paired device with approval scope", async () => { + loadSessionEntryMock.mockReturnValueOnce({ canonicalKey: "agent:main:child" }); + const approvalReplay = { + sessionKey: "agent:main:child", + updatedAtMs: 43, + approvals: [], + truncated: false, + } satisfies SessionApprovalReplay; + const { context, subscribeSessionMessageEvents } = createContext({ replay: approvalReplay }); + + const respond = await subscribe({ + body: { key: "child", includeApprovals: true }, + client: createClient({ scopes: ["operator.approvals"], deviceId: "phone" }), + context, + }); + + expect(subscribeSessionMessageEvents).toHaveBeenCalledWith( + "conn-approval-reviewer", + "agent:main:child", + { includeApprovals: true }, + ); + expect(respond).toHaveBeenCalledWith( + true, + { subscribed: true, key: "agent:main:child", approvalReplay }, + undefined, + ); + }); + + it.each([ + { + name: "approval scope without a paired device", + client: createClient({ scopes: ["operator.approvals"] }), + }, + { + name: "paired device without approval authority", + client: createClient({ scopes: ["operator.read"], deviceId: "phone" }), + }, + ])("rejects $name", async ({ client }) => { + const { context, listSessionPendingApprovals, subscribeSessionMessageEvents } = createContext( + {}, + ); + + const respond = await subscribe({ + body: { key: "agent:main:child", includeApprovals: true }, + client, + context, + }); + + expect(listSessionPendingApprovals).not.toHaveBeenCalled(); + expect(subscribeSessionMessageEvents).not.toHaveBeenCalled(); + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + code: "INVALID_REQUEST", + message: expect.stringContaining("operator.approvals"), + }), + ); + }); + + it("keeps the non-approval response shape and skips replay", async () => { + loadSessionEntryMock.mockReturnValueOnce({ canonicalKey: "agent:main:child" }); + const { context, listSessionPendingApprovals, subscribeSessionMessageEvents } = createContext( + {}, + ); + + const respond = await subscribe({ + body: { key: "child" }, + client: createClient({ scopes: ["operator.read"] }), + context, + }); + + expect(listSessionPendingApprovals).not.toHaveBeenCalled(); + expect(subscribeSessionMessageEvents).toHaveBeenCalled(); + expect(subscribeSessionMessageEvents.mock.calls[0]?.slice(0, 2)).toEqual([ + "conn-approval-reviewer", + "agent:main:child", + ]); + expect(respond).toHaveBeenCalledWith( + true, + { subscribed: true, key: "agent:main:child" }, + undefined, + ); + expect(respond.mock.calls[0]?.[1]).not.toHaveProperty("approvalReplay"); + }); + + it.each([ + { name: "throws", replayError: new Error("database unavailable") }, + { name: "returns no snapshot", replayError: undefined }, + ])("restores the prior subscription when replay $name", async ({ replayError }) => { + const { + context, + listSessionPendingApprovals, + logError, + rollbackSubscription, + subscribeSessionMessageEvents, + } = createContext({ replayError }); + + const respond = await subscribe({ + body: { key: "agent:main:child", includeApprovals: true }, + client: createClient({ scopes: ["operator.admin"] }), + context, + }); + + expect(subscribeSessionMessageEvents).toHaveBeenCalledWith( + "conn-approval-reviewer", + "agent:main:child", + { includeApprovals: true }, + ); + expect(subscribeSessionMessageEvents.mock.invocationCallOrder[0]).toBeLessThan( + listSessionPendingApprovals.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY, + ); + expect(rollbackSubscription).toHaveBeenCalledTimes(1); + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ code: "UNAVAILABLE" }), + ); + if (replayError) { + expect(logError).toHaveBeenCalledWith(expect.stringContaining("database unavailable")); + } + }); +}); diff --git a/src/gateway/server-methods/sessions.ts b/src/gateway/server-methods/sessions.ts index 40276f9baa7d..45f015b35cb1 100644 --- a/src/gateway/server-methods/sessions.ts +++ b/src/gateway/server-methods/sessions.ts @@ -92,7 +92,8 @@ import { recordSessionCompacted, } from "../../sessions/session-state-events.js"; import { createLazyRuntimeModule } from "../../shared/lazy-runtime.js"; -import { ADMIN_SCOPE } from "../operator-scopes.js"; +import { canReviewOperatorApproval } from "../operator-approval-authorization.js"; +import { ADMIN_SCOPE, APPROVALS_SCOPE } from "../operator-scopes.js"; import { resolveSessionKeyForRun } from "../server-session-key.js"; import { createFileBackedCompactionCheckpointStore, @@ -998,6 +999,17 @@ export const sessionsHandlers: GatewayRequestHandlers = { if (!key) { return; } + if (p.includeApprovals === true && !canReviewOperatorApproval(client)) { + respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + `sessions.messages.subscribe includeApprovals requires a paired device and gateway scope: ${APPROVALS_SCOPE}`, + ), + ); + return; + } const cfg = context.getRuntimeConfig(); const requestedAgent = resolveRequestedGlobalAgentId(cfg, key, p.agentId); if (!requestedAgent.ok) { @@ -1012,8 +1024,52 @@ export const sessionsHandlers: GatewayRequestHandlers = { defaultAgentId: resolveDefaultAgentId(cfg), }); if (connId) { - context.subscribeSessionMessageEvents(connId, subscriptionKey); - respond(true, { subscribed: true, key: canonicalKey }, undefined); + let approvalReplay; + if (p.includeApprovals === true) { + // Subscribe before the authoritative snapshot so a transition cannot + // land between replay and live delivery. Clients reconcile by id. + const rollbackSubscription = context.subscribeSessionMessageEvents( + connId, + subscriptionKey, + { includeApprovals: true }, + ); + try { + approvalReplay = context.listSessionPendingApprovals?.(subscriptionKey, client); + } catch (error) { + rollbackSubscription?.(); + context.logGateway.error(`session approval replay failed: ${String(error)}`); + respond( + false, + undefined, + errorShape(ErrorCodes.UNAVAILABLE, "session approval replay unavailable"), + ); + return; + } + if (!approvalReplay) { + rollbackSubscription?.(); + respond( + false, + undefined, + errorShape(ErrorCodes.UNAVAILABLE, "session approval replay unavailable"), + ); + return; + } + } else { + context.subscribeSessionMessageEvents(connId, subscriptionKey); + } + respond( + true, + { + subscribed: true, + key: canonicalKey, + ...(p.includeApprovals === true + ? { + approvalReplay, + } + : {}), + }, + undefined, + ); return; } respond(true, { subscribed: false, key: canonicalKey }, undefined); diff --git a/src/gateway/server-methods/shared-types.ts b/src/gateway/server-methods/shared-types.ts index 2750a9a4a929..0fa72d55acc7 100644 --- a/src/gateway/server-methods/shared-types.ts +++ b/src/gateway/server-methods/shared-types.ts @@ -1,3 +1,4 @@ +import type { SessionApprovalReplay } from "../../../packages/gateway-protocol/src/index.js"; // Shared server-method types define the client, context, response, and handler // contracts used by every gateway RPC method module. import type { @@ -95,6 +96,10 @@ export type GatewayRequestContext = { execApprovalManager?: ExecApprovalManager; pluginApprovalManager?: ExecApprovalManager; forwardPluginApprovalRequest?: (request: PluginApprovalRequest) => Promise; + listSessionPendingApprovals?: ( + sessionKey: string, + client: GatewayClient | null, + ) => SessionApprovalReplay; loadGatewayModelCatalog: (params?: { readOnly?: boolean }) => Promise; loadGatewayModelCatalogSnapshot: (params?: { readOnly?: boolean; @@ -156,7 +161,11 @@ export type GatewayRequestContext = { ) => ChatRunEntry | undefined; subscribeSessionEvents: (connId: string) => void; unsubscribeSessionEvents: (connId: string) => void; - subscribeSessionMessageEvents: (connId: string, sessionKey: string) => void; + subscribeSessionMessageEvents: ( + connId: string, + sessionKey: string, + opts?: { includeApprovals?: boolean }, + ) => (() => void) | undefined; unsubscribeSessionMessageEvents: (connId: string, sessionKey: string) => void; unsubscribeAllSessionEvents: (connId: string) => void; getSessionEventSubscriberConnIds: () => ReadonlySet; diff --git a/src/gateway/server-request-context.test.ts b/src/gateway/server-request-context.test.ts index 99a004289f55..9bf529361686 100644 --- a/src/gateway/server-request-context.test.ts +++ b/src/gateway/server-request-context.test.ts @@ -30,6 +30,7 @@ function makeContextParams( isTerminalEnabled: vi.fn(() => false), execApprovalManager: undefined, pluginApprovalManager: undefined, + listSessionPendingApprovals: undefined, loadGatewayModelCatalog: vi.fn(async () => []), loadGatewayModelCatalogSnapshot: vi.fn(async () => ({ entries: [], routeVariants: [] })), getHealthCache: vi.fn(() => null), diff --git a/src/gateway/server-request-context.ts b/src/gateway/server-request-context.ts index 319030bd0477..60044e983202 100644 --- a/src/gateway/server-request-context.ts +++ b/src/gateway/server-request-context.ts @@ -21,6 +21,7 @@ export type GatewayRequestContextParams = { execApprovalManager: GatewayRequestContext["execApprovalManager"]; forwardPluginApprovalRequest?: GatewayRequestContext["forwardPluginApprovalRequest"]; pluginApprovalManager: GatewayRequestContext["pluginApprovalManager"]; + listSessionPendingApprovals: GatewayRequestContext["listSessionPendingApprovals"]; loadGatewayModelCatalog: GatewayRequestContext["loadGatewayModelCatalog"]; loadGatewayModelCatalogSnapshot: GatewayRequestContext["loadGatewayModelCatalogSnapshot"]; getHealthCache: GatewayRequestContext["getHealthCache"]; @@ -107,6 +108,7 @@ export function createGatewayRequestContext( execApprovalManager: params.execApprovalManager, forwardPluginApprovalRequest: params.forwardPluginApprovalRequest, pluginApprovalManager: params.pluginApprovalManager, + listSessionPendingApprovals: params.listSessionPendingApprovals, loadGatewayModelCatalog: params.loadGatewayModelCatalog, loadGatewayModelCatalogSnapshot: params.loadGatewayModelCatalogSnapshot, getHealthCache: params.getHealthCache, diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 71fb43fd1d7c..eaabffe41102 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -40,6 +40,7 @@ import { } from "../infra/diagnostics-timeline.js"; import { isTruthyEnvValue, isVitestRuntimeEnv, logAcceptedEnvOption } from "../infra/env.js"; import { ensureOpenClawCliOnPath } from "../infra/path-env.js"; +import type { PluginApprovalRequestPayload } from "../infra/plugin-approvals.js"; import { readGatewayRestartHandoffSync } from "../infra/restart-handoff.js"; import { setGatewaySigusr1RestartPolicy, setPreRestartDeferralCheck } from "../infra/restart.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; @@ -73,6 +74,7 @@ import { recordRemoteNodeInfo, removeRemoteNodeInfo } from "../skills/runtime/re import { createAuthRateLimiter, type AuthRateLimiter } from "./auth-rate-limit.js"; import { resolveGatewayAuth } from "./auth.js"; import type { RestartRecoveryCandidate } from "./chat-abort.js"; +import type { ExecApprovalManager } from "./exec-approval-manager.js"; import { ADMIN_SCOPE } from "./method-scopes.js"; import { STARTUP_UNAVAILABLE_GATEWAY_METHODS, @@ -1391,6 +1393,30 @@ export async function startGatewayServer( ); Object.assign(runtimeState, runtimeServices); + const { createOperatorApprovalSessionEventRuntime } = + await import("./operator-approval-session-events.js"); + // Managers publish through this runtime, while replay routes durable + // expiry back through the owning manager to release its parked waiter once. + let approvalManagersForReplay: + | { + exec: ExecApprovalManager; + plugin: ExecApprovalManager; + } + | undefined; + const approvalSessionEvents = createOperatorApprovalSessionEventRuntime({ + clients, + sessionMessageSubscribers, + broadcastToConnIds, + controlUiBasePath, + reconcileTerminal: (record) => { + const manager = + record.kind === "exec" + ? approvalManagersForReplay?.exec + : approvalManagersForReplay?.plugin; + return manager?.reconcileDurableTerminal(record) ?? false; + }, + }); + const { execApprovalManager, forwardPluginApprovalRequest, @@ -1411,10 +1437,12 @@ export async function startGatewayServer( stopChannel, getChannelAutostartSuppression: channelManager.getAutostartSuppression, logChannels, + onApprovalLifecycle: approvalSessionEvents.publish, }), coreGatewayHandlers: coreGatewayHandlersLocal, }; }); + approvalManagersForReplay = { exec: execApprovalManager, plugin: pluginApprovalManager }; const attachedGatewayExtraHandlers: GatewayRequestHandlers = { ...pluginRegistry.gatewayHandlers, ...extraHandlers, @@ -1652,6 +1680,7 @@ export async function startGatewayServer( execApprovalManager, forwardPluginApprovalRequest, pluginApprovalManager, + listSessionPendingApprovals: approvalSessionEvents.replay, loadGatewayModelCatalog, loadGatewayModelCatalogSnapshot, getHealthCache,