mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-19 01:50:51 -05:00
fix(craft): preserve approval outcomes and report expired requests
This commit is contained in:
1 parent
b9d2816087
commit
10f195f3e0
35 files changed
+1137
-508
No files matched your search
@@ -122,6 +122,7 @@ here. Read the component that documents the setting in its env table.
|
||||
| `backend/ee/onyx/server/enterprise_settings/` | whitelabelling-and-theme |
|
||||
| `backend/ee/onyx/db/standard_answer.py`, `backend/ee/onyx/server/manage/standard_answer.py`, `*/onyxbot/slack/handlers/handle_standard_answers.py` | standard-answers |
|
||||
| `backend/onyx/db/notification.py`, `db/release_notes.py`, `db/admin_banner.py`, `server/features/notifications/`, `features/release_notes/`, `features/admin_banner/` | notifications |
|
||||
| `backend/onyx/db/action_approval.py` | craft-external-apps, craft-streaming |
|
||||
| `backend/onyx/db/action_policy.py`, `backend/onyx/server/features/build/action_policies.py`, `web/src/app/craft/v1/apps/action-policies/` | craft-external-apps, craft-scheduled-tasks, craft-admin, editions-and-gating |
|
||||
| `backend/onyx/db/models.py` | **any**. Check which tables the diff touches, then map those. |
|
||||
| `backend/alembic/`, `backend/alembic_tenants/` | the component that owns the table being migrated |
|
||||
|
||||
@@ -23,7 +23,7 @@
|
||||
`backend/onyx/db/external_app.py`, `backend/onyx/db/action_policy.py`,
|
||||
`backend/onyx/server/features/build/external_apps/` (`api.py`, `models.py`,
|
||||
`oauth.py`), `backend/onyx/server/features/build/approvals/api.py`,
|
||||
`backend/onyx/server/features/build/db/action_approval.py`
|
||||
`backend/onyx/db/action_approval.py`
|
||||
|
||||
**Read first:** `[[craft-admin]]` for the admin surface that configures what
|
||||
this document enforces, then
|
||||
@@ -72,9 +72,10 @@ sync (`server.py:_Readiness`).
|
||||
|---|---|---|---|
|
||||
| GET | `/api/build/apps/{id}/oauth/start` | `start_external_app_oauth` (`server/features/build/external_apps/oauth.py`) | Begins a user's per-user OAuth connect flow for a built-in app. |
|
||||
| POST | `/api/build/apps/oauth/callback` | `handle_external_app_oauth_callback` (`oauth.py`) | Exchanges the authorization code, stamps `expires_at`, stores per-user credentials. |
|
||||
| GET | `/api/build/approvals/sessions/{session_id}/live` | `list_live_approvals` (`server/features/build/approvals/api.py`) | Polled by the frontend to render pending approval cards. |
|
||||
| POST | `/api/build/approvals/{approval_id}/decision` | `submit_decision` (`approvals/api.py`) | Accepts `APPROVED` or `REJECTED`. |
|
||||
| POST | `/api/build/approvals/{approval_id}/session-grant` | `submit_session_grant` (`approvals/api.py`) | Approves and remembers the grant for the rest of the session (see §4.5). |
|
||||
| GET | `/api/build/approvals/sessions/{session_id}/live` | `list_live_approvals` (`server/features/build/approvals/api.py`) | Returns pending approval cards. |
|
||||
| GET | `/api/build/approvals/sessions/{session_id}/status` | `list_session_approval_status` (`approvals/api.py`) | Polls IDs, decisions, and server liveness without loading request payloads. |
|
||||
| GET | `/api/build/approvals/sessions/{session_id}` | `list_session_approvals` (`approvals/api.py`) | Owner-scoped: all live requests plus the 50 most recent outcomes, including rejected and expired requests. |
|
||||
| POST | `/api/build/approvals/{approval_id}/decision` | `submit_decision` (`approvals/api.py`) | Accepts `approve_once`, `approve_session`, or `reject`. |
|
||||
|
||||
Admin CRUD for `ExternalApp` (`/api/build/admin/apps*`) is owned by
|
||||
`[[craft-admin]]`; this document owns what those settings mean at request
|
||||
@@ -166,7 +167,7 @@ ActionApproval -- one row per gated request the proxy parked or auto-decided
|
||||
parked for a human or auto-decided by a grant. `decision IS NULL` means
|
||||
pending or a proxy-crash orphan; `actions[0]` (JSONB, insertion-sorted
|
||||
strictest-first) is what governed the request
|
||||
(`server/features/build/db/action_approval.py:insert_action_approval`).
|
||||
(`db/action_approval.py:insert_action_approval`).
|
||||
- `Sandbox.encrypted_pat` (`models.py`): the sandbox's own scoped Onyx
|
||||
API token, decrypted only inside `OnyxPatResolver.resolve`
|
||||
(`sandbox_proxy/resolvers/onyx_pat.py`).
|
||||
@@ -209,8 +210,8 @@ Reads return `effective_policy`, `workspace_policy`, and `stored_policy` for eac
|
||||
Saves validate catalog IDs and workspace restrictions directly; they do not construct UI responses.
|
||||
All workspace write routes call `replace_workspace_action_policies` for validation and persistence. `db/external_app.py` owns app configuration and credentials.
|
||||
|
||||
`ApprovalDecision` stores `APPROVED`, `REJECTED`, or `EXPIRED`.
|
||||
The decision endpoint accepts APPROVED or REJECTED. Approve for session uses the session-grant endpoint.
|
||||
`ApprovalChoice` accepts `approve_once`, `approve_session`, or `reject`. These are request choices;
|
||||
`ApprovalDecision` stores `APPROVED`, `REJECTED`, or `EXPIRED`. There is one decision endpoint.
|
||||
|
||||
The migration preserves workspace policy values and converts known actions in
|
||||
pre-approved task apps into task policy rows. It adds no permission snapshots.
|
||||
@@ -438,14 +439,15 @@ For an `ASK`-policied action with no applicable grant,
|
||||
adds it to an in-memory `ParkedApprovals` set, and best-effort `RPUSH`es its
|
||||
id onto `approval:announce:{session_id}` (`approval_cache.py:announce_approval`)
|
||||
so the chat-stream merger can surface the card on the live SSE stream; a miss
|
||||
just falls back to the frontend's next pending-request poll
|
||||
(`GET /api/build/approvals/sessions/{session_id}/live`). The proxy then
|
||||
just falls back to the frontend's next status poll
|
||||
(`GET /api/build/approvals/sessions/{session_id}/status`). A status change refreshes
|
||||
the full history once. The proxy then
|
||||
`BLPOP`s `approval:wake:{approval_id}` for up to
|
||||
`SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS`
|
||||
(`approval_cache.py:wait_for_wake`); the user's decision
|
||||
(`POST /api/build/approvals/{approval_id}/decision`) writes the DB row via
|
||||
`try_record_decision` (the sole race arbiter,
|
||||
`server/features/build/db/action_approval.py:try_record_decision`) and
|
||||
`db/action_approval.py:try_record_decision`) and
|
||||
`RPUSH`es the wake key. A timeout with no wake claims `EXPIRED`
|
||||
conditionally, falling back to whatever decision actually won the DB race
|
||||
(`gate.py:_claim_expired_or_read_winner`). See `[[craft-sessions]]` for how
|
||||
@@ -453,8 +455,12 @@ the session and its live stream are structured; see
|
||||
`[[observability]]` for notification delivery
|
||||
(`_notify_approval_requested`).
|
||||
|
||||
Approval endpoints check ownership and the existing 180-second eligibility window.
|
||||
Conditional writes arbitrate approval, rejection, and proxy expiry.
|
||||
The shared decision handler checks ownership and the existing 180-second eligibility window.
|
||||
Conditional writes arbitrate approval, rejection, and proxy expiry. Identical retries return the
|
||||
saved result; retries cannot change approval scope. Session grants and matching pending decisions
|
||||
commit before cache updates and wake notifications. Session grants lock the parent session before updating approval rows to prevent concurrent grants from deadlocking. `ApprovalView.expires_at` is computed from
|
||||
creation time and configuration for display; no deadline column is stored.
|
||||
`can_approve_for_session` is false for unrecognized external actions and deleted targets.
|
||||
|
||||
A **session grant** (`_session_grant`) can approve the currently permitted
|
||||
`ASK` actions after the user chooses "Approve for session". Cached action IDs
|
||||
@@ -698,7 +704,7 @@ they can never disagree about which server owns a request; see
|
||||
| changes identity resolution (`sandbox_identity/resolution.py`, `sandbox_identity/kubernetes.py`, `sandbox_identity/docker.py`) | the duplicate-IP fail-loud check at initial sync; `/healthz` readiness semantics; the session-tag verification in `resolve_session_by_id` (§4.3); both `SandboxIPLookup` backends must stay behavior-identical |
|
||||
| changes credential injection (`credential_injection.py`, any resolver) | the first-claim-wins resolver order in `server.py:build_resolvers`; whether a claim with no headers (`CLAIMED`) vs. a hard failure (`BLOCKED`) is still correct for the new resolver; `logging_utils.py`'s "header names only, never values" invariant |
|
||||
| changes the CA bootstrap or persistence (`certificate_authority/bootstrap.py`, `certificate_authority/kubernetes.py`, `certificate_authority/docker.py`) | every running sandbox's trust store (a rotated CA without a coordinated sandbox restart breaks every in-flight TLS interception); the K8s cross-namespace `ConfigMap` projection; the half-written-state fail-loud recovery path |
|
||||
| changes the approval flow (`gate.py`'s park/wake, `approval_cache.py`, `action_approval.py`) | `[[craft-sessions]]`'s live-stream announce path; the SIGTERM drain (`GateAddon.drain_inflight`) that must terminalize every parked approval before the proxy pod exits; the frontend's pending-approval polling fallback |
|
||||
| changes the approval flow (`gate.py`'s park/wake, `approval_cache.py`, `action_approval.py`) | `[[craft-sessions]]`'s live-stream announce path; the SIGTERM drain (`GateAddon.drain_inflight`) that must terminalize every parked approval before the proxy pod exits; the frontend's approval-status polling fallback |
|
||||
| changes the request body cap (`PARSER_MAX_BODY_BYTES`) | §9's body-limit footgun; whether the new limit still matches or exceeds the relevant upstream's own limit, so the proxy is never the more restrictive failure |
|
||||
| changes `GatedApp`/`WorkspaceActionPolicy` | both consumers: `external_apps/api.py` (admin writes) and this document's live enforcement (`request_evaluator.py`, `addons/gate.py`); MCP servers share this table with external apps |
|
||||
|
||||
|
||||
@@ -197,7 +197,8 @@ per tenant every 30 s):
|
||||
5. On success, marks `succeeded` with a summary
|
||||
(`_summary_from_state`/`_summary_from_session_messages`). On a
|
||||
`RequestPermissionRequest` the agent can't resolve alone, marks
|
||||
`awaiting_approval` and notifies (`NotificationType.SCHEDULED_TASK_AWAITING_APPROVAL`).
|
||||
`failed` with `agent_permission_required` and notifies (`NotificationType.SCHEDULED_TASK_FAILED`).
|
||||
Closing the event stream aborts the underlying turn; no durable resume is claimed.
|
||||
On any executor-level failure (crash, budget exceeded, terminal agent error, a cancelled turn), marks
|
||||
`failed` and notifies (`NotificationType.SCHEDULED_TASK_FAILED`). There
|
||||
are no retries in this version: a failed run is one row, and the user
|
||||
@@ -237,7 +238,8 @@ run ended cannot fall back to personal permissions. Separate interactive convers
|
||||
|
||||
Approve for session remains available. It satisfies ASK for specific actions in that session and
|
||||
never overrides DENY. The proxy keeps its bounded 180-second wait; execution is not durably paused.
|
||||
A recovered action failure does not automatically change the executor's success result.
|
||||
Rejected and expired requests remain visible in approval history. A recovered action failure does
|
||||
not automatically change the executor's success result.
|
||||
|
||||
Create and patch accept `action_policies`, a list of app IDs and action-policy maps. Omitted settings
|
||||
on patch remain unchanged. The editor sends the choices shown in its draft. Empty maps clear editable choices;
|
||||
@@ -249,7 +251,8 @@ settings and lock reasons for the task owner.
|
||||
|
||||
## 5. Contracts and invariants
|
||||
|
||||
1. **Task permissions apply only to the active RUNNING execution.** Recheck after waits;
|
||||
1. **Task permissions apply only to the active RUNNING execution.** Losing the prompt
|
||||
lease closes the producer and aborts the scheduled turn; there is no successor runner. Recheck after waits;
|
||||
a finished run cannot authorize later requests through a cached grant. Requests
|
||||
from scheduled sessions without a running task are blocked, rather than using
|
||||
personal settings. A newly opened task editor waits for fresh policy data;
|
||||
|
||||
@@ -127,9 +127,9 @@ protects.
|
||||
| GET | `/build/sessions/{id}/turns/{turn_id}` | `get_interactive_turn` (`interactive_turns/api.py`) | Read an owned turn's status, including its terminal outcome. |
|
||||
| GET | `/build/sessions/{id}/turns/{turn_id}/events` | `get_interactive_turn_events` (`interactive_turns/api.py`) | SSE attach/resume to a running turn; also (re)starts the runner if it stalled. |
|
||||
| GET/PUT/POST/PATCH/DELETE | `/build/sessions/{id}/generate-name`, `/name`, `/public`, `/files`, `/pptx-preview/{path}`, `/output-thumbnail/{path}`, `/webapp-info`, `/webapp-download`, `/download-directory/{path}`, `/upload`, `/files/{path}`, `/scheduled-run-context`, `/scheduled-run-events` | `session/api.py` | Naming, sharing (`sharing_scope`), workspace file browse and upload, webapp info, and the scheduled-run banner and live events. |
|
||||
| GET | `/build/approvals/sessions/{id}` | `list_session_approvals` (`approvals/api.py`) | All live requests and the 50 most recent outcomes. |
|
||||
| GET | `/build/approvals/sessions/{id}/live` | `list_live_approvals` (`approvals/api.py`) | |
|
||||
| POST | `/build/approvals/{approval_id}/decision` | `submit_decision` (`approvals/api.py`) | |
|
||||
| POST | `/build/approvals/{approval_id}/session-grant` | `submit_session_grant` (`approvals/api.py`) | Pre-approval; see §4.6. |
|
||||
| GET/POST | `/build/user-library/tree`, `/build/user-library/upload`, `/build/user-library/upload-zip`, `/build/user-library/directories` | `user_library/api.py` | User library CRUD. |
|
||||
| DELETE | `/build/user-library/files/{document_id}` | `delete_file` (`user_library/api.py`) | |
|
||||
| GET | `/build/admin/base-instructions` | `get_base_instructions` (`api.py`) | Owned by `[[craft-admin]]`. |
|
||||
@@ -508,10 +508,9 @@ recovery mechanics this depends on.
|
||||
sandbox proxy blocks an ASK-policied request (owned by [[craft-admin]] / [[craft-external-apps]])
|
||||
└─ insert_action_approval (pending row) db/action_approval.py
|
||||
└─ ApprovalRequestedPacket over SSE packets.py, session/streaming.py
|
||||
(FE fetches full contents from /live, Postgres stays source of truth)
|
||||
GET /build/approvals/sessions/{id}/live approvals/api.py:list_live_approvals
|
||||
(FE fetches requests and outcomes; Postgres stays source of truth)
|
||||
GET /build/approvals/sessions/{id} approvals/api.py:list_session_approvals
|
||||
POST /build/approvals/{id}/decision approvals/api.py:submit_decision
|
||||
POST /build/approvals/{id}/session-grant approvals/api.py:submit_session_grant
|
||||
└─ try_record_decision (conditional UPDATE) db/action_approval.py
|
||||
└─ approval_cache.send_wake wakes the parked proxy request
|
||||
```
|
||||
@@ -521,7 +520,7 @@ not this component: an `ASK`-policied request is held there, waiting on the
|
||||
approval row's decision. This document owns the human-facing side: the
|
||||
pending row, the packet that tells the frontend to render a card
|
||||
(`ApprovalRequestedPacket`, carrying only ids so the frontend refetches
|
||||
contents from `/live` and Postgres stays the single source of truth,
|
||||
contents from the session approvals endpoint and Postgres stays the single source of truth,
|
||||
`approvals/api.py` module docstring), and the decision endpoints.
|
||||
|
||||
`try_record_decision` (`db/action_approval.py`) is a conditional `UPDATE ...
|
||||
@@ -529,11 +528,11 @@ WHERE decision IS NULL`, the sole race arbiter; a second decision on an
|
||||
already-decided row returns the existing decision if it matches
|
||||
(idempotent) or a `CONFLICT` if it doesn't. A decision wakes the parked
|
||||
proxy request via a `approval:wake:{id}` cache channel
|
||||
(`_send_wake_best_effort`); a missed wake just falls back to the proxy's own
|
||||
(`submit_decision`); a missed wake just falls back to the proxy's own
|
||||
`SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS` wait.
|
||||
|
||||
**Pre-approval is "session-grant", not a separate mechanism.**
|
||||
`submit_session_grant` (`approvals/api.py`) approves the specific request
|
||||
**Approve for session uses the shared decision endpoint.**
|
||||
With `ApprovalChoice.APPROVE_SESSION`, `submit_decision` (`approvals/api.py`) approves the specific request
|
||||
*and* every other currently-pending or future request in the session that
|
||||
matches the same `(gated_app_id)` target and whose required action types are
|
||||
a subset of what was just granted (`actions_requiring_approval`,
|
||||
@@ -552,6 +551,10 @@ anything older as orphaned (the proxy that was waiting on it is gone), so
|
||||
the frontend never offers to decide a request that can no longer have any
|
||||
effect.
|
||||
|
||||
The session approvals endpoint returns all live requests and the 50 most recent outcomes.
|
||||
Expired undecided requests count as outcomes. The frontend uses one query for cards and refreshes it after decisions.
|
||||
The query loads app identities together to avoid one database read per card.
|
||||
|
||||
### 4.7 Artifacts, receipts, and export
|
||||
|
||||
`backend/onyx/server/features/build/session/output_cards.py` compares complete manifests before and after each interactive or scheduled prompt, while the runner owns the prompt slot.
|
||||
@@ -792,7 +795,7 @@ A directory scan reports current files, not exact per-turn provenance.
|
||||
| changes the turn executor (`interactive_turns/executor.py`) | every streamed terminal path still calls `finalize_persist` before `finish_turn` (the prompt-slot rejection path persists an error row instead); the timeout-continuation loop's `MAX_TIMEOUT_CONTINUATIONS` cap; `[[craft-streaming]]`'s event shapes the executor switches on (`PromptResponse`, `ActivityTimeoutError`, `SandboxError`) |
|
||||
| changes locking (`prompt_slot`, `acquire_active_turn_lock`, `session_creation_lock`) | the CONFLICT-vs-silent-failure behavior at each acquisition site; whether a reclaim (`claim_turn_for_runner`) can still race a live runner; `[[craft-sandboxes]]`'s reaper, which also touches sandbox state under related locks |
|
||||
| changes interrupt (`interrupt_signal.py`, `interrupt_message`) | the FE's pending Stop request barrier and known-turn terminal outcome contract; whether partial output still commits before `finish_turn(CANCELLED)`; the direct `abort_opencode_session` remaining best-effort, with a short HTTP timeout under the admission lock |
|
||||
| adds an approval point | `ApprovalRequestedPacket` plumbing through `[[craft-streaming]]`; `list_live_approvals`'s time-window filter; whether the new point's target maps to a `GatedApp (kind, target_id)` the session-grant matching in `submit_session_grant` can actually cover |
|
||||
| adds an approval point | `ApprovalRequestedPacket` plumbing through `[[craft-streaming]]`; `list_live_approvals`'s time-window filter; whether the new point's target maps to a `GatedApp (kind, target_id)` the session-grant matching in `submit_decision` can actually cover |
|
||||
| changes session state (`BuildSession` columns, `session_ready.py`) | `session_runtime_intact`'s fast-path check; `reconcile_session_llm_config`'s short-circuit comparison; any snapshot/restore path in `[[craft-sandboxes]]` that reads these columns |
|
||||
| changes artifact upsert or hashing | the FE artifact list/version display; `export_docx`/`download_artifact`, which read the live workspace, not the DB row's cached bytes |
|
||||
| changes user library sync | both trigger sets (upload/delete synchronous sync via `sync_user_library_to_active_sandboxes`, and session-provisioning sync via `build_managed_content_payload`/`push_managed_content`); the shared `Document`/cc-pair machinery other ingestion consumers also read |
|
||||
|
||||
@@ -43,6 +43,13 @@ The editor covers external apps and MCP servers. Workspace-disabled actions
|
||||
remain visible and locked. Task editors use the same action rows with
|
||||
independent saved choices.
|
||||
|
||||
Approval cards keep **Approve once**, **Approve for session**, and **Reject**.
|
||||
All three call the approval decision endpoint. The session polls lightweight
|
||||
approval status every ten seconds. Status changes refresh the history. The history
|
||||
contains live requests and recent outcomes, so completed requests remain visible.
|
||||
Approval cards use the server live status to show expiry. Device clock differences cannot block decisions.
|
||||
The displayed approval deadline comes from the server; a browser does not own the wait.
|
||||
|
||||
The Working group shows the tool-call count without a failed-count badge.
|
||||
Its tool details start collapsed, including failures; users can expand each call
|
||||
to read its output.
|
||||
|
||||
+51
-3
@@ -7,8 +7,8 @@ from datetime import datetime, timezone
|
||||
from typing import Any
|
||||
from uuid import UUID
|
||||
|
||||
from sqlalchemy import select, update
|
||||
from sqlalchemy.orm import Session, selectinload
|
||||
from sqlalchemy import and_, or_, select, update
|
||||
from sqlalchemy.orm import Session, load_only, selectinload
|
||||
|
||||
from onyx.db.action_policy import get_or_create_gated_app_id
|
||||
from onyx.db.enums import (
|
||||
@@ -77,6 +77,7 @@ def try_record_decision(
|
||||
approval_id: UUID,
|
||||
decision: ApprovalDecision,
|
||||
decided_via: ApprovalDecidedVia | None = None,
|
||||
created_after: datetime | None = None,
|
||||
) -> ActionApproval | None:
|
||||
"""Conditional UPDATE that succeeds only while `decision IS NULL`.
|
||||
|
||||
@@ -94,6 +95,8 @@ def try_record_decision(
|
||||
.returning(ActionApproval)
|
||||
.execution_options(synchronize_session=False)
|
||||
)
|
||||
if created_after is not None:
|
||||
stmt = stmt.where(ActionApproval.created_at >= created_after)
|
||||
row = db_session.execute(stmt).scalar_one_or_none()
|
||||
db_session.flush()
|
||||
if row is not None:
|
||||
@@ -110,7 +113,7 @@ def get_action_approval(
|
||||
|
||||
|
||||
def get_action_approval_for_user(
|
||||
db_session: Session, approval_id: UUID, user_id: UUID
|
||||
db_session: Session, approval_id: UUID, user_id: UUID, *, lock_session: bool = False
|
||||
) -> ActionApproval | None:
|
||||
"""Row only if the caller owns the parent build_session.
|
||||
|
||||
@@ -122,9 +125,54 @@ def get_action_approval_for_user(
|
||||
.where(ActionApproval.approval_id == approval_id)
|
||||
.where(BuildSession.user_id == user_id)
|
||||
)
|
||||
if lock_session:
|
||||
# Serialize grants before either request locks its own approval row.
|
||||
stmt = stmt.with_for_update(of=BuildSession)
|
||||
return db_session.scalar(stmt)
|
||||
|
||||
|
||||
def list_session_action_approvals(
|
||||
db_session: Session,
|
||||
session_id: UUID,
|
||||
*,
|
||||
created_after: datetime,
|
||||
status_only: bool = False,
|
||||
) -> list[ActionApproval]:
|
||||
"""All live requests and the 50 most recent completed or expired requests."""
|
||||
is_live = and_(
|
||||
ActionApproval.decision.is_(None),
|
||||
ActionApproval.created_at >= created_after,
|
||||
)
|
||||
recent_outcomes = (
|
||||
select(ActionApproval.approval_id)
|
||||
.where(ActionApproval.session_id == session_id, ~is_live)
|
||||
.order_by(ActionApproval.created_at.desc(), ActionApproval.approval_id.desc())
|
||||
.limit(50)
|
||||
)
|
||||
return list(
|
||||
db_session.scalars(
|
||||
select(ActionApproval)
|
||||
.where(
|
||||
ActionApproval.session_id == session_id,
|
||||
or_(is_live, ActionApproval.approval_id.in_(recent_outcomes)),
|
||||
)
|
||||
.options(
|
||||
load_only(
|
||||
ActionApproval.approval_id,
|
||||
ActionApproval.created_at,
|
||||
ActionApproval.decision,
|
||||
raiseload=True,
|
||||
)
|
||||
if status_only
|
||||
else selectinload(ActionApproval.gated_app)
|
||||
)
|
||||
.order_by(
|
||||
ActionApproval.created_at.desc(), ActionApproval.approval_id.desc()
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def list_session_pending_action_approvals(
|
||||
db_session: Session,
|
||||
session_id: UUID,
|
||||
@@ -515,6 +515,7 @@ class ScheduledTaskErrorClass(str, PyEnum):
|
||||
TIMEOUT = "timeout"
|
||||
STUCK = "stuck"
|
||||
AGENT_EXCEPTION = "agent_exception"
|
||||
AGENT_PERMISSION_REQUIRED = "agent_permission_required"
|
||||
|
||||
|
||||
class ScheduledTaskSkipReason(str, PyEnum):
|
||||
|
||||
@@ -16,6 +16,7 @@ from sqlalchemy.orm import Session
|
||||
|
||||
from onyx.cache.interface import CACHE_TRANSIENT_ERRORS, CacheBackend
|
||||
from onyx.configs.constants import NotificationType
|
||||
from onyx.db import action_approval
|
||||
from onyx.db.action_policy import (
|
||||
get_active_scheduled_task_context,
|
||||
get_gated_app_id,
|
||||
@@ -81,7 +82,6 @@ from onyx.server.features.build.configs import (
|
||||
MCP_SESSION_TAG_HEADER,
|
||||
SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS,
|
||||
)
|
||||
from onyx.server.features.build.db import action_approval
|
||||
from onyx.utils.logger import setup_logger
|
||||
|
||||
logger = setup_logger()
|
||||
|
||||
@@ -7,26 +7,31 @@ wake just falls back to the proxy's wait timeout.
|
||||
"""
|
||||
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import Any, Literal
|
||||
from enum import StrEnum
|
||||
from typing import Any
|
||||
from uuid import UUID
|
||||
|
||||
from fastapi import APIRouter, Depends
|
||||
from pydantic import BaseModel, ConfigDict, computed_field
|
||||
from pydantic import AliasPath, BaseModel, ConfigDict, Field, computed_field
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from onyx.auth.permissions import require_permission
|
||||
from onyx.cache.factory import get_cache_backend
|
||||
from onyx.cache.interface import CACHE_TRANSIENT_ERRORS, CacheBackend
|
||||
from onyx.db import action_approval
|
||||
from onyx.db.engine.sql_engine import get_session
|
||||
from onyx.db.enums import ApprovalDecidedVia, ApprovalDecision, GatedAppKind, Permission
|
||||
from onyx.db.models import ActionApproval, User
|
||||
from onyx.error_handling.error_codes import OnyxErrorCode
|
||||
from onyx.error_handling.exceptions import OnyxError
|
||||
from onyx.external_apps.matching.engine import MatchedAction, actions_requiring_approval
|
||||
from onyx.external_apps.matching.engine import (
|
||||
WHOLE_DOMAIN_ACTION_TYPE,
|
||||
MatchedAction,
|
||||
actions_requiring_approval,
|
||||
)
|
||||
from onyx.external_apps.presentation.decode import decode_payload
|
||||
from onyx.sandbox_proxy import approval_cache
|
||||
from onyx.server.features.build.configs import SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS
|
||||
from onyx.server.features.build.db import action_approval
|
||||
from onyx.server.features.build.db.build_session import get_build_session
|
||||
from onyx.utils.logger import setup_logger
|
||||
from shared_configs.contextvars import get_current_tenant_id
|
||||
@@ -37,10 +42,15 @@ logger = setup_logger()
|
||||
router = APIRouter(prefix="/approvals")
|
||||
|
||||
|
||||
class ApprovalChoice(StrEnum):
|
||||
APPROVE_ONCE = "approve_once"
|
||||
APPROVE_SESSION = "approve_session"
|
||||
REJECT = "reject"
|
||||
|
||||
|
||||
class DecisionBody(BaseModel):
|
||||
model_config = ConfigDict(extra="forbid")
|
||||
# EXPIRED is server-only (set by the proxy on timeout).
|
||||
decision: Literal[ApprovalDecision.APPROVED, ApprovalDecision.REJECTED]
|
||||
decision: ApprovalChoice
|
||||
|
||||
|
||||
class ApprovalView(BaseModel):
|
||||
@@ -55,6 +65,19 @@ class ApprovalView(BaseModel):
|
||||
created_at: datetime
|
||||
decision: ApprovalDecision | None
|
||||
decided_at: datetime | None
|
||||
target_kind: GatedAppKind | None = Field(
|
||||
default=None, validation_alias=AliasPath("gated_app", "kind"), exclude=True
|
||||
)
|
||||
|
||||
@computed_field
|
||||
@property
|
||||
def can_approve_for_session(self) -> bool:
|
||||
if self.target_kind is None or not actions_requiring_approval(self.actions):
|
||||
return False
|
||||
return not (
|
||||
self.target_kind == GatedAppKind.EXTERNAL_APP
|
||||
and any(a.action_type == WHOLE_DOMAIN_ACTION_TYPE for a in self.actions)
|
||||
)
|
||||
|
||||
@computed_field
|
||||
@property
|
||||
@@ -64,15 +87,28 @@ class ApprovalView(BaseModel):
|
||||
action_type = self.actions[0].action_type if self.actions else ""
|
||||
return decode_payload(action_type, self.payload)
|
||||
|
||||
@computed_field
|
||||
@property
|
||||
def expires_at(self) -> datetime:
|
||||
"""Display cutoff for the existing proxy wait; no stored approval deadline."""
|
||||
return self.created_at + timedelta(
|
||||
seconds=SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS
|
||||
)
|
||||
|
||||
@computed_field
|
||||
@property
|
||||
def is_live(self) -> bool:
|
||||
if self.decision is not None:
|
||||
return False
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(
|
||||
seconds=SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS
|
||||
)
|
||||
return self.created_at >= cutoff
|
||||
return self.expires_at >= datetime.now(timezone.utc)
|
||||
|
||||
|
||||
class ApprovalStatusView(BaseModel):
|
||||
"""Small polling response; full request payloads stay in the history read."""
|
||||
|
||||
approval_id: UUID
|
||||
decision: ApprovalDecision | None
|
||||
is_live: bool
|
||||
|
||||
|
||||
class ApprovalListResponse(BaseModel):
|
||||
@@ -85,38 +121,66 @@ def _row_target(row: ActionApproval) -> tuple[GatedAppKind, int] | None:
|
||||
return row.gated_app.target_key if row.gated_app is not None else None
|
||||
|
||||
|
||||
def _send_wake_best_effort(approval_id: UUID, decision: ApprovalDecision) -> None:
|
||||
try:
|
||||
cache = get_cache_backend(tenant_id=get_current_tenant_id())
|
||||
approval_cache.send_wake(approval_id, decision, cache)
|
||||
except CACHE_TRANSIENT_ERRORS as e:
|
||||
logger.warning(
|
||||
"approval.wake_failed approval_id=%s error=%s",
|
||||
approval_id,
|
||||
str(e),
|
||||
)
|
||||
|
||||
|
||||
def _existing_decision_response(
|
||||
view: ApprovalView, requested: ApprovalDecision, approval_id: UUID
|
||||
row: ActionApproval, requested: ApprovalDecision, *, for_session: bool
|
||||
) -> ApprovalView:
|
||||
"""Same decision is idempotent; a different one is a CONFLICT."""
|
||||
if view.decision == requested:
|
||||
return view
|
||||
existing = view.decision.value if view.decision is not None else "unknown"
|
||||
logger.info(
|
||||
"approval.decision_conflict approval_id=%s "
|
||||
"existing_decision=%s requested_decision=%s",
|
||||
approval_id,
|
||||
existing,
|
||||
requested.value,
|
||||
)
|
||||
"""A retry must match both the decision and its scope."""
|
||||
if row.decision == requested and (
|
||||
requested == ApprovalDecision.REJECTED
|
||||
or (row.decided_via == ApprovalDecidedVia.SESSION_GRANT) == for_session
|
||||
):
|
||||
return ApprovalView.model_validate(row)
|
||||
existing = row.decision.value if row.decision is not None else "pending"
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.CONFLICT,
|
||||
f"decision already recorded ({existing})",
|
||||
f"approval request cannot accept this decision ({existing})",
|
||||
)
|
||||
|
||||
|
||||
@router.get("/sessions/{session_id}")
|
||||
def list_session_approvals(
|
||||
session_id: UUID,
|
||||
user: User = Depends(require_permission(Permission.BASIC_ACCESS)),
|
||||
db_session: Session = Depends(get_session),
|
||||
) -> ApprovalListResponse:
|
||||
if get_build_session(session_id, user.id, db_session) is None:
|
||||
raise OnyxError(OnyxErrorCode.NOT_FOUND, "session not found")
|
||||
return ApprovalListResponse(
|
||||
items=[
|
||||
ApprovalView.model_validate(row)
|
||||
for row in action_approval.list_session_action_approvals(
|
||||
db_session,
|
||||
session_id,
|
||||
created_after=datetime.now(timezone.utc)
|
||||
- timedelta(seconds=SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS),
|
||||
)
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
@router.get("/sessions/{session_id}/status")
|
||||
def list_session_approval_status(
|
||||
session_id: UUID,
|
||||
user: User = Depends(require_permission(Permission.BASIC_ACCESS)),
|
||||
db_session: Session = Depends(get_session),
|
||||
) -> list[ApprovalStatusView]:
|
||||
if get_build_session(session_id, user.id, db_session) is None:
|
||||
raise OnyxError(OnyxErrorCode.NOT_FOUND, "session not found")
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(
|
||||
seconds=SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS
|
||||
)
|
||||
return [
|
||||
ApprovalStatusView(
|
||||
approval_id=row.approval_id,
|
||||
decision=row.decision,
|
||||
is_live=row.decision is None and row.created_at >= cutoff,
|
||||
)
|
||||
for row in action_approval.list_session_action_approvals(
|
||||
db_session, session_id, created_after=cutoff, status_only=True
|
||||
)
|
||||
]
|
||||
|
||||
|
||||
@router.get("/sessions/{session_id}/live")
|
||||
def list_live_approvals(
|
||||
session_id: UUID,
|
||||
@@ -148,187 +212,109 @@ def submit_decision(
|
||||
user: User = Depends(require_permission(Permission.BASIC_ACCESS)),
|
||||
db_session: Session = Depends(get_session),
|
||||
) -> ApprovalView:
|
||||
"""Record the caller's decision on a pending approval request."""
|
||||
decision = (
|
||||
ApprovalDecision.REJECTED
|
||||
if body.decision == ApprovalChoice.REJECT
|
||||
else ApprovalDecision.APPROVED
|
||||
)
|
||||
for_session = body.decision == ApprovalChoice.APPROVE_SESSION
|
||||
current = action_approval.get_action_approval_for_user(
|
||||
db_session, approval_id, user.id
|
||||
db_session, approval_id, user.id, lock_session=for_session
|
||||
)
|
||||
if current is None:
|
||||
raise OnyxError(OnyxErrorCode.NOT_FOUND, "approval request not found")
|
||||
|
||||
if current.decision is not None:
|
||||
return _existing_decision_response(
|
||||
ApprovalView.model_validate(current), body.decision, approval_id
|
||||
)
|
||||
return _existing_decision_response(current, decision, for_session=for_session)
|
||||
|
||||
decided = action_approval.try_record_decision(
|
||||
db_session,
|
||||
approval_id=approval_id,
|
||||
decision=body.decision,
|
||||
decided_via=ApprovalDecidedVia.USER,
|
||||
)
|
||||
if decided is None:
|
||||
# Lost the race. Expire the cached row — SQLAlchemy's identity
|
||||
# map would otherwise hand back the pre-UPDATE `current`.
|
||||
db_session.expire(current)
|
||||
winner = action_approval.get_action_approval(db_session, approval_id)
|
||||
if winner is None:
|
||||
# FK cascade dropped the row between our two reads.
|
||||
raise OnyxError(OnyxErrorCode.NOT_FOUND, "approval request not found")
|
||||
if winner.decision is None:
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.INTERNAL_ERROR,
|
||||
"approval row reverted to pending unexpectedly",
|
||||
)
|
||||
return _existing_decision_response(
|
||||
ApprovalView.model_validate(winner), body.decision, approval_id
|
||||
)
|
||||
|
||||
db_session.commit()
|
||||
logger.info(
|
||||
"approval.decision_recorded approval_id=%s session_id=%s "
|
||||
"user_id=%s decision=%s",
|
||||
approval_id,
|
||||
current.session_id,
|
||||
user.id,
|
||||
body.decision.value,
|
||||
)
|
||||
|
||||
_send_wake_best_effort(approval_id, body.decision)
|
||||
|
||||
return ApprovalView.model_validate(decided)
|
||||
|
||||
|
||||
@router.post("/{approval_id}/session-grant")
|
||||
def submit_session_grant(
|
||||
approval_id: UUID,
|
||||
user: User = Depends(require_permission(Permission.BASIC_ACCESS)),
|
||||
db_session: Session = Depends(get_session),
|
||||
) -> ApprovalView:
|
||||
"""Approve this request and similar app/action requests for the session."""
|
||||
current = action_approval.get_action_approval_for_user(
|
||||
db_session, approval_id, user.id
|
||||
)
|
||||
if current is None:
|
||||
raise OnyxError(OnyxErrorCode.NOT_FOUND, "approval request not found")
|
||||
session_id = current.session_id
|
||||
target = _row_target(current)
|
||||
if target is None:
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.INVALID_INPUT,
|
||||
"approval request cannot be granted for a session",
|
||||
)
|
||||
target_kind, target_id = target
|
||||
if current.decision is not None:
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.CONFLICT,
|
||||
"approval request already resolved",
|
||||
)
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(
|
||||
seconds=SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS
|
||||
)
|
||||
if current.created_at < cutoff:
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.CONFLICT,
|
||||
"approval request is no longer live",
|
||||
)
|
||||
raise OnyxError(OnyxErrorCode.CONFLICT, "approval request is no longer live")
|
||||
|
||||
grant_action_types = actions_requiring_approval(current.actions)
|
||||
if not grant_action_types:
|
||||
target = _row_target(current) if for_session else None
|
||||
if for_session and not ApprovalView.model_validate(current).can_approve_for_session:
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.INVALID_INPUT,
|
||||
"approval request has no grantable actions",
|
||||
OnyxErrorCode.INVALID_INPUT, "approval request has no grantable actions"
|
||||
)
|
||||
|
||||
approved_ids: list[UUID] = []
|
||||
decided_current = action_approval.try_record_decision(
|
||||
decided = action_approval.try_record_decision(
|
||||
db_session,
|
||||
approval_id=approval_id,
|
||||
decision=ApprovalDecision.APPROVED,
|
||||
decided_via=ApprovalDecidedVia.SESSION_GRANT,
|
||||
decision=decision,
|
||||
decided_via=(
|
||||
ApprovalDecidedVia.SESSION_GRANT if for_session else ApprovalDecidedVia.USER
|
||||
),
|
||||
created_after=cutoff,
|
||||
)
|
||||
if decided_current is None:
|
||||
if decided is None:
|
||||
db_session.expire(current)
|
||||
winner = action_approval.get_action_approval(db_session, approval_id)
|
||||
if winner is None:
|
||||
raise OnyxError(OnyxErrorCode.NOT_FOUND, "approval request not found")
|
||||
if winner.decision is None:
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.INTERNAL_ERROR,
|
||||
"approval row reverted to pending unexpectedly",
|
||||
return _existing_decision_response(winner, decision, for_session=for_session)
|
||||
|
||||
approved_ids = [approval_id]
|
||||
grant_source_rows: list[ActionApproval] = []
|
||||
if target is not None:
|
||||
grant_source_rows = action_approval.list_session_grant_action_approvals(
|
||||
db_session, session_id=current.session_id, gated_app_id=current.gated_app_id
|
||||
)
|
||||
granted_actions = {
|
||||
action
|
||||
for row in grant_source_rows
|
||||
for action in actions_requiring_approval(row.actions)
|
||||
}
|
||||
pending_rows = action_approval.list_session_pending_action_approvals(
|
||||
db_session, current.session_id, created_after=cutoff, load_target=True
|
||||
)
|
||||
for row in pending_rows:
|
||||
required = set(actions_requiring_approval(row.actions))
|
||||
if (
|
||||
_row_target(row) != target
|
||||
or not required
|
||||
or not required.issubset(granted_actions)
|
||||
):
|
||||
continue
|
||||
approved = action_approval.try_record_decision(
|
||||
db_session,
|
||||
approval_id=row.approval_id,
|
||||
decision=ApprovalDecision.APPROVED,
|
||||
decided_via=ApprovalDecidedVia.SESSION_GRANT,
|
||||
created_after=cutoff,
|
||||
)
|
||||
existing = winner.decision.value
|
||||
raise OnyxError(
|
||||
OnyxErrorCode.CONFLICT,
|
||||
f"approval request already resolved ({existing})",
|
||||
)
|
||||
approved_ids.append(approval_id)
|
||||
if approved is not None:
|
||||
approved_ids.append(row.approval_id)
|
||||
|
||||
# Commit all covered decisions before publishing grants or waking requests.
|
||||
db_session.commit()
|
||||
_send_wake_best_effort(approval_id, ApprovalDecision.APPROVED)
|
||||
|
||||
grant_source_rows = action_approval.list_session_grant_action_approvals(
|
||||
db_session,
|
||||
session_id=session_id,
|
||||
gated_app_id=current.gated_app_id,
|
||||
)
|
||||
cache: CacheBackend | None = None
|
||||
try:
|
||||
cache = get_cache_backend(tenant_id=get_current_tenant_id())
|
||||
except CACHE_TRANSIENT_ERRORS as e:
|
||||
logger.warning(
|
||||
"approval.session_grant_cache_failed approval_id=%s session_id=%s error=%s",
|
||||
approval_id,
|
||||
session_id,
|
||||
str(e),
|
||||
if target is not None:
|
||||
cache: CacheBackend | None = None
|
||||
try:
|
||||
cache = get_cache_backend(tenant_id=get_current_tenant_id())
|
||||
except CACHE_TRANSIENT_ERRORS:
|
||||
logger.warning(
|
||||
"approval.session_grant_cache_failed approval_id=%s", approval_id
|
||||
)
|
||||
approval_cache.hydrate_session_grants(
|
||||
session_id=current.session_id,
|
||||
kind=target[0],
|
||||
target_id=target[1],
|
||||
rows=grant_source_rows,
|
||||
cache=cache,
|
||||
)
|
||||
granted_action_types = approval_cache.hydrate_session_grants(
|
||||
session_id=session_id,
|
||||
kind=target_kind,
|
||||
target_id=target_id,
|
||||
rows=grant_source_rows,
|
||||
cache=cache,
|
||||
)
|
||||
|
||||
pending_rows = action_approval.list_session_pending_action_approvals(
|
||||
db_session, session_id, created_after=cutoff, load_target=True
|
||||
)
|
||||
for row in pending_rows:
|
||||
if row.approval_id == approval_id:
|
||||
continue
|
||||
if _row_target(row) != target:
|
||||
continue
|
||||
row_action_types = set(actions_requiring_approval(row.actions))
|
||||
covered = bool(row_action_types) and row_action_types.issubset(
|
||||
granted_action_types
|
||||
)
|
||||
if not covered:
|
||||
continue
|
||||
decided = action_approval.try_record_decision(
|
||||
db_session,
|
||||
approval_id=row.approval_id,
|
||||
decision=ApprovalDecision.APPROVED,
|
||||
decided_via=ApprovalDecidedVia.SESSION_GRANT,
|
||||
)
|
||||
if decided is not None:
|
||||
approved_ids.append(row.approval_id)
|
||||
|
||||
db_session.commit()
|
||||
|
||||
for approved_id in approved_ids:
|
||||
if approved_id == approval_id:
|
||||
continue
|
||||
_send_wake_best_effort(approved_id, ApprovalDecision.APPROVED)
|
||||
|
||||
for decided_id in approved_ids:
|
||||
try:
|
||||
cache = get_cache_backend(tenant_id=get_current_tenant_id())
|
||||
approval_cache.send_wake(decided_id, decision, cache)
|
||||
except CACHE_TRANSIENT_ERRORS as error:
|
||||
logger.warning(
|
||||
"approval.wake_failed approval_id=%s error=%s", decided_id, error
|
||||
)
|
||||
logger.info(
|
||||
"approval.session_grant_recorded approval_id=%s session_id=%s "
|
||||
"user_id=%s target=%s:%s action_types=%s approved_count=%s",
|
||||
"approval.decision_recorded approval_id=%s session_id=%s decision=%s for_session=%s",
|
||||
approval_id,
|
||||
session_id,
|
||||
user.id,
|
||||
target_kind.value,
|
||||
target_id,
|
||||
grant_action_types,
|
||||
len(approved_ids),
|
||||
current.session_id,
|
||||
decision.value,
|
||||
for_session,
|
||||
)
|
||||
|
||||
return ApprovalView.model_validate(decided_current)
|
||||
return ApprovalView.model_validate(decided)
|
||||
@@ -24,10 +24,9 @@ Lifecycle (see ``docs/craft/features/scheduled-tasks/overview.md``):
|
||||
persisting each event with ``persist_sandbox_event``. Enforce a 30 min
|
||||
monotonic budget (Celery thread-pool time limits are silently
|
||||
disabled — see CLAUDE.md).
|
||||
6. On ``RequestPermissionRequest`` (approval gate): mark
|
||||
``AWAITING_APPROVAL``, emit a notification, return without writing a
|
||||
terminal status. Resume mechanics are owned by the approvals project;
|
||||
until that ships these runs are "terminal-for-display".
|
||||
6. On an agent ``RequestPermissionRequest``, stop execution and record FAILED
|
||||
with an actionable reason. Proxy action approvals instead keep their existing
|
||||
bounded wait and do not end the agent stream.
|
||||
7. On clean stream completion: ``finalize_persist``, derive a
|
||||
~120-char summary, mark ``SUCCEEDED``.
|
||||
8. On any exception inside the drive loop: mark ``FAILED`` with the
|
||||
@@ -331,16 +330,12 @@ def _drive_agent(
|
||||
task_prompt: str,
|
||||
sandbox_id: UUID,
|
||||
budget_seconds: int,
|
||||
) -> bool:
|
||||
) -> None:
|
||||
"""Drive the agent for a single scheduled run.
|
||||
|
||||
Creates the BuildSession, persists the user prompt, iterates sandbox
|
||||
events with the shared persistence consumer, and writes the terminal
|
||||
run status.
|
||||
|
||||
Returns:
|
||||
``True`` if the run paused on an approval gate (run status is
|
||||
AWAITING_APPROVAL). ``False`` otherwise (run status is terminal).
|
||||
"""
|
||||
# We open a single session for the whole drive so that the
|
||||
# `persist_sandbox_event` calls (which write `BuildMessage` rows) and the
|
||||
@@ -418,7 +413,7 @@ def _drive_agent(
|
||||
notif_type=NotificationType.SCHEDULED_TASK_FAILED,
|
||||
)
|
||||
db_session.commit()
|
||||
return False
|
||||
return
|
||||
output_manager = get_sandbox_manager()
|
||||
output_snapshot: TurnOutputSnapshot | None = None
|
||||
event_stream = session_manager.yield_sandbox_events(
|
||||
@@ -455,9 +450,8 @@ def _drive_agent(
|
||||
slot.extend()
|
||||
if slot.lost:
|
||||
break
|
||||
# Approval gate: mark awaiting_approval, return. Resume
|
||||
# mechanics are owned by the approvals project; this is
|
||||
# "terminal for display" until it ships.
|
||||
# Agent permission requests cannot resume this executor.
|
||||
# Closing the stream aborts the underlying turn.
|
||||
if isinstance(sandbox_event, RequestPermissionRequest):
|
||||
approval_required = True
|
||||
break
|
||||
@@ -524,7 +518,9 @@ def _drive_agent(
|
||||
mark_run_status(
|
||||
db_session=db_session,
|
||||
run_id=run_id,
|
||||
status=ScheduledTaskRunStatus.AWAITING_APPROVAL,
|
||||
status=ScheduledTaskRunStatus.FAILED,
|
||||
error_class=ScheduledTaskErrorClass.AGENT_PERMISSION_REQUIRED,
|
||||
error_detail="Execution stopped because the agent needs permission. Review permissions and run the task again.",
|
||||
summary=summary or None,
|
||||
)
|
||||
_notify(
|
||||
@@ -533,10 +529,10 @@ def _drive_agent(
|
||||
task_name=task_name,
|
||||
task_id=task_id,
|
||||
run_id=run_id,
|
||||
notif_type=NotificationType.SCHEDULED_TASK_AWAITING_APPROVAL,
|
||||
notif_type=NotificationType.SCHEDULED_TASK_FAILED,
|
||||
)
|
||||
db_session.commit()
|
||||
return True
|
||||
return
|
||||
|
||||
if terminal_error is not None or cancelled or not got_prompt_response:
|
||||
if terminal_error is not None:
|
||||
@@ -579,7 +575,7 @@ def _drive_agent(
|
||||
error_class.value,
|
||||
error_detail,
|
||||
)
|
||||
return False
|
||||
return
|
||||
|
||||
# Clean completion path.
|
||||
summary = summary_from_chunks or _summary_from_session_messages(
|
||||
@@ -598,7 +594,7 @@ def _drive_agent(
|
||||
final_event_count,
|
||||
session_id,
|
||||
)
|
||||
return False
|
||||
return
|
||||
except Exception as exc:
|
||||
# Roll back any uncommitted persistence work and translate
|
||||
# to FAILED. Do NOT re-raise — Celery would retry, which is
|
||||
@@ -637,7 +633,7 @@ def _drive_agent(
|
||||
run_id,
|
||||
)
|
||||
logger.exception("Scheduled run %s failed", run_id)
|
||||
return False
|
||||
return
|
||||
finally:
|
||||
try:
|
||||
event_stream.close()
|
||||
|
||||
@@ -15,6 +15,13 @@ import pytest
|
||||
from sqlalchemy import delete
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from onyx.db.action_approval import (
|
||||
get_action_approval,
|
||||
get_action_approval_for_user,
|
||||
insert_action_approval,
|
||||
list_session_pending_action_approvals,
|
||||
try_record_decision,
|
||||
)
|
||||
from onyx.db.enums import (
|
||||
ActionPolicy,
|
||||
ApprovalDecidedVia,
|
||||
@@ -23,13 +30,6 @@ from onyx.db.enums import (
|
||||
GatedAppKind,
|
||||
)
|
||||
from onyx.db.models import ActionApproval, BuildSession, ExternalApp
|
||||
from onyx.server.features.build.db.action_approval import (
|
||||
get_action_approval,
|
||||
get_action_approval_for_user,
|
||||
insert_action_approval,
|
||||
list_session_pending_action_approvals,
|
||||
try_record_decision,
|
||||
)
|
||||
from tests.common.craft.payloads import action_entry, default_action_entries
|
||||
from tests.external_dependency_unit.craft.db_helpers import (
|
||||
force_approval_created_at,
|
||||
|
||||
@@ -4,38 +4,44 @@ constructed ``User`` and the test ``db_session`` (no ``TestClient``)."""
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Callable
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from threading import Barrier
|
||||
from uuid import UUID, uuid4
|
||||
|
||||
import pytest
|
||||
import redis
|
||||
from sqlalchemy import text
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from onyx.cache.factory import get_cache_backend
|
||||
from onyx.db import action_approval
|
||||
from onyx.db.action_approval import (
|
||||
get_action_approval,
|
||||
get_action_approval_for_user,
|
||||
insert_action_approval,
|
||||
try_record_decision,
|
||||
)
|
||||
from onyx.db.enums import (
|
||||
ActionPolicy,
|
||||
ApprovalDecidedVia,
|
||||
ApprovalDecision,
|
||||
GatedAppKind,
|
||||
)
|
||||
from onyx.db.models import BuildSession
|
||||
from onyx.db.models import ActionApproval, BuildSession
|
||||
from onyx.error_handling.error_codes import OnyxErrorCode
|
||||
from onyx.error_handling.exceptions import OnyxError
|
||||
from onyx.external_apps.matching.engine import MatchedAction
|
||||
from onyx.sandbox_proxy import approval_cache
|
||||
from onyx.server.features.build.approvals.api import (
|
||||
ApprovalChoice,
|
||||
DecisionBody,
|
||||
list_live_approvals,
|
||||
list_session_approval_status,
|
||||
list_session_approvals,
|
||||
submit_decision,
|
||||
submit_session_grant,
|
||||
)
|
||||
from onyx.server.features.build.configs import SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS
|
||||
from onyx.server.features.build.db.action_approval import (
|
||||
get_action_approval,
|
||||
get_action_approval_for_user,
|
||||
insert_action_approval,
|
||||
try_record_decision,
|
||||
)
|
||||
from shared_configs.configs import POSTGRES_DEFAULT_SCHEMA_STANDARD_VALUE
|
||||
from tests.common.craft.payloads import action_entry
|
||||
from tests.common.craft.payloads import default_action_entries as _default_actions
|
||||
@@ -175,7 +181,7 @@ def test_submit_decision_happy_path_returns_refreshed_row(
|
||||
|
||||
view = submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.REJECTED),
|
||||
body=DecisionBody(decision=ApprovalChoice.REJECT),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -213,13 +219,13 @@ def test_submit_decision_same_decision_retry_is_idempotent(
|
||||
|
||||
first = submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.REJECTED),
|
||||
body=DecisionBody(decision=ApprovalChoice.REJECT),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
second = submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.REJECTED),
|
||||
body=DecisionBody(decision=ApprovalChoice.REJECT),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -252,7 +258,7 @@ def test_submit_decision_different_decision_raises_conflict(
|
||||
|
||||
submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.REJECTED),
|
||||
body=DecisionBody(decision=ApprovalChoice.REJECT),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -260,7 +266,7 @@ def test_submit_decision_different_decision_raises_conflict(
|
||||
with pytest.raises(OnyxError) as exc_info:
|
||||
submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.APPROVED),
|
||||
body=DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -296,7 +302,7 @@ def test_submit_decision_not_found(
|
||||
with pytest.raises(OnyxError) as exc_info:
|
||||
submit_decision(
|
||||
approval_id=target_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.APPROVED),
|
||||
body=DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -327,7 +333,7 @@ def test_submit_decision_pushes_wake_on_redis(
|
||||
|
||||
view = submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.APPROVED),
|
||||
body=DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -398,8 +404,24 @@ def test_submit_session_grant_approves_matching_pending_rows(
|
||||
wakes.append((str(approval_id), decision))
|
||||
|
||||
monkeypatch.setattr(approval_cache, "send_wake", _record_wake)
|
||||
read_grants = action_approval.list_session_grant_action_approvals
|
||||
grant_reads = 0
|
||||
|
||||
response = submit_session_grant(
|
||||
def read_grants_once(
|
||||
db_session: Session, session_id: UUID, *, gated_app_id: int | None
|
||||
) -> list[ActionApproval]:
|
||||
nonlocal grant_reads
|
||||
grant_reads += 1
|
||||
if grant_reads > 1:
|
||||
raise RuntimeError("Database unavailable after the decision commits")
|
||||
return read_grants(db_session, session_id, gated_app_id=gated_app_id)
|
||||
|
||||
monkeypatch.setattr(
|
||||
action_approval, "list_session_grant_action_approvals", read_grants_once
|
||||
)
|
||||
|
||||
response = submit_decision(
|
||||
body=DecisionBody(decision=ApprovalChoice.APPROVE_SESSION),
|
||||
approval_id=current.approval_id,
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
@@ -475,7 +497,7 @@ def test_submit_decision_swallows_transient_wake_failure(
|
||||
|
||||
view = submit_decision(
|
||||
approval_id=approval.approval_id,
|
||||
body=DecisionBody(decision=ApprovalDecision.APPROVED),
|
||||
body=DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user=user,
|
||||
db_session=db_session,
|
||||
)
|
||||
@@ -545,3 +567,312 @@ def test_list_live_approvals_returns_multi_action_view(
|
||||
assert view.app_name == "Linear"
|
||||
assert view.payload == {"query": "mutation { issueCreate { id } }"}
|
||||
assert view.actions == expected_actions
|
||||
|
||||
|
||||
@pytest.mark.parametrize("choice", ["approve_once", "approve_session", "reject"])
|
||||
def test_all_decisions_reject_stale_requests(
|
||||
choice: str,
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
_stub_send_wake_noop(monkeypatch)
|
||||
user = make_user(db_session, email_prefix="stale_decision")
|
||||
session = build_session_with_user(user=user)
|
||||
row = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
)
|
||||
db_session.commit()
|
||||
force_approval_created_at(
|
||||
db_session,
|
||||
row.approval_id,
|
||||
datetime.now(timezone.utc)
|
||||
- timedelta(seconds=SANDBOX_APPROVAL_WAIT_TIMEOUT_SECONDS + 10),
|
||||
)
|
||||
with pytest.raises(OnyxError) as exc:
|
||||
submit_decision(
|
||||
row.approval_id,
|
||||
DecisionBody.model_validate({"decision": choice}),
|
||||
user,
|
||||
db_session,
|
||||
)
|
||||
assert exc.value.error_code == OnyxErrorCode.CONFLICT
|
||||
db_session.refresh(row)
|
||||
assert row.decision is None
|
||||
|
||||
|
||||
def test_session_decision_retry_cannot_change_scope(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
_stub_send_wake_noop(monkeypatch)
|
||||
user = make_user(db_session, email_prefix="decision_scope")
|
||||
session = build_session_with_user(user=user)
|
||||
app = make_external_app(db_session, skill=make_skill(db_session), auth_template={})
|
||||
row = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
target=(GatedAppKind.EXTERNAL_APP, app.id),
|
||||
)
|
||||
db_session.commit()
|
||||
body = DecisionBody(decision=ApprovalChoice.APPROVE_SESSION)
|
||||
first = submit_decision(row.approval_id, body, user, db_session)
|
||||
retry = submit_decision(row.approval_id, body, user, db_session)
|
||||
assert retry.decision == first.decision == ApprovalDecision.APPROVED
|
||||
with pytest.raises(OnyxError) as exc:
|
||||
submit_decision(
|
||||
row.approval_id,
|
||||
DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user,
|
||||
db_session,
|
||||
)
|
||||
assert exc.value.error_code == OnyxErrorCode.CONFLICT
|
||||
|
||||
|
||||
def test_approve_once_cannot_be_upgraded_by_retry(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
_stub_send_wake_noop(monkeypatch)
|
||||
user = make_user(db_session, email_prefix="decision_once")
|
||||
session = build_session_with_user(user=user)
|
||||
row = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
)
|
||||
db_session.commit()
|
||||
submit_decision(
|
||||
row.approval_id,
|
||||
DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user,
|
||||
db_session,
|
||||
)
|
||||
with pytest.raises(OnyxError) as exc:
|
||||
submit_decision(
|
||||
row.approval_id,
|
||||
DecisionBody(decision=ApprovalChoice.APPROVE_SESSION),
|
||||
user,
|
||||
db_session,
|
||||
)
|
||||
assert exc.value.error_code == OnyxErrorCode.CONFLICT
|
||||
db_session.refresh(row)
|
||||
assert row.decided_via == ApprovalDecidedVia.USER
|
||||
|
||||
|
||||
def test_history_retains_expired_requests_and_checks_owner(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
owner = make_user(db_session, email_prefix="history_owner")
|
||||
other = make_user(db_session, email_prefix="history_other")
|
||||
session = build_session_with_user(user=owner)
|
||||
row = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
)
|
||||
try_record_decision(
|
||||
db_session, approval_id=row.approval_id, decision=ApprovalDecision.EXPIRED
|
||||
)
|
||||
db_session.commit()
|
||||
assert list_live_approvals(session.id, owner, db_session).items == []
|
||||
history = list_session_approvals(session.id, owner, db_session)
|
||||
assert [item.decision for item in history.items] == [ApprovalDecision.EXPIRED]
|
||||
assert not history.items[0].is_live
|
||||
with pytest.raises(OnyxError) as exc:
|
||||
list_session_approvals(session.id, other, db_session)
|
||||
assert exc.value.error_code == OnyxErrorCode.NOT_FOUND
|
||||
|
||||
|
||||
def test_unknown_external_action_can_only_be_approved_once(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
_stub_send_wake_noop(monkeypatch)
|
||||
user = make_user(db_session, email_prefix="unknown_action")
|
||||
session = build_session_with_user(user=user)
|
||||
app = make_external_app(db_session, skill=make_skill(db_session), auth_template={})
|
||||
row = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=[action_entry("unspecified")],
|
||||
app_name="Custom",
|
||||
payload={},
|
||||
target=(GatedAppKind.EXTERNAL_APP, app.id),
|
||||
)
|
||||
db_session.commit()
|
||||
view = list_session_approvals(session.id, user, db_session).items[0]
|
||||
assert not view.can_approve_for_session
|
||||
with pytest.raises(OnyxError) as exc:
|
||||
submit_decision(
|
||||
row.approval_id,
|
||||
DecisionBody(decision=ApprovalChoice.APPROVE_SESSION),
|
||||
user,
|
||||
db_session,
|
||||
)
|
||||
assert exc.value.error_code == OnyxErrorCode.INVALID_INPUT
|
||||
result = submit_decision(
|
||||
row.approval_id,
|
||||
DecisionBody(decision=ApprovalChoice.APPROVE_ONCE),
|
||||
user,
|
||||
db_session,
|
||||
)
|
||||
assert result.decision == ApprovalDecision.APPROVED
|
||||
|
||||
|
||||
def test_history_keeps_all_live_requests_when_outcomes_exceed_limit(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
owner = make_user(db_session, email_prefix="busy_history")
|
||||
session = build_session_with_user(user=owner)
|
||||
pending = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
)
|
||||
for _ in range(51):
|
||||
insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
decision=ApprovalDecision.REJECTED,
|
||||
)
|
||||
db_session.commit()
|
||||
|
||||
history = list_session_approvals(session.id, owner, db_session)
|
||||
assert len(history.items) == 51
|
||||
assert [item.approval_id for item in history.items if item.is_live] == [
|
||||
pending.approval_id
|
||||
]
|
||||
|
||||
|
||||
def test_concurrent_session_approvals_do_not_deadlock(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
from onyx.db import action_approval
|
||||
from onyx.db.models import User
|
||||
from shared_configs.contextvars import CURRENT_TENANT_ID_CONTEXTVAR
|
||||
|
||||
user = make_user(db_session, email_prefix="concurrent_grants")
|
||||
session = build_session_with_user(user=user)
|
||||
app = make_external_app(db_session, skill=make_skill(db_session), auth_template={})
|
||||
ids = [
|
||||
insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=[action_entry("slack.chat.post")],
|
||||
app_name="Slack",
|
||||
payload={},
|
||||
target=(GatedAppKind.EXTERNAL_APP, app.id),
|
||||
).approval_id
|
||||
for _ in range(2)
|
||||
]
|
||||
user_id = user.id
|
||||
db_session.commit()
|
||||
barrier = Barrier(2)
|
||||
original = action_approval.get_action_approval_for_user
|
||||
|
||||
def synchronized_read(
|
||||
db: Session, approval_id: UUID, user_id: UUID, *, lock_session: bool = False
|
||||
) -> ActionApproval | None:
|
||||
barrier.wait(timeout=10)
|
||||
return original(db, approval_id, user_id, lock_session=lock_session)
|
||||
|
||||
monkeypatch.setattr(
|
||||
action_approval, "get_action_approval_for_user", synchronized_read
|
||||
)
|
||||
_stub_send_wake_noop(monkeypatch)
|
||||
engine = db_session.get_bind()
|
||||
|
||||
def approve(approval_id: UUID) -> ApprovalDecision | None:
|
||||
token = CURRENT_TENANT_ID_CONTEXTVAR.set(POSTGRES_DEFAULT_SCHEMA_STANDARD_VALUE)
|
||||
try:
|
||||
with Session(engine) as db:
|
||||
db.execute(text("SET LOCAL lock_timeout = '5s'"))
|
||||
caller = db.get(User, user_id)
|
||||
assert caller is not None
|
||||
return submit_decision(
|
||||
approval_id=approval_id,
|
||||
body=DecisionBody(decision=ApprovalChoice.APPROVE_SESSION),
|
||||
user=caller,
|
||||
db_session=db,
|
||||
).decision
|
||||
finally:
|
||||
CURRENT_TENANT_ID_CONTEXTVAR.reset(token)
|
||||
|
||||
with ThreadPoolExecutor(max_workers=2) as pool:
|
||||
results = list(pool.map(approve, ids))
|
||||
assert results == [ApprovalDecision.APPROVED, ApprovalDecision.APPROVED]
|
||||
|
||||
|
||||
def test_status_poll_omits_payloads_and_reports_expiry(
|
||||
db_session: Session,
|
||||
tenant_context: None, # noqa: ARG001
|
||||
build_session_with_user: Callable[..., BuildSession],
|
||||
) -> None:
|
||||
from sqlalchemy import inspect
|
||||
|
||||
from onyx.db.action_approval import (
|
||||
list_session_action_approvals,
|
||||
)
|
||||
|
||||
owner = make_user(db_session, email_prefix="status_owner")
|
||||
other = make_user(db_session, email_prefix="status_other")
|
||||
session = build_session_with_user(user=owner)
|
||||
row = insert_action_approval(
|
||||
db_session,
|
||||
session_id=session.id,
|
||||
actions=_default_actions(),
|
||||
app_name="Slack",
|
||||
payload={"large": "x" * 100_000},
|
||||
)
|
||||
db_session.commit()
|
||||
approval_id = row.approval_id
|
||||
db_session.expunge(row)
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(seconds=180)
|
||||
rows = list_session_action_approvals(
|
||||
db_session, session.id, created_after=cutoff, status_only=True
|
||||
)
|
||||
assert "payload" in inspect(rows[0]).unloaded
|
||||
assert "actions" in inspect(rows[0]).unloaded
|
||||
status = list_session_approval_status(session.id, owner, db_session)
|
||||
assert status[0].model_dump() == {
|
||||
"approval_id": approval_id,
|
||||
"decision": None,
|
||||
"is_live": True,
|
||||
}
|
||||
rows[0].created_at = cutoff - timedelta(seconds=1)
|
||||
db_session.commit()
|
||||
assert not list_session_approval_status(session.id, owner, db_session)[0].is_live
|
||||
with pytest.raises(OnyxError) as exc:
|
||||
list_session_approval_status(session.id, other, db_session)
|
||||
assert exc.value.error_code == OnyxErrorCode.NOT_FOUND
|
||||
@@ -561,6 +561,7 @@ def test_run_fails_when_wake_fails(
|
||||
"approval",
|
||||
"lost_lease",
|
||||
"lost_final_lease",
|
||||
"lost_running_lease",
|
||||
"timeout",
|
||||
"exception",
|
||||
],
|
||||
@@ -616,6 +617,11 @@ def test_output_cards_follow_producer_close_and_precede_terminal_status(
|
||||
*_args: object, **_kwargs: object
|
||||
) -> Generator[SandboxEvent | RequestPermissionRequest, None, None]:
|
||||
lifecycle.append("producer_started")
|
||||
if outcome == "lost_running_lease":
|
||||
assert stub_sandbox_manager.last_prompt_slot is not None
|
||||
stub_sandbox_manager.last_prompt_slot.lost = True
|
||||
# Scheduled turns have no successor; teardown must still abort.
|
||||
assert _kwargs.get("should_abort_on_teardown") is None
|
||||
try:
|
||||
if outcome in {"exception", "timeout"}:
|
||||
yield AgentMessageChunk.model_validate(
|
||||
@@ -651,7 +657,11 @@ def test_output_cards_follow_producer_close_and_precede_terminal_status(
|
||||
db_session.expire_all()
|
||||
assert run.session_id is not None
|
||||
cards = get_output_cards(run.session_id, test_user, db_session).cards
|
||||
if outcome == "lost_lease":
|
||||
if outcome == "lost_running_lease":
|
||||
assert lifecycle == ["baseline", "producer_started", "producer_closed"]
|
||||
assert cards == []
|
||||
assert run.status == ScheduledTaskRunStatus.FAILED
|
||||
elif outcome == "lost_lease":
|
||||
assert lifecycle == ["baseline"]
|
||||
assert cards == []
|
||||
assert run.status == ScheduledTaskRunStatus.FAILED
|
||||
@@ -675,10 +685,14 @@ def test_output_cards_follow_producer_close_and_precede_terminal_status(
|
||||
"exception": ScheduledTaskRunStatus.FAILED,
|
||||
"timeout": ScheduledTaskRunStatus.FAILED,
|
||||
"lost_final_lease": ScheduledTaskRunStatus.FAILED,
|
||||
"approval": ScheduledTaskRunStatus.AWAITING_APPROVAL,
|
||||
"approval": ScheduledTaskRunStatus.FAILED,
|
||||
}[outcome]
|
||||
)
|
||||
|
||||
if outcome == "approval":
|
||||
assert (
|
||||
run.error_class == ScheduledTaskErrorClass.AGENT_PERMISSION_REQUIRED.value
|
||||
)
|
||||
if outcome == "timeout":
|
||||
assert run.error_class == ScheduledTaskErrorClass.TIMEOUT.value
|
||||
if outcome == "lost_final_lease":
|
||||
|
||||
@@ -16,6 +16,7 @@ import pytest
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from onyx.db import action_approval
|
||||
from onyx.db.action_policy import (
|
||||
get_gated_app_id,
|
||||
get_or_create_gated_app_id,
|
||||
@@ -36,7 +37,6 @@ from onyx.db.mcp import (
|
||||
update_mcp_server__no_commit,
|
||||
)
|
||||
from onyx.db.models import BuildSession, GatedApp, MCPServer, Tool
|
||||
from onyx.server.features.build.db import action_approval
|
||||
from tests.external_dependency_unit.conftest import create_test_user
|
||||
|
||||
CraftServerFactory = Callable[..., MCPServer]
|
||||
|
||||
@@ -69,9 +69,9 @@ def _post_decision(
|
||||
return client.post(
|
||||
url,
|
||||
json={
|
||||
"decision": "REJECTED"
|
||||
"decision": "reject"
|
||||
if decision == ApprovalDecision.REJECTED
|
||||
else "APPROVED"
|
||||
else "approve_once"
|
||||
},
|
||||
headers=user.headers,
|
||||
cookies=user.cookies,
|
||||
|
||||
@@ -250,9 +250,9 @@ def _submit_decision_response(
|
||||
return http_client.post(
|
||||
f"{API_SERVER_URL}/build/approvals/{approval_id}/decision",
|
||||
json={
|
||||
"decision": "REJECTED"
|
||||
"decision": "reject"
|
||||
if decision == ApprovalDecision.REJECTED
|
||||
else "APPROVED"
|
||||
else "approve_once"
|
||||
},
|
||||
headers=api_user.headers,
|
||||
cookies=api_user.cookies,
|
||||
|
||||
@@ -159,7 +159,9 @@
|
||||
"GET /build/admin/apps": "admin",
|
||||
"GET /build/admin/apps/built-in/options": "admin",
|
||||
"GET /build/admin/base-instructions": "admin",
|
||||
"GET /build/approvals/sessions/{session_id}": "basic",
|
||||
"GET /build/approvals/sessions/{session_id}/live": "basic",
|
||||
"GET /build/approvals/sessions/{session_id}/status": "basic",
|
||||
"GET /build/apps": "basic",
|
||||
"GET /build/apps/{external_app_id}/oauth/start": "basic",
|
||||
"GET /build/debug/opencode-logs/stream": "basic",
|
||||
@@ -536,7 +538,6 @@
|
||||
"POST /build/admin/apps/built-in": "admin",
|
||||
"POST /build/admin/apps/custom": "admin",
|
||||
"POST /build/approvals/{approval_id}/decision": "basic",
|
||||
"POST /build/approvals/{approval_id}/session-grant": "basic",
|
||||
"POST /build/apps/connect/{request_id}/decision": "basic",
|
||||
"POST /build/apps/oauth/callback": "basic",
|
||||
"POST /build/apps/{external_app_id}/credentials": "basic",
|
||||
|
||||
@@ -47,7 +47,7 @@ Approval persistence and API:
|
||||
and `ScheduledTaskEnabledApp`.
|
||||
- `backend/onyx/db/enums.py` defines `ActionPolicy`,
|
||||
`ApprovalDecision`, and `ApprovalDecidedVia`.
|
||||
- `backend/onyx/server/features/build/db/action_approval.py` owns approval DB
|
||||
- `backend/onyx/db/action_approval.py` owns approval DB
|
||||
operations. `try_record_decision` is the race arbiter.
|
||||
- `backend/onyx/server/features/build/approvals/api.py` exposes live approval
|
||||
listing, approve/reject, and approve-for-session.
|
||||
@@ -406,8 +406,8 @@ Older undecided rows are treated as proxy-crash orphans and excluded from
|
||||
|
||||
### Approve Once
|
||||
|
||||
`POST /api/build/approvals/{approval_id}/decision` accepts only `APPROVED` or
|
||||
`REJECTED`.
|
||||
`POST /api/build/approvals/{approval_id}/decision` accepts `approve_once`,
|
||||
`approve_session`, or `reject`. These commands map to stored approval outcomes.
|
||||
|
||||
The API:
|
||||
|
||||
@@ -423,12 +423,12 @@ The API:
|
||||
### Reject
|
||||
|
||||
Reject follows the same endpoint as approve once, with
|
||||
`decision=REJECTED`. The proxy returns a structured 403 to the sandbox and does
|
||||
`decision=reject`. The proxy returns a structured 403 to the sandbox and does
|
||||
not forward the original upstream request.
|
||||
|
||||
### Approve For Session
|
||||
|
||||
`POST /api/build/approvals/{approval_id}/session-grant` approves the current
|
||||
`POST /api/build/approvals/{approval_id}/decision` with `decision=approve_session` approves the current
|
||||
request and later matching requests for the same session.
|
||||
|
||||
The API:
|
||||
@@ -555,15 +555,11 @@ row, pushes the announce key, and the stream carries only:
|
||||
- one polling `approval:announce:{session_id}` every second
|
||||
|
||||
Both feed the existing SSE stream. `useBuildStreaming` handles the packet by
|
||||
invalidating `SWR_KEYS.buildSessionLiveApprovals(sessionId)`.
|
||||
invalidating `SWR_KEYS.buildSessionApprovals(sessionId)`.
|
||||
|
||||
`useLiveApprovals` fetches
|
||||
`GET /api/build/approvals/sessions/{session_id}/live` and also refreshes every
|
||||
10 seconds to catch removals from expiration, another tab, or another user
|
||||
session.
|
||||
|
||||
`LiveApprovalsRegion` renders live approval rows at the trailing assistant slot
|
||||
in the chat. `ApprovalCard` offers:
|
||||
`SessionApprovalsRegion` fetches
|
||||
`GET /api/build/approvals/sessions/{session_id}` and refreshes every 10 seconds.
|
||||
It displays pending requests, rejections, and expired requests in the chat. `ApprovalCard` offers:
|
||||
|
||||
- Approve once
|
||||
- Approve for session
|
||||
|
||||
@@ -90,8 +90,8 @@ dispatch_due_scheduled_tasks run_scheduled_task(run_id)
|
||||
├─ insert queued run session, task.prompt):
|
||||
├─ next_run_at = croniter _persist_acp_events([event])
|
||||
│ .next(now) if budget exceeded → failed
|
||||
└─ enqueue run_scheduled_task ───► if approval required →
|
||||
(run_id, expires=900, awaiting_approval
|
||||
└─ enqueue run_scheduled_task ───► if agent permission required → failed
|
||||
(run_id, expires=900, proxy action approval → wait up to 180s
|
||||
queue=scheduled_tasks) mark succeeded / failed
|
||||
COMMIT; emit Notification if failed
|
||||
|
||||
@@ -195,13 +195,13 @@ PATCH/POST.
|
||||
```
|
||||
queued ─► running ─► succeeded
|
||||
│ └─► failed (executor crash | budget | ACP error)
|
||||
└─► awaiting_approval ─► (approvals project resumes) ─► running ─► ...
|
||||
└─► failed (agent permission required; no durable resume)
|
||||
|
||||
dispatcher also writes: skipped (prior run still in flight; next_run_at advances)
|
||||
```
|
||||
|
||||
Resume mechanics for `awaiting_approval` are owned by the approvals
|
||||
project; until that ships, treat as terminal-for-display.
|
||||
Agent permission requests stop execution and produce Failed with a clear reason.
|
||||
Proxy action approvals keep their existing 180-second wait. They do not durably pause the run.
|
||||
|
||||
Multiple scheduled runs and the interactive `send_message` path can
|
||||
execute against the same sandbox concurrently — there is no
|
||||
|
||||
@@ -6,7 +6,7 @@ New tasks start with all apps disabled. Enabling an app reveals Approval setting
|
||||
|
||||
Task-disabled apps and workspace-disabled actions remain blocked. When users cannot manage an app's permissions, workspace settings apply. Otherwise task settings apply, with Ask as the fallback for missing actions.
|
||||
|
||||
Approve once and Approve for session remain available. Session approval covers specific actions and cannot override Disabled. The proxy waits up to 180 seconds; it does not durably pause execution.
|
||||
Approve once and Approve for session remain available. Session approval covers specific actions and cannot override Disabled. The proxy waits up to 180 seconds; it does not durably pause execution. Expired and rejected requests remain visible in session approval history.
|
||||
|
||||
The migration converts known actions only in apps a task previously pre-approved. It creates no settings for other apps. Some actions that previously ran automatically may now require approval. The old grant table and runtime cache are removed.
|
||||
|
||||
|
||||
@@ -171,7 +171,7 @@ jest.mock("@/app/craft/components/SkillsStaleNotice", () => ({
|
||||
__esModule: true,
|
||||
default: () => null,
|
||||
}));
|
||||
jest.mock("@/app/craft/components/approvals/LiveApprovalsRegion", () => ({
|
||||
jest.mock("@/app/craft/components/approvals/SessionApprovalsRegion", () => ({
|
||||
__esModule: true,
|
||||
default: () => null,
|
||||
}));
|
||||
|
||||
@@ -53,7 +53,7 @@ import ScheduledRunBanner, {
|
||||
} from "@/app/craft/components/ScheduledRunBanner";
|
||||
import BuildWelcome from "@/app/craft/components/BuildWelcome";
|
||||
import BuildMessageList from "@/app/craft/components/BuildMessageList";
|
||||
import LiveApprovalsRegion from "@/app/craft/components/approvals/LiveApprovalsRegion";
|
||||
import SessionApprovalsRegion from "@/app/craft/components/approvals/SessionApprovalsRegion";
|
||||
import AgentSwitcher from "@/app/craft/components/AgentSwitcher";
|
||||
import SubagentView from "@/app/craft/components/SubagentView";
|
||||
import SandboxStatusIndicator from "@/app/craft/components/SandboxStatusIndicator";
|
||||
@@ -680,7 +680,7 @@ function ChatPanelContent({
|
||||
<span>{t("responseStopped.label")}</span>
|
||||
</div>
|
||||
)}
|
||||
<LiveApprovalsRegion
|
||||
<SessionApprovalsRegion
|
||||
sessionId={sessionId ?? existingSessionId ?? null}
|
||||
/>
|
||||
</>
|
||||
|
||||
@@ -75,6 +75,7 @@ const SLACK_APPROVAL: ApprovalView = {
|
||||
channel: "#sales-ops",
|
||||
text: "Heads up — 3 enterprise deals slipped to Q4 on procurement review. Recovery playbook in thread. 🧵",
|
||||
},
|
||||
expires_at: null,
|
||||
created_at: "2026-05-28T15:42:11Z",
|
||||
decision: null,
|
||||
decided_at: null,
|
||||
|
||||
@@ -46,6 +46,7 @@ function approval(overrides: Partial<ApprovalView>): ApprovalView {
|
||||
actions: [SLACK_POST_MESSAGE],
|
||||
app_name: "Slack",
|
||||
payload: {},
|
||||
expires_at: null,
|
||||
created_at: "2026-05-28T15:42:11Z",
|
||||
decision: null,
|
||||
decided_at: null,
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import { render, screen, setupUser, waitFor } from "@tests/setup/test-utils";
|
||||
import ApprovalCard from "@/app/craft/components/approvals/ApprovalCard";
|
||||
import { postApprovalDecision } from "@/app/craft/services/apiServices";
|
||||
import type { ApprovalView } from "@/app/craft/types/approvals";
|
||||
jest.mock("@/app/craft/services/apiServices", () => ({
|
||||
postApprovalDecision: jest.fn(),
|
||||
ApprovalConflictError: class extends Error {},
|
||||
}));
|
||||
const approval: ApprovalView = {
|
||||
approval_id: "request",
|
||||
session_id: "session",
|
||||
actions: [
|
||||
{
|
||||
action_type: "send",
|
||||
display_name: "Send message",
|
||||
description: "",
|
||||
policy: "ASK",
|
||||
},
|
||||
],
|
||||
app_name: "Slack",
|
||||
payload: {},
|
||||
display_payload: {},
|
||||
created_at: "2026-10-01T00:00:00Z",
|
||||
expires_at: null,
|
||||
decision: null,
|
||||
decided_at: null,
|
||||
is_live: true,
|
||||
};
|
||||
// Decision submission
|
||||
describe("Decision submission", () => {
|
||||
it("uses the shared decision endpoint for session approval", async () => {
|
||||
const user = setupUser();
|
||||
render(<ApprovalCard approval={approval} />);
|
||||
await user.click(screen.getByRole("button", { name: /approve.*session/i }));
|
||||
await waitFor(() =>
|
||||
expect(postApprovalDecision).toHaveBeenCalledWith(
|
||||
"request",
|
||||
"approve_session"
|
||||
)
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// Expiry
|
||||
describe("Expiry", () => {
|
||||
it("shows expired requests without approval controls", () => {
|
||||
render(
|
||||
<ApprovalCard
|
||||
approval={{ ...approval, is_live: false, decision: "EXPIRED" }}
|
||||
/>
|
||||
);
|
||||
expect(screen.getByText("Expired")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Rejected")).not.toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByText("Approval expired. This request did not run.")
|
||||
).toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByRole("button", { name: /approve.*once/i })
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
it("treats abandoned undecided requests outside the live window as expired", () => {
|
||||
render(<ApprovalCard approval={{ ...approval, is_live: false }} />);
|
||||
expect(screen.getByText("Expired")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Rejected")).not.toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByText("Approval expired. This request did not run.")
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("keeps server-live approvals actionable when the device clock is ahead", () => {
|
||||
render(
|
||||
<ApprovalCard
|
||||
approval={{
|
||||
...approval,
|
||||
expires_at: new Date(Date.now() - 600_000).toISOString(),
|
||||
}}
|
||||
/>
|
||||
);
|
||||
expect(
|
||||
screen.getByRole("button", { name: /approve.*once/i })
|
||||
).toBeEnabled();
|
||||
expect(screen.queryByText("Expired")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// Session approval eligibility
|
||||
describe("Session approval eligibility", () => {
|
||||
it("does not offer session approval for unidentified actions", () => {
|
||||
render(
|
||||
<ApprovalCard
|
||||
approval={{ ...approval, can_approve_for_session: false }}
|
||||
/>
|
||||
);
|
||||
expect(
|
||||
screen.queryByRole("button", { name: /approve.*session/i })
|
||||
).not.toBeInTheDocument();
|
||||
expect(
|
||||
screen.getByRole("button", { name: /approve.*once/i })
|
||||
).toBeEnabled();
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,7 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { useTranslations } from "next-intl";
|
||||
import { useFormatter, useTranslations } from "next-intl";
|
||||
import { useSWRConfig } from "swr";
|
||||
|
||||
import { Button, Collapsible, Text, Tooltip } from "@opal/components";
|
||||
@@ -15,10 +15,9 @@ import {
|
||||
import {
|
||||
ApprovalConflictError,
|
||||
postApprovalDecision,
|
||||
postApprovalSessionGrant,
|
||||
} from "@/app/craft/services/apiServices";
|
||||
import {
|
||||
ApprovalAction,
|
||||
ApprovalDecision,
|
||||
ApprovalSubmitDecision,
|
||||
ApprovalView,
|
||||
} from "@/app/craft/types/approvals";
|
||||
@@ -33,47 +32,7 @@ interface ApprovalCardProps {
|
||||
approval: ApprovalView;
|
||||
defaultOpen?: boolean;
|
||||
/** Seed a decided state for Storybook (real approvals start pending). */
|
||||
defaultDecision?: ApprovalSubmitDecision | null;
|
||||
}
|
||||
|
||||
type ApprovalsTranslate = ReturnType<typeof useTranslations<"craft.approvals">>;
|
||||
|
||||
// Single-action: name the action; multi-action: just count them. The
|
||||
// per-action breakdown (with descriptions) is always shown in the body.
|
||||
function approvalHeadline(
|
||||
approval: ApprovalView,
|
||||
t: ApprovalsTranslate
|
||||
): string {
|
||||
if (approval.actions.length === 1) {
|
||||
return t("headline.singleAction", {
|
||||
action: approval.actions[0]!.display_name,
|
||||
app: approval.app_name,
|
||||
});
|
||||
}
|
||||
return t("headline.multiAction", {
|
||||
count: approval.actions.length,
|
||||
app: approval.app_name,
|
||||
});
|
||||
}
|
||||
|
||||
function ActionList({ actions }: { actions: ApprovalAction[] }) {
|
||||
return (
|
||||
<div className="flex flex-col gap-2">
|
||||
{actions.map((action) => (
|
||||
<div
|
||||
key={action.action_type}
|
||||
className="flex flex-col gap-0.5 px-3 py-2 rounded-08 bg-background-neutral-01 border-[0.5px] border-border-01"
|
||||
>
|
||||
<Text font="main-ui-action" color="text-05">
|
||||
{action.display_name}
|
||||
</Text>
|
||||
<Text font="secondary-body" color="text-03">
|
||||
{action.description}
|
||||
</Text>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
defaultDecision?: Exclude<ApprovalDecision, "EXPIRED"> | null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -88,47 +47,58 @@ export default function ApprovalCard({
|
||||
}: ApprovalCardProps) {
|
||||
const t = useTranslations("craft.approvals");
|
||||
const { mutate } = useSWRConfig();
|
||||
const format = useFormatter();
|
||||
const tActionPolicies = useTranslations("craft.actionPolicies");
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
const [errorMessage, setErrorMessage] = useState<string | null>(null);
|
||||
const [isOpen, setIsOpen] = useState(defaultOpen);
|
||||
// Optimistic decision so the comet settles before the row drops from /live.
|
||||
const [decision, setDecision] = useState<ApprovalSubmitDecision | null>(
|
||||
defaultDecision
|
||||
// Optimistic decision so the comet settles before the row is refreshed.
|
||||
const [decision, setDecision] = useState<ApprovalDecision | null>(
|
||||
defaultDecision ?? approval.decision
|
||||
);
|
||||
|
||||
// Guards setState after the post-decision SWR revalidation drops
|
||||
// this row from /live and the card unmounts mid-await.
|
||||
// this row and the card unmounts mid-await.
|
||||
const mountedRef = useRef(true);
|
||||
const settleTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
useEffect(() => {
|
||||
mountedRef.current = true;
|
||||
return () => {
|
||||
mountedRef.current = false;
|
||||
if (settleTimer.current) clearTimeout(settleTimer.current);
|
||||
};
|
||||
}, []);
|
||||
|
||||
const swrKey = SWR_KEYS.buildSessionLiveApprovals(approval.session_id);
|
||||
const decided = decision !== null;
|
||||
const swrKey = SWR_KEYS.buildSessionApprovals(approval.session_id);
|
||||
const expired =
|
||||
decision === "EXPIRED" || (decision === null && !approval.is_live);
|
||||
const decided = decision !== null || expired;
|
||||
const approved = decision === "APPROVED";
|
||||
const headline = approvalHeadline(approval, t);
|
||||
const action = approval.actions[0];
|
||||
const headline =
|
||||
approval.actions.length === 1 && action
|
||||
? t("headline.singleAction", {
|
||||
action: action.display_name,
|
||||
app: approval.app_name,
|
||||
})
|
||||
: t("headline.multiAction", {
|
||||
count: approval.actions.length,
|
||||
app: approval.app_name,
|
||||
});
|
||||
const headerText = decided ? headline : t("header.required", { headline });
|
||||
|
||||
async function submitDecision(
|
||||
next: ApprovalSubmitDecision,
|
||||
request: () => Promise<void>,
|
||||
refetchDelayMs = SETTLE_HOLD_MS
|
||||
) {
|
||||
async function submitDecision(choice: ApprovalSubmitDecision) {
|
||||
setSubmitting(true);
|
||||
setErrorMessage(null);
|
||||
setDecision(next);
|
||||
setDecision(choice === "reject" ? "REJECTED" : "APPROVED");
|
||||
try {
|
||||
await request();
|
||||
if (refetchDelayMs === 0) {
|
||||
await postApprovalDecision(approval.approval_id, choice);
|
||||
if (choice === "approve_session") {
|
||||
void mutate(swrKey);
|
||||
} else {
|
||||
settleTimer.current = setTimeout(() => {
|
||||
void mutate(swrKey);
|
||||
}, refetchDelayMs);
|
||||
}, SETTLE_HOLD_MS);
|
||||
}
|
||||
} catch (e) {
|
||||
// 409 = already resolved (by someone else, or expired by the
|
||||
@@ -218,7 +188,13 @@ export default function ApprovalCard({
|
||||
color="inherit"
|
||||
wordWrap="whitespace-nowrap"
|
||||
>
|
||||
{approved ? t("status.approved") : t("status.rejected")}
|
||||
{t(
|
||||
approved
|
||||
? "status.approved"
|
||||
: expired
|
||||
? "status.expired"
|
||||
: "status.rejected"
|
||||
)}
|
||||
</Text>
|
||||
</div>
|
||||
) : null}
|
||||
@@ -241,58 +217,58 @@ export default function ApprovalCard({
|
||||
</button>
|
||||
</Collapsible.Trigger>
|
||||
</div>
|
||||
{(expired || decision === "REJECTED") && (
|
||||
<div className="px-3 pb-2">
|
||||
<Text font="secondary-body" color="status-error-05">
|
||||
{tActionPolicies(expired ? "expired" : "rejected")}
|
||||
</Text>
|
||||
</div>
|
||||
)}
|
||||
{!decided && approval.expires_at && (
|
||||
<div className="px-3 pb-2">
|
||||
<Text font="secondary-body" color="text-03">
|
||||
{tActionPolicies("deadline", {
|
||||
time: format.dateTime(new Date(approval.expires_at), {
|
||||
hour: "numeric",
|
||||
minute: "2-digit",
|
||||
second: "2-digit",
|
||||
}),
|
||||
})}
|
||||
</Text>
|
||||
</div>
|
||||
)}
|
||||
{!decided && (
|
||||
<div className="flex flex-wrap items-center justify-end gap-1 px-3 pb-2">
|
||||
<Button
|
||||
prominence="primary"
|
||||
size="sm"
|
||||
disabled={submitting}
|
||||
onClick={() =>
|
||||
void submitDecision("APPROVED", async () => {
|
||||
await postApprovalDecision(
|
||||
approval.approval_id,
|
||||
"APPROVED"
|
||||
);
|
||||
})
|
||||
}
|
||||
onClick={() => void submitDecision("approve_once")}
|
||||
aria-label={t("approveOnce.ariaLabel")}
|
||||
>
|
||||
{t("approveOnce.button")}
|
||||
</Button>
|
||||
<Tooltip
|
||||
tooltip={t("approveSession.tooltip")}
|
||||
delayDuration={200}
|
||||
>
|
||||
<Button
|
||||
prominence="secondary"
|
||||
size="sm"
|
||||
disabled={submitting}
|
||||
onClick={() =>
|
||||
void submitDecision(
|
||||
"APPROVED",
|
||||
async () => {
|
||||
await postApprovalSessionGrant(approval.approval_id);
|
||||
},
|
||||
0
|
||||
)
|
||||
}
|
||||
aria-label={t("approveSession.tooltip")}
|
||||
{approval.can_approve_for_session !== false && (
|
||||
<Tooltip
|
||||
tooltip={t("approveSession.tooltip")}
|
||||
delayDuration={200}
|
||||
>
|
||||
{t("approveSession.button")}
|
||||
</Button>
|
||||
</Tooltip>
|
||||
<Button
|
||||
prominence="secondary"
|
||||
size="sm"
|
||||
disabled={submitting}
|
||||
onClick={() => void submitDecision("approve_session")}
|
||||
aria-label={t("approveSession.tooltip")}
|
||||
>
|
||||
{t("approveSession.button")}
|
||||
</Button>
|
||||
</Tooltip>
|
||||
)}
|
||||
<Button
|
||||
prominence="secondary"
|
||||
size="sm"
|
||||
disabled={submitting}
|
||||
onClick={() =>
|
||||
void submitDecision("REJECTED", async () => {
|
||||
await postApprovalDecision(
|
||||
approval.approval_id,
|
||||
"REJECTED"
|
||||
);
|
||||
})
|
||||
}
|
||||
onClick={() => void submitDecision("reject")}
|
||||
aria-label={t("reject.ariaLabel")}
|
||||
>
|
||||
{t("reject.button")}
|
||||
@@ -301,7 +277,21 @@ export default function ApprovalCard({
|
||||
)}
|
||||
<Collapsible.Content>
|
||||
<div className="p-2 flex flex-col gap-3">
|
||||
<ActionList actions={approval.actions} />
|
||||
<div className="flex flex-col gap-2">
|
||||
{approval.actions.map((action) => (
|
||||
<div
|
||||
key={action.action_type}
|
||||
className="flex flex-col gap-0.5 px-3 py-2 rounded-08 bg-background-neutral-01 border-[0.5px] border-border-01"
|
||||
>
|
||||
<Text font="main-ui-action" color="text-05">
|
||||
{action.display_name}
|
||||
</Text>
|
||||
<Text font="secondary-body" color="text-03">
|
||||
{action.description}
|
||||
</Text>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
<PayloadView payload={approval.display_payload} />
|
||||
{errorMessage && (
|
||||
<div className="text-status-error-05">
|
||||
|
||||
@@ -1,38 +0,0 @@
|
||||
"use client";
|
||||
|
||||
import ApprovalCard from "@/app/craft/components/approvals/ApprovalCard";
|
||||
import { useLiveApprovals } from "@/app/craft/hooks/useLiveApprovals";
|
||||
|
||||
interface LiveApprovalsRegionProps {
|
||||
sessionId: string | null;
|
||||
}
|
||||
|
||||
// Renders one ApprovalCard per row returned by /live (already filtered
|
||||
// to undecided + within wait window). No outer logo/wrapper — caller is
|
||||
// responsible for placing this inside the previous assistant message
|
||||
// region so cards visually attach to the agent's last turn.
|
||||
//
|
||||
// SWR cache invalidation is owned by useBuildStreaming's
|
||||
// approval_requested handler and by ApprovalCard itself after a
|
||||
// decision — this component just reads.
|
||||
export default function LiveApprovalsRegion({
|
||||
sessionId,
|
||||
}: LiveApprovalsRegionProps) {
|
||||
const { data } = useLiveApprovals(sessionId);
|
||||
|
||||
if (!sessionId || !data || data.items.length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const sorted = [...data.items].sort(
|
||||
(a, b) => Date.parse(a.created_at) - Date.parse(b.created_at)
|
||||
);
|
||||
|
||||
return (
|
||||
<div data-testid="live-approvals-region" className="flex flex-col gap-3">
|
||||
{sorted.map((approval) => (
|
||||
<ApprovalCard key={approval.approval_id} approval={approval} />
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -0,0 +1,168 @@
|
||||
import {
|
||||
act,
|
||||
render,
|
||||
screen,
|
||||
setupUser,
|
||||
waitFor,
|
||||
} from "@tests/setup/test-utils";
|
||||
import SessionApprovalsRegion from "@/app/craft/components/approvals/SessionApprovalsRegion";
|
||||
import type { ApprovalView } from "@/app/craft/types/approvals";
|
||||
|
||||
// Stored outcomes
|
||||
describe("Approval history", () => {
|
||||
it.each(["APPROVED", "REJECTED"] as const)(
|
||||
"retains the %s outcome after refreshing the session query",
|
||||
async (decision) => {
|
||||
const user = setupUser();
|
||||
let approval: ApprovalView = {
|
||||
approval_id: "request",
|
||||
session_id: "session",
|
||||
actions: [
|
||||
{
|
||||
action_type: "send",
|
||||
display_name: "Send message",
|
||||
description: "",
|
||||
policy: "ASK",
|
||||
},
|
||||
],
|
||||
app_name: "Slack",
|
||||
payload: {},
|
||||
display_payload: {},
|
||||
created_at: "2026-10-01T00:00:00Z",
|
||||
expires_at: null,
|
||||
decision: null,
|
||||
decided_at: null,
|
||||
is_live: true,
|
||||
};
|
||||
const fetchMock = jest
|
||||
.spyOn(global, "fetch")
|
||||
.mockImplementation(async (url, options) => {
|
||||
if (options?.method === "POST") {
|
||||
approval = { ...approval, decision, is_live: false };
|
||||
return new Response(JSON.stringify(approval), { status: 200 });
|
||||
}
|
||||
if (String(url).endsWith("/status")) {
|
||||
return new Response(
|
||||
JSON.stringify([
|
||||
{
|
||||
approval_id: approval.approval_id,
|
||||
decision: approval.decision,
|
||||
is_live: approval.is_live,
|
||||
},
|
||||
]),
|
||||
{ status: 200 }
|
||||
);
|
||||
}
|
||||
return new Response(JSON.stringify({ items: [approval] }), {
|
||||
status: 200,
|
||||
});
|
||||
});
|
||||
render(<SessionApprovalsRegion sessionId="session" />);
|
||||
await user.click(
|
||||
await screen.findByRole("button", {
|
||||
name:
|
||||
decision === "APPROVED" ? /approve.*once/i : "Reject this action",
|
||||
})
|
||||
);
|
||||
await waitFor(
|
||||
() =>
|
||||
expect(
|
||||
fetchMock.mock.calls.filter(
|
||||
([url, options]) =>
|
||||
!options?.method && !String(url).endsWith("/status")
|
||||
)
|
||||
).toHaveLength(2),
|
||||
{ timeout: 2000 }
|
||||
);
|
||||
expect(
|
||||
screen.getByText(
|
||||
decision === "APPROVED"
|
||||
? "Approved"
|
||||
: "Request rejected. This request did not run."
|
||||
)
|
||||
).toBeInTheDocument();
|
||||
fetchMock.mockRestore();
|
||||
}
|
||||
);
|
||||
});
|
||||
|
||||
// Metadata polling
|
||||
describe("Approval status polling", () => {
|
||||
it("polls metadata and reloads full history only when request status changes", async () => {
|
||||
jest.useFakeTimers();
|
||||
let status: {
|
||||
approval_id: string;
|
||||
decision: string | null;
|
||||
is_live: boolean;
|
||||
}[] = [];
|
||||
const fetchMock = jest
|
||||
.spyOn(global, "fetch")
|
||||
.mockImplementation(
|
||||
async (url) =>
|
||||
new Response(
|
||||
JSON.stringify(
|
||||
String(url).endsWith("/status") ? status : { items: [] }
|
||||
),
|
||||
{ status: 200 }
|
||||
)
|
||||
);
|
||||
try {
|
||||
render(<SessionApprovalsRegion sessionId="poll-session" />);
|
||||
await act(async () => {
|
||||
await jest.advanceTimersByTimeAsync(1);
|
||||
});
|
||||
const historyCalls = () =>
|
||||
fetchMock.mock.calls.filter(([url]) =>
|
||||
String(url).endsWith("/poll-session")
|
||||
);
|
||||
expect(historyCalls()).toHaveLength(1);
|
||||
await act(async () => {
|
||||
await jest.advanceTimersByTimeAsync(10_000);
|
||||
});
|
||||
expect(historyCalls()).toHaveLength(1);
|
||||
for (const change of [
|
||||
[{ approval_id: "first", decision: null, is_live: false }],
|
||||
[{ approval_id: "first", decision: "APPROVED", is_live: false }],
|
||||
[{ approval_id: "new", decision: null, is_live: true }],
|
||||
]) {
|
||||
status = change;
|
||||
const before = historyCalls().length;
|
||||
await act(async () => {
|
||||
await jest.advanceTimersByTimeAsync(10_000);
|
||||
});
|
||||
expect(historyCalls()).toHaveLength(before + 1);
|
||||
}
|
||||
} finally {
|
||||
fetchMock.mockRestore();
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("refreshes history when the first status poll finds a newer request", async () => {
|
||||
const fetchMock = jest
|
||||
.spyOn(global, "fetch")
|
||||
.mockImplementation(
|
||||
async (url) =>
|
||||
new Response(
|
||||
JSON.stringify(
|
||||
String(url).endsWith("/status")
|
||||
? [{ approval_id: "new", decision: null, is_live: true }]
|
||||
: { items: [] }
|
||||
),
|
||||
{ status: 200 }
|
||||
)
|
||||
);
|
||||
try {
|
||||
render(<SessionApprovalsRegion sessionId="initial-race" />);
|
||||
await waitFor(() =>
|
||||
expect(
|
||||
fetchMock.mock.calls.filter(([url]) =>
|
||||
String(url).endsWith("/initial-race")
|
||||
)
|
||||
).toHaveLength(2)
|
||||
);
|
||||
} finally {
|
||||
fetchMock.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,64 @@
|
||||
"use client";
|
||||
|
||||
import useSWR from "swr";
|
||||
import { useEffect, useRef } from "react";
|
||||
import isEqual from "lodash/isEqual";
|
||||
import { errorHandlingFetcher } from "@/lib/fetcher";
|
||||
import { SWR_KEYS } from "@/lib/swr-keys";
|
||||
import type {
|
||||
ApprovalListResponse,
|
||||
ApprovalStatusView,
|
||||
} from "@/app/craft/types/approvals";
|
||||
import ApprovalCard from "@/app/craft/components/approvals/ApprovalCard";
|
||||
|
||||
interface SessionApprovalsRegionProps {
|
||||
sessionId: string | null;
|
||||
}
|
||||
|
||||
export default function SessionApprovalsRegion({
|
||||
sessionId,
|
||||
}: SessionApprovalsRegionProps) {
|
||||
const { data, mutate } = useSWR<ApprovalListResponse, Error>(
|
||||
sessionId ? SWR_KEYS.buildSessionApprovals(sessionId) : null,
|
||||
errorHandlingFetcher
|
||||
);
|
||||
const { data: status } = useSWR<ApprovalStatusView[], Error>(
|
||||
sessionId ? SWR_KEYS.buildSessionApprovalStatus(sessionId) : null,
|
||||
errorHandlingFetcher,
|
||||
{ refreshInterval: 10_000 }
|
||||
);
|
||||
const lastStatus = useRef<{
|
||||
sessionId: string | null;
|
||||
items: ApprovalStatusView[];
|
||||
} | null>(null);
|
||||
useEffect(() => {
|
||||
if (!status || !data) return;
|
||||
const previous = lastStatus.current;
|
||||
lastStatus.current = { sessionId, items: status };
|
||||
const baseline =
|
||||
previous?.sessionId === sessionId
|
||||
? previous.items
|
||||
: data.items.map(({ approval_id, decision, is_live }) => ({
|
||||
approval_id,
|
||||
decision,
|
||||
is_live,
|
||||
}));
|
||||
if (!isEqual(baseline, status)) {
|
||||
void mutate();
|
||||
}
|
||||
}, [status, data, sessionId, mutate]);
|
||||
const approvals = [...(data?.items ?? [])].sort(
|
||||
(a, b) => Date.parse(a.created_at) - Date.parse(b.created_at)
|
||||
);
|
||||
if (!sessionId || approvals.length === 0) return null;
|
||||
return (
|
||||
<div data-testid="session-approvals-region" className="flex flex-col gap-3">
|
||||
{approvals.map((approval) => (
|
||||
<ApprovalCard
|
||||
key={`${approval.approval_id}:${approval.decision ?? "pending"}`}
|
||||
approval={approval}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -836,7 +836,7 @@ export function useBuildStreaming({
|
||||
}
|
||||
|
||||
case "approval_requested": {
|
||||
void globalMutate(SWR_KEYS.buildSessionLiveApprovals(sessionId));
|
||||
void globalMutate(SWR_KEYS.buildSessionApprovals(sessionId));
|
||||
break;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,26 +0,0 @@
|
||||
"use client";
|
||||
|
||||
import useSWR from "swr";
|
||||
|
||||
import { fetchLiveApprovals } from "@/app/craft/services/apiServices";
|
||||
import { ApprovalListResponse } from "@/app/craft/types/approvals";
|
||||
import { SWR_KEYS } from "@/lib/swr-keys";
|
||||
|
||||
// How often to re-check /live for server-side state changes the client
|
||||
// has no streaming signal for: an approval expiring (180s server cap),
|
||||
// being decided by another device, or being decided by a collaborator.
|
||||
// New approvals already arrive via the `approval_requested` packet path,
|
||||
// so this poll is the "catch removals" loop, not the primary signal.
|
||||
const LIVE_APPROVALS_REFRESH_MS = 10_000;
|
||||
|
||||
// Thin SWR wrapper. Every component that needs to invalidate this list
|
||||
// can do so with globalMutate on the same SWR key — no callback prop.
|
||||
export function useLiveApprovals(sessionId: string | null) {
|
||||
return useSWR<ApprovalListResponse>(
|
||||
sessionId ? SWR_KEYS.buildSessionLiveApprovals(sessionId) : null,
|
||||
sessionId ? () => fetchLiveApprovals(sessionId) : null,
|
||||
{
|
||||
refreshInterval: LIVE_APPROVALS_REFRESH_MS,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -23,9 +23,8 @@ import {
|
||||
type OutputCard,
|
||||
} from "@/app/craft/types/streamingTypes";
|
||||
import {
|
||||
ApprovalListResponse,
|
||||
ApprovalSubmitDecision,
|
||||
ApprovalView,
|
||||
ApprovalSubmitDecision,
|
||||
} from "@/app/craft/types/approvals";
|
||||
import {
|
||||
RATE_LIMITED_ERROR_CODE,
|
||||
@@ -1012,20 +1011,6 @@ export async function fetchPptxPreview(
|
||||
// Approvals API
|
||||
// =============================================================================
|
||||
|
||||
export async function fetchLiveApprovals(
|
||||
sessionId: string
|
||||
): Promise<ApprovalListResponse> {
|
||||
const res = await fetch(
|
||||
`${BUILD_API_BASE}/approvals/sessions/${sessionId}/live`
|
||||
);
|
||||
|
||||
if (!res.ok) {
|
||||
throw new Error(`Failed to fetch live approvals: ${res.status}`);
|
||||
}
|
||||
|
||||
return res.json();
|
||||
}
|
||||
|
||||
// Lets the approval card distinguish "already resolved" from a generic
|
||||
// network error so both can flow through the same SWR revalidation
|
||||
// while keeping logs clean.
|
||||
@@ -1064,29 +1049,6 @@ export async function postApprovalDecision(
|
||||
return res.json();
|
||||
}
|
||||
|
||||
export async function postApprovalSessionGrant(
|
||||
approvalId: string
|
||||
): Promise<ApprovalView> {
|
||||
const res = await fetch(
|
||||
`${BUILD_API_BASE}/approvals/${approvalId}/session-grant`,
|
||||
{
|
||||
method: "POST",
|
||||
}
|
||||
);
|
||||
|
||||
if (res.status === 409) {
|
||||
const body: ErrorResponseBody = await res.json().catch(() => ({}));
|
||||
throw new ApprovalConflictError(body.detail ?? "decision conflict");
|
||||
}
|
||||
if (!res.ok) {
|
||||
const errorData: ErrorResponseBody = await res.json().catch(() => ({}));
|
||||
throw new Error(
|
||||
errorData.detail || `Failed to approve for session: ${res.status}`
|
||||
);
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// User Library API
|
||||
// =============================================================================
|
||||
|
||||
@@ -1,19 +1,18 @@
|
||||
import type { ActionPolicy } from "@/app/craft/v1/apps/registry";
|
||||
|
||||
export type ApprovalDecision = "APPROVED" | "REJECTED" | "EXPIRED";
|
||||
|
||||
// Server-only EXPIRED is excluded; clients can only submit a yes/no.
|
||||
export type ApprovalSubmitDecision = "APPROVED" | "REJECTED";
|
||||
|
||||
// Mirrors backend `ActionPolicy`. DENY blocks before persistence, so
|
||||
// persisted entries are always ASK or ALWAYS — included only to match
|
||||
// the backend enum exactly.
|
||||
export type ApprovalActionPolicy = "ASK" | "ALWAYS" | "DENY";
|
||||
export type ApprovalSubmitDecision =
|
||||
| "approve_once"
|
||||
| "approve_session"
|
||||
| "reject";
|
||||
|
||||
// Mirrors backend `ActionMatch`.
|
||||
export interface ApprovalAction {
|
||||
action_type: string;
|
||||
display_name: string;
|
||||
description: string;
|
||||
policy: ApprovalActionPolicy;
|
||||
policy: ActionPolicy;
|
||||
}
|
||||
|
||||
export interface ApprovalView {
|
||||
@@ -25,6 +24,8 @@ export interface ApprovalView {
|
||||
payload: Record<string, unknown>;
|
||||
display_payload: Record<string, unknown>;
|
||||
created_at: string;
|
||||
expires_at: string | null;
|
||||
can_approve_for_session?: boolean;
|
||||
decision: ApprovalDecision | null;
|
||||
decided_at: string | null;
|
||||
is_live: boolean;
|
||||
@@ -33,3 +34,9 @@ export interface ApprovalView {
|
||||
export interface ApprovalListResponse {
|
||||
items: ApprovalView[];
|
||||
}
|
||||
|
||||
export interface ApprovalStatusView {
|
||||
approval_id: string;
|
||||
decision: ApprovalDecision | null;
|
||||
is_live: boolean;
|
||||
}
|
||||
@@ -90,7 +90,11 @@ interface SummaryCellProps {
|
||||
function SummaryCell({ row }: SummaryCellProps) {
|
||||
const tReason = useTranslations("craft.tasks.runHistory.nonClickable");
|
||||
const reason = getNonClickableReason(row, tReason);
|
||||
const raw = row.summary ?? row.skip_reason ?? row.error_class;
|
||||
const tActionPolicies = useTranslations("craft.actionPolicies");
|
||||
const raw =
|
||||
row.error_class === "agent_permission_required"
|
||||
? tActionPolicies("agentPermissionRequired")
|
||||
: (row.summary ?? row.skip_reason ?? row.error_class);
|
||||
// Heading strip must run before toPlainString collapses newlines.
|
||||
const stripped = raw
|
||||
? toPlainString(markdown(raw.replace(/^#{1,6}\s+/gm, "")))
|
||||
|
||||
@@ -241,8 +241,10 @@ export const SWR_KEYS = {
|
||||
buildExternalAppsAdmin: "/api/build/admin/apps",
|
||||
buildExternalAppsBuiltInOptions: "/api/build/admin/apps/built-in/options",
|
||||
buildBaseInstructions: "/api/build/admin/base-instructions",
|
||||
buildSessionLiveApprovals: (sessionId: string) =>
|
||||
`/api/build/approvals/sessions/${sessionId}/live`,
|
||||
buildSessionApprovalStatus: (sessionId: string) =>
|
||||
`/api/build/approvals/sessions/${sessionId}/status`,
|
||||
buildSessionApprovals: (sessionId: string) =>
|
||||
`/api/build/approvals/sessions/${sessionId}`,
|
||||
|
||||
// ── Token Rate Limits ─────────────────────────────────────────────────────
|
||||
globalTokenRateLimits: "/api/admin/token-rate-limits/global",
|
||||
|
||||
Reference in new issue
Block a user