feat: add Pandora read-only action runner#135
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR extends the Pandora Operator Action Center with an approval/execution lifecycle. It adds new approve, execute, and events API routes; hardens cancel/dry-run routes against client userId overrides; introduces approved/executing statuses with transition guards; updates dashboard data to include event previews and status counts; adds UI components for controls and timelines; adds a DB migration for the new status set; and updates docs and tests accordingly. ChangesOperator Action Approval Workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ApproveRoute
participant ExecuteRoute
participant OperatorActionService
participant Supabase
Client->>ApproveRoute: POST /approve
ApproveRoute->>OperatorActionService: approveOperatorAction(userId, actionId)
OperatorActionService->>Supabase: assertTransition -> approved
Supabase-->>OperatorActionService: updated row
OperatorActionService-->>ApproveRoute: action
ApproveRoute-->>Client: { ok: true, action }
Client->>ExecuteRoute: POST /execute
ExecuteRoute->>OperatorActionService: executeApprovedOperatorAction(userId, actionId)
OperatorActionService->>Supabase: assertTransition approved -> executing
OperatorActionService->>Supabase: persist completed/failed + timestamps
Supabase-->>OperatorActionService: updated row
OperatorActionService-->>ExecuteRoute: { action, result }
ExecuteRoute-->>Client: { ok: true, action, result }
sequenceDiagram
participant User
participant OperatorActionControls
participant APIRoute
participant Browser
User->>OperatorActionControls: click Approve/Execute/Cancel
OperatorActionControls->>APIRoute: POST /api/pandora/operator-actions/{id}/{verb}
APIRoute-->>OperatorActionControls: { ok, action }
OperatorActionControls->>Browser: window.location.reload()
Related PRs: None identified. Suggested labels: pandora, api, backend, frontend Suggested reviewers: besfeng23 🐰 A dry-run once, now approved with care, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66f33ba157
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| alter table public.pandora_operator_actions | ||
| add constraint pandora_operator_actions_status_check | ||
| check (status in ('proposed','dry_ran','approved','executing','completed','blocked','failed','cancelled')); |
There was a problem hiding this comment.
In any deployment that already has operator actions created with mode = 'queued_only', the previous service stored those rows with status = 'queued' and the original constraint allowed that value. This migration removes queued from the allowed set without updating existing rows first, so PostgreSQL will reject the new check constraint when such rows exist and the rollout will be blocked. Please migrate existing queued rows to a supported state or keep queued allowed until cleanup is complete.
Useful? React with 👍 / 👎.
| if (existing) return existing; | ||
| const now = new Date().toISOString(); const requestId = randomUUID(); | ||
| const row = { id: randomUUID(), user_id: input.userId, request_id: requestId, idempotency_key: idempotencyKey, action_type: input.actionType, namespace: input.namespace ?? null, mode, status: mode === "queued_only" ? "queued" : "proposed", title: titleFor(input.actionType), description: "Operator-proposed safe action. Initial implementation is dry-run or queued-only and cannot mutate core memory truth tables.", payload: input.payload ?? {}, result: {}, warnings: [], created_at: now, updated_at: now, approved_at: null, completed_at: null, failed_at: null }; | ||
| const row = { id: randomUUID(), user_id: input.userId, request_id: requestId, idempotency_key: idempotencyKey, action_type: input.actionType, namespace: input.namespace ?? null, mode, status: "proposed", title: titleFor(input.actionType), description: "Operator-proposed safe action. Initial implementation is dry-run or queued-only and cannot mutate core memory truth tables.", payload: input.payload ?? {}, result: {}, warnings: [], created_at: now, updated_at: now, approved_at: null, completed_at: null, failed_at: null }; |
There was a problem hiding this comment.
When an API client still submits mode: "queued_only" (the route and assertMode continue to accept it), this now stores the action as proposed. The new transition table allows proposed -> approved -> executing, so a request that explicitly chose queued-only mode can be approved and executed by the new runner instead of remaining queue-only. Either reject queued_only for runner actions or preserve a non-executable queued state for that mode.
Useful? React with 👍 / 👎.
| const result = envelope({ ...executing, status: "completed" }, built.result, built.warnings); | ||
| const completed = await single(client.from("pandora_operator_actions").update({ status: "completed", result, warnings: built.warnings, completed_at: completedAt, updated_at: completedAt }).eq("user_id", input.userId).eq("id", input.actionId).select("*").single()); | ||
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "completed", message: "Approved read-only execution completed. No core memory mutation was performed.", metadata: result }); | ||
| return completed ?? { ...executing, status: "completed", result, warnings: built.warnings, completed_at: completedAt, updated_at: completedAt }; |
There was a problem hiding this comment.
If the terminal update fails after the read-only work succeeds (for example because the migration/grants are missing or RLS rejects the update), single() returns null and this fallback returns a synthetic completed action anyway. The route then reports { ok: true } even though the database may still show the action as approved/executing and no durable completion row was written, which contradicts the runner's audit-backed completion semantics. Please propagate the update error instead of fabricating the terminal state.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
components/pandora/OperatorActionEnvelope.tsx (1)
12-14: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win"Pending" is misleading for actions that will never reach that stage.
For a
cancelled,failed, orblockedaction that never got approved,approved_at ?? "Pending"implies approval is still forthcoming. Consider distinguishing "will not happen" (e.g., "N/A") from "not yet happened" (e.g., "Pending") based onaction.status.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/pandora/OperatorActionEnvelope.tsx` around lines 12 - 14, The `OperatorActionEnvelope` status timestamps currently use `?? "Pending"` for `approved_at`, `completed_at`, and `failed_at`, which can mislead users for terminal states like cancelled, failed, or blocked. Update the render logic to branch on `action.status` so `approved_at` shows a non-applicable label such as “N/A” when approval will never happen, while still showing “Pending” only for actions that are still in progress. Keep the change localized to the timestamp display in `OperatorActionEnvelope` and use the existing `action.status` field to decide the label.lib/services/pandora-dashboard-service.ts (1)
44-53: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winN+1 event queries per dashboard load.
For every operator action returned (up to 10), a separate round trip to
pandora_operator_action_eventsis issued. This is an N+1 query pattern on a dashboard read path; a single query filteringaction_idvia.in(...)(grouping/slicing the top-3-per-action client-side) would cut this to one round trip.♻️ Suggested single-query approach
- const actionEvents = await Promise.all(operatorActions.map(async (action) => { - try { - const result = await client.from("pandora_operator_action_events").select("*").eq("user_id", input.userId).eq("action_id", action.id).order("created_at", { ascending: false }).limit(3); - if (result.error) { actionWarnings.push(`operator action events unavailable for ${action.id}; showing action without timeline.`); return { actionId: action.id, events: [] as Row[] }; } - return { actionId: action.id, events: Array.isArray(result.data) ? result.data : [] }; - } catch { - actionWarnings.push(`operator action events unavailable for ${action.id}; showing action without timeline.`); - return { actionId: action.id, events: [] as Row[] }; - } - })); - const eventsByAction = new Map(actionEvents.map((item) => [item.actionId, item.events])); + const actionIds = operatorActions.map((action) => action.id); + const eventsByAction = new Map<string, Row[]>(); + if (actionIds.length > 0) { + try { + const result = await client.from("pandora_operator_action_events").select("*").eq("user_id", input.userId).in("action_id", actionIds).order("created_at", { ascending: false }); + if (result.error) { + actionWarnings.push("operator action events unavailable; showing actions without timeline."); + } else { + for (const row of (Array.isArray(result.data) ? result.data : [])) { + const list = eventsByAction.get(row.action_id) ?? []; + if (list.length < 3) list.push(row); + eventsByAction.set(row.action_id, list); + } + } + } catch { + actionWarnings.push("operator action events unavailable; showing actions without timeline."); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/services/pandora-dashboard-service.ts` around lines 44 - 53, The action event fetch in pandoraDashboardService is using one query per operator action, creating an N+1 read path. Refactor the Promise.all block around client.from("pandora_operator_action_events") to fetch all needed events in a single query using .in(...) on action ids, then group the results by actionId and apply the top-3 slicing client-side before building the actionEvents array. Keep the existing warning behavior in the actionWarnings path and preserve the actionId/events shape returned from this section.components/pandora/OperatorActionList.tsx (1)
6-6: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
blockedandexecutingshare the same color.Both map to
"amber", making an in-progress execution visually indistinguishable from a blocked/problem state in the status pill — potentially confusing for an operator scanning action history for issues that need attention.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/pandora/OperatorActionList.tsx` at line 6, The status color mapping in OperatorActionList uses the same amber color for both blocked and executing, which makes active work look like an error state. Update the colors Record in OperatorActionList.tsx so executing has a distinct non-blocking color while blocked keeps the warning/error color, and verify the status pill rendering still clearly differentiates action states.components/pandora/OperatorActionControls.tsx (1)
4-7: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo error handling on action transition requests.
postActionreloads the page regardless of the fetch outcome. If approve/execute/cancel/dry-run is rejected (e.g., invalid transition, ownership check failure), the operator gets no feedback — the page just reloads with the action unchanged and no indication of why.♻️ Suggested improvement
async function postAction(actionId: string, verb: "dry-run" | "approve" | "execute" | "cancel") { - await fetch(`/api/pandora/operator-actions/${actionId}/${verb}`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) }); - window.location.reload(); + const response = await fetch(`/api/pandora/operator-actions/${actionId}/${verb}`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) }); + if (!response.ok) { + const body = await response.json().catch(() => ({})); + window.alert(body.error ?? `Failed to ${verb} action.`); + return; + } + window.location.reload(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/pandora/OperatorActionControls.tsx` around lines 4 - 7, The postAction helper currently reloads the page even when the operator-action request fails, so rejected transitions provide no feedback. Update postAction in OperatorActionControls to check the fetch response before calling window.location.reload, handle non-OK responses and network errors, and surface a clear message to the user (e.g. through existing UI state/notification handling) when approve, execute, cancel, or dry-run is rejected.app/api/pandora/operator-actions/[id]/approve/route.ts (2)
10-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCatch-all maps every failure to HTTP 400, hiding not-found and server errors.
The
catchfunnels not-found (getOperatorActionthrows), invalid-transition (assertTransition), and genuine infra failures (Supabase/network) into a single400. That mislabels server faults as client errors, so 5xx-based alerting and client retry logic won't trigger, and the rawerror.messageis returned without any server-side log. Consider classifying:404for not-found,409/400for invalid state,500(logged) for the rest. This pattern is shared by execute/cancel/dry-run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/pandora/operator-actions/`[id]/approve/route.ts around lines 10 - 11, The approve route’s catch-all in the approveOperatorAction flow is mapping not-found, invalid-transition, and infrastructure failures to the same 400 response. Update the error handling around the route handler in approve/route.ts so it distinguishes expected domain errors from unexpected failures, returning 404 for missing actions, 400/409 for invalid state transitions, and 500 for Supabase/network or other unexpected errors; also add server-side logging for the unexpected branch. Use the existing approveOperatorAction, createSupabaseServerClient, and NextResponse.json path to keep the classification close to the current handler logic, and apply the same pattern to the matching execute/cancel/dry-run routes.
6-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the shared POST scaffolding.
The body-parse →
assertNoClientUserIdOverride→resolvePandoraServerSession→ try/catch envelope is duplicated near-verbatim across approve/execute/cancel/dry-run. A small wrapper (e.g.withOperatorActionSession(handler)) would centralize the auth contract and the error-mapping fix above, reducing drift risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/pandora/operator-actions/`[id]/approve/route.ts around lines 6 - 12, The POST handler scaffolding in approve/execute/cancel/dry-run is duplicated and should be centralized to avoid drift. Extract the shared body parsing, assertNoClientUserIdOverride, resolvePandoraServerSession, and try/catch/error-mapping flow into a reusable wrapper such as withOperatorActionSession, then have POST in approve/route.ts delegate only the action-specific work (approveOperatorAction and id lookup) through that helper. Keep the wrapper responsible for consistent blocker and error responses across all operator action routes.tests/unit/pandora-operator-action-service.test.ts (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest title claims
dry_rancoverage that the test body doesn't exercise.The test is titled "approves proposed and dry_ran actions..." but only ever creates a
proposedaction and approves it directly; it never transitions the action todry_ranbefore callingapproveOperatorAction. If a regression breaks approving fromdry_ranspecifically, this test won't catch it despite implying it does.♻️ Suggested addition
it("approves proposed and dry_ran actions, then executes approved read-only action", async () => { const s=store(); const c=client(s); const a=await proposeOperatorAction(c,{userId:USER, actionType:"verify_pack_supersession", namespace:"real_life"}); const approved=await approveOperatorAction(c,{userId:USER, actionId:a.id}); expect(approved.status).toBe("approved"); const completed=await executeApprovedOperatorAction(c,{userId:USER, actionId:a.id}); expect(completed.status).toBe("completed"); expect(completed.result.no_mutation_performed).toBe(true); expect(JSON.stringify(completed.result)).toContain("pack_supersession"); expect(s.memory_context_packs).toHaveLength(2); }); + it("approves a dry_ran action", async () => { const s=store(); const c=client(s); const a=await proposeOperatorAction(c,{userId:USER, actionType:"verify_pack_supersession", namespace:"real_life"}); await dryRunOperatorAction(c,{userId:USER, actionId:a.id}); const approved=await approveOperatorAction(c,{userId:USER, actionId:a.id}); expect(approved.status).toBe("approved"); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/pandora-operator-action-service.test.ts` at line 18, The test name overstates coverage because `proposeOperatorAction`, `approveOperatorAction`, and `executeApprovedOperatorAction` only exercise the proposed flow and never the `dry_ran` state. Update this test in `tests/unit/pandora-operator-action-service.test.ts` so it explicitly transitions the created action through `dry_ran` before approval, or split it into separate cases that cover proposed and dry_ran approval paths. Keep the assertions on the completed read-only execution, but make sure the test body matches the `dry_ran` behavior claimed by the title.tests/unit/pandora-operator-action-ui.test.tsx (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssertion is trivially satisfied by the always-rendered status-count grid, not the action list.
countsByStatusfixture includes all 8 status names with fixed values, andOperatorActionCenterCardunconditionally renders everycountsByStatuskey as text (<span>{status}</span>) regardless of theactionsprop. Soexpect(html).toContain(s)for"approved"/"executing"/etc. passes purely from that grid — this test doesn't actually verify that per-action status rendering (e.g., inOperatorActionList) works correctly for the constructedactionsarray. A regression there would go undetected.♻️ Suggested strengthening
- it("renders approved/executing/completed/failed/cancelled statuses", () => { const actions=["approved","executing","completed","failed","cancelled"].map((status,i)=>({...base,id:String(i),status: status as OperatorActionStatus})); const html=renderToStaticMarkup(<OperatorActionCenterCard data={{actions,warnings:[], countsByStatus}} />); for (const s of ["approved","executing","completed","failed","cancelled"]) expect(html).toContain(s); }); + it("renders approved/executing/completed/failed/cancelled statuses", () => { const actions=["approved","executing","completed","failed","cancelled"].map((status,i)=>({...base,id:String(i),status: status as OperatorActionStatus})); const html=renderToStaticMarkup(<OperatorActionCenterCard data={{actions,warnings:[], countsByStatus}} />); for (const s of ["approved","executing","completed","failed","cancelled"]) expect((html.match(new RegExp(s,"g")) ?? []).length).toBeGreaterThan(1); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/pandora-operator-action-ui.test.tsx` at line 12, The status-rendering test is too weak because it only matches the always-rendered counts grid in OperatorActionCenterCard, not the per-action rows. Update the test to assert against the action list rendering path, using OperatorActionList or the action section inside OperatorActionCenterCard, and verify each constructed action status is actually displayed from the actions array. Keep countsByStatus from satisfying the assertion by narrowing the query to the action item markup or by using a fixture that isolates the list output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/pandora/operator-actions/`[id]/events/route.ts:
- Line 9: The catch-all in the events route is masking real server failures by
always returning a 404, and it also disagrees with the other operator-action
handlers. Update the `catch` block in the `events` route so only the expected
not-found case maps to the same status used by sibling routes like the
approve/execute/cancel/dry-run handlers, and let unexpected errors from
`NextResponse.json` or the Supabase/client setup surface as a 500 instead. Keep
the response shape consistent while distinguishing missing actions from genuine
infrastructure errors.
In `@app/api/pandora/operator-actions/`[id]/execute/route.ts:
- Line 10: The execute route in the operator-actions handler always returns ok:
true from the NextResponse.json call even when executeApprovedOperatorAction
produces a failed action. Update the route logic around
executeApprovedOperatorAction and the returned action.result so ok is derived
from the terminal action.status, or return a non-200 response when the action
ends in failed, while keeping the success path unchanged.
In `@components/pandora/OperatorActionControls.tsx`:
- Around line 10-15: The Cancel button in OperatorActionControls is still shown
for blocked actions, even though cancelOperatorAction() cannot transition
blocked to cancelled. Update the terminal/status gating in
OperatorActionControls so blocked is treated like a non-cancelable state, and
only render the Cancel affordance for statuses that can actually be cancelled;
keep the logic aligned with postAction and cancelOperatorAction so the UI never
exposes a dead action.
In `@docs/pandora-operator-action-center.md`:
- Around line 31-39: The action-center transition table is missing the `blocked`
lifecycle state in the terminal-state row, which creates a mismatch with the
service behavior. Update the state transition documentation in the transition
table so `blocked` is listed alongside `completed`, `failed`, and `cancelled` as
terminal, using the existing action-state terminology in the table to keep the
lifecycle description accurate.
In `@lib/services/pandora-dashboard-service.ts`:
- Around line 55-56: The status bucket list is duplicated in pandora dashboard
logic and related UI fixtures, so update the counting code in
PandoraDashboardService to use a shared source of truth instead of a local
literal. Create or reuse a single OPERATOR_ACTION_STATUSES constant (for example
in types.ts or the service module) and import it in actionStatuses,
OperatorActionCenterCard, and mock-data so all counts/empty states stay aligned
when OperatorActionStatus changes.
In `@lib/services/pandora-operator-action-service.ts`:
- Around line 99-122: Guard the status transitions in
executeApprovedOperatorAction by making each database update conditional on the
expected current status, not just userId and actionId. Update the
approved-to-executing and executing-to-completed/failed writes to include the
current status in the filter, and treat a null return from single(...) as a lost
race so the action is not advanced twice or double-logged.
---
Nitpick comments:
In `@app/api/pandora/operator-actions/`[id]/approve/route.ts:
- Around line 10-11: The approve route’s catch-all in the approveOperatorAction
flow is mapping not-found, invalid-transition, and infrastructure failures to
the same 400 response. Update the error handling around the route handler in
approve/route.ts so it distinguishes expected domain errors from unexpected
failures, returning 404 for missing actions, 400/409 for invalid state
transitions, and 500 for Supabase/network or other unexpected errors; also add
server-side logging for the unexpected branch. Use the existing
approveOperatorAction, createSupabaseServerClient, and NextResponse.json path to
keep the classification close to the current handler logic, and apply the same
pattern to the matching execute/cancel/dry-run routes.
- Around line 6-12: The POST handler scaffolding in
approve/execute/cancel/dry-run is duplicated and should be centralized to avoid
drift. Extract the shared body parsing, assertNoClientUserIdOverride,
resolvePandoraServerSession, and try/catch/error-mapping flow into a reusable
wrapper such as withOperatorActionSession, then have POST in approve/route.ts
delegate only the action-specific work (approveOperatorAction and id lookup)
through that helper. Keep the wrapper responsible for consistent blocker and
error responses across all operator action routes.
In `@components/pandora/OperatorActionControls.tsx`:
- Around line 4-7: The postAction helper currently reloads the page even when
the operator-action request fails, so rejected transitions provide no feedback.
Update postAction in OperatorActionControls to check the fetch response before
calling window.location.reload, handle non-OK responses and network errors, and
surface a clear message to the user (e.g. through existing UI state/notification
handling) when approve, execute, cancel, or dry-run is rejected.
In `@components/pandora/OperatorActionEnvelope.tsx`:
- Around line 12-14: The `OperatorActionEnvelope` status timestamps currently
use `?? "Pending"` for `approved_at`, `completed_at`, and `failed_at`, which can
mislead users for terminal states like cancelled, failed, or blocked. Update the
render logic to branch on `action.status` so `approved_at` shows a
non-applicable label such as “N/A” when approval will never happen, while still
showing “Pending” only for actions that are still in progress. Keep the change
localized to the timestamp display in `OperatorActionEnvelope` and use the
existing `action.status` field to decide the label.
In `@components/pandora/OperatorActionList.tsx`:
- Line 6: The status color mapping in OperatorActionList uses the same amber
color for both blocked and executing, which makes active work look like an error
state. Update the colors Record in OperatorActionList.tsx so executing has a
distinct non-blocking color while blocked keeps the warning/error color, and
verify the status pill rendering still clearly differentiates action states.
In `@lib/services/pandora-dashboard-service.ts`:
- Around line 44-53: The action event fetch in pandoraDashboardService is using
one query per operator action, creating an N+1 read path. Refactor the
Promise.all block around client.from("pandora_operator_action_events") to fetch
all needed events in a single query using .in(...) on action ids, then group the
results by actionId and apply the top-3 slicing client-side before building the
actionEvents array. Keep the existing warning behavior in the actionWarnings
path and preserve the actionId/events shape returned from this section.
In `@tests/unit/pandora-operator-action-service.test.ts`:
- Line 18: The test name overstates coverage because `proposeOperatorAction`,
`approveOperatorAction`, and `executeApprovedOperatorAction` only exercise the
proposed flow and never the `dry_ran` state. Update this test in
`tests/unit/pandora-operator-action-service.test.ts` so it explicitly
transitions the created action through `dry_ran` before approval, or split it
into separate cases that cover proposed and dry_ran approval paths. Keep the
assertions on the completed read-only execution, but make sure the test body
matches the `dry_ran` behavior claimed by the title.
In `@tests/unit/pandora-operator-action-ui.test.tsx`:
- Line 12: The status-rendering test is too weak because it only matches the
always-rendered counts grid in OperatorActionCenterCard, not the per-action
rows. Update the test to assert against the action list rendering path, using
OperatorActionList or the action section inside OperatorActionCenterCard, and
verify each constructed action status is actually displayed from the actions
array. Keep countsByStatus from satisfying the assertion by narrowing the query
to the action item markup or by using a fixture that isolates the list output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48e43ef8-b346-4435-b8b7-32f3b9423bd7
📒 Files selected for processing (20)
app/api/pandora/operator-actions/[id]/approve/route.tsapp/api/pandora/operator-actions/[id]/cancel/route.tsapp/api/pandora/operator-actions/[id]/dry-run/route.tsapp/api/pandora/operator-actions/[id]/events/route.tsapp/api/pandora/operator-actions/[id]/execute/route.tscomponents/pandora/OperatorActionCenterCard.tsxcomponents/pandora/OperatorActionControls.tsxcomponents/pandora/OperatorActionEnvelope.tsxcomponents/pandora/OperatorActionList.tsxcomponents/pandora/OperatorActionTimeline.tsxcomponents/pandora/mock-data.tscomponents/pandora/types.tsdocs/pandora-operator-action-center.mddocs/pandora-readonly-action-runner.mdlib/services/pandora-dashboard-service.tslib/services/pandora-operator-action-service.tssupabase/migrations/20260703010000_pandora_operator_action_runner_statuses.sqltests/unit/pandora-operator-action-guards.test.tstests/unit/pandora-operator-action-service.test.tstests/unit/pandora-operator-action-ui.test.tsx
| export async function GET(request: NextRequest, context: { params: Promise<{ id: string }> }) { | ||
| const session = await resolvePandoraServerSession({ request }); if (!session.ok) return NextResponse.json({ ok: false, blockers: session.blockers }, { status: 401 }); | ||
| try { const { id } = await context.params; const supabase = await createSupabaseServerClient(); const events = await listOperatorActionEvents(supabase as unknown as OperatorActionDbClient, { userId: session.session.userId, actionId: id }); return NextResponse.json({ ok: true, events }); } | ||
| catch (error) { return NextResponse.json({ ok: false, error: error instanceof Error ? error.message : "Action not found" }, { status: 404 }); } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
404 catch-all also masks server errors and diverges from sibling routes.
Any failure here (including a Supabase/client construction fault) returns 404, while approve/execute/cancel/dry-run map the same not-found condition to 400. Beyond hiding 5xx faults, the inconsistent not-found status across the operator-action routes will complicate client handling. Align the not-found status and let genuine infra failures surface as 500.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/pandora/operator-actions/`[id]/events/route.ts at line 9, The
catch-all in the events route is masking real server failures by always
returning a 404, and it also disagrees with the other operator-action handlers.
Update the `catch` block in the `events` route so only the expected not-found
case maps to the same status used by sibling routes like the
approve/execute/cancel/dry-run handlers, and let unexpected errors from
`NextResponse.json` or the Supabase/client setup surface as a 500 instead. Keep
the response shape consistent while distinguishing missing actions from genuine
infrastructure errors.
| let body: unknown; try { body = await request.json(); } catch { body = {}; } | ||
| const rejected = await assertNoClientUserIdOverride(request, body); if (rejected) return NextResponse.json({ ok: false, blockers: rejected.blockers }, { status: 400 }); | ||
| const session = await resolvePandoraServerSession({ request }); if (!session.ok) return NextResponse.json({ ok: false, blockers: session.blockers }, { status: 401 }); | ||
| try { const { id } = await context.params; const supabase = await createSupabaseServerClient(); const action = await executeApprovedOperatorAction(supabase as unknown as OperatorActionDbClient, { userId: session.session.userId, actionId: id }); return NextResponse.json({ ok: true, action, result: action.result }); } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return ok: false for failed executions
executeApprovedOperatorAction can return a failed action on internal errors, but this route still responds with HTTP 200 { ok: true, ... }. Callers will treat failures as success unless they inspect action.status; derive ok from the terminal status or map failed executions to a non-200 response.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/pandora/operator-actions/`[id]/execute/route.ts at line 10, The
execute route in the operator-actions handler always returns ok: true from the
NextResponse.json call even when executeApprovedOperatorAction produces a failed
action. Update the route logic around executeApprovedOperatorAction and the
returned action.result so ok is derived from the terminal action.status, or
return a non-200 response when the action ends in failed, while keeping the
success path unchanged.
| const terminal = ["completed", "failed", "cancelled"].includes(action.status); | ||
| return <div className="pd-action-controls"> | ||
| {action.status === "proposed" ? <button className="button-link" type="button" onClick={() => postAction(action.id, "dry-run")}>Prepare dry-run</button> : null} | ||
| {(action.status === "proposed" || action.status === "dry_ran") ? <button className="button-link button-link--primary" type="button" onClick={() => postAction(action.id, "approve")}>Approve</button> : null} | ||
| {action.status === "approved" ? <button className="button-link button-link--primary" type="button" onClick={() => postAction(action.id, "execute")}>Execute approved read-only action</button> : null} | ||
| {!terminal && action.status !== "executing" ? <button className="button-link" type="button" onClick={() => postAction(action.id, "cancel")}>Cancel</button> : null} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B2 -A6 'TERMINAL' lib/services/pandora-operator-action-service.tsRepository: besfeng23/Memory
Length of output: 2683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the frontend component and any status handling around it.
ast-grep outline components/pandora/OperatorActionControls.tsx --view expanded
printf '\n---\n'
rg -n -A3 -B3 'postAction|blocked|dry_ran|executing|cancel' components/pandora/OperatorActionControls.tsx components/pandora -g '!**/node_modules/**'
printf '\n---\n'
# Inspect service-side action status handling and any relevant transitions.
sed -n '1,180p' lib/services/pandora-operator-action-service.ts
printf '\n---\n'
rg -n -A3 -B3 'Cannot cancel terminal Pandora operator action|Invalid Pandora operator action transition|blocked' lib/services/pandora-operator-action-service.ts lib/services -g '!**/node_modules/**'Repository: besfeng23/Memory
Length of output: 50373
Exclude blocked from the Cancel affordance. cancelOperatorAction() rejects blocked via assertTransition(..., "cancelled"), but this UI still renders Cancel because terminal omits it. That leaves a dead button for blocked actions, and the fetch path doesn’t surface the failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/pandora/OperatorActionControls.tsx` around lines 10 - 15, The
Cancel button in OperatorActionControls is still shown for blocked actions, even
though cancelOperatorAction() cannot transition blocked to cancelled. Update the
terminal/status gating in OperatorActionControls so blocked is treated like a
non-cancelable state, and only render the Cancel affordance for statuses that
can actually be cancelled; keep the logic aligned with postAction and
cancelOperatorAction so the UI never exposes a dead action.
| | From | To | | ||
| | --- | --- | | ||
| | `proposed` | `dry_ran`, `approved`, `cancelled` | | ||
| | `dry_ran` | `approved`, `cancelled` | | ||
| | `approved` | `executing`, `cancelled` | | ||
| | `executing` | `completed`, `failed` | | ||
| | `completed`, `failed`, `cancelled` | terminal | | ||
|
|
||
| ## Why dry-run comes before live actions | ||
| Invalid transitions fail safely and do not create misleading completion states. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include blocked in the terminal-state list.
The service treats blocked as terminal, but this doc only marks completed/failed/cancelled as terminal. That mismatch will confuse readers about the lifecycle.
Suggested fix
-| `completed`, `failed`, `cancelled` | terminal |
+| `completed`, `failed`, `cancelled`, `blocked` | terminal |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | From | To | | |
| | --- | --- | | |
| | `proposed` | `dry_ran`, `approved`, `cancelled` | | |
| | `dry_ran` | `approved`, `cancelled` | | |
| | `approved` | `executing`, `cancelled` | | |
| | `executing` | `completed`, `failed` | | |
| | `completed`, `failed`, `cancelled` | terminal | | |
| ## Why dry-run comes before live actions | |
| Invalid transitions fail safely and do not create misleading completion states. | |
| | From | To | | |
| | --- | --- | | |
| | `proposed` | `dry_ran`, `approved`, `cancelled` | | |
| | `dry_ran` | `approved`, `cancelled` | | |
| | `approved` | `executing`, `cancelled` | | |
| | `executing` | `completed`, `failed` | | |
| | `completed`, `failed`, `cancelled`, `blocked` | terminal | | |
| Invalid transitions fail safely and do not create misleading completion states. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/pandora-operator-action-center.md` around lines 31 - 39, The
action-center transition table is missing the `blocked` lifecycle state in the
terminal-state row, which creates a mismatch with the service behavior. Update
the state transition documentation in the transition table so `blocked` is
listed alongside `completed`, `failed`, and `cancelled` as terminal, using the
existing action-state terminology in the table to keep the lifecycle description
accurate.
| const actionStatuses = ["proposed", "dry_ran", "approved", "executing", "completed", "blocked", "failed", "cancelled"] as const; | ||
| const countsByStatus = Object.fromEntries(actionStatuses.map((status) => [status, operatorActions.filter((action) => action.status === status).length])) as PandoraDashboardData["operatorActions"]["countsByStatus"]; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Hardcoded status list duplicated across files.
actionStatuses duplicates the same 8-status literal also hardcoded in components/pandora/OperatorActionCenterCard.tsx (emptyCounts) and components/pandora/mock-data.ts (countsByStatus fixture). If OperatorActionStatus gains/loses a value, all three call sites must be updated in lockstep or the UI silently drops/omits a status bucket. Consider exporting a single OPERATOR_ACTION_STATUSES constant from components/pandora/types.ts (or this service) and importing it everywhere.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/services/pandora-dashboard-service.ts` around lines 55 - 56, The status
bucket list is duplicated in pandora dashboard logic and related UI fixtures, so
update the counting code in PandoraDashboardService to use a shared source of
truth instead of a local literal. Create or reuse a single
OPERATOR_ACTION_STATUSES constant (for example in types.ts or the service
module) and import it in actionStatuses, OperatorActionCenterCard, and mock-data
so all counts/empty states stay aligned when OperatorActionStatus changes.
| export async function executeApprovedOperatorAction(client: OperatorActionDbClient, input: { userId: string; actionId: string }): Promise<OperatorActionRow> { | ||
| const action = await getOperatorAction(client, input); | ||
| assertTransition(action.status, "executing"); | ||
| assertAction(action.action_type); | ||
| assertMode(action.mode); | ||
| const executingAt = new Date().toISOString(); | ||
| const executing = await single(client.from("pandora_operator_actions").update({ status: "executing", updated_at: executingAt }).eq("user_id", input.userId).eq("id", input.actionId).select("*").single()) ?? { ...action, status: "executing" as const, updated_at: executingAt }; | ||
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "executing", message: "Approved read-only verification execution started.", metadata: { no_mutation_performed: true } }); | ||
| try { | ||
| const built = await buildDryRunResult(client, input.userId, executing); | ||
| const completedAt = new Date().toISOString(); | ||
| const result = envelope({ ...executing, status: "completed" }, built.result, built.warnings); | ||
| const completed = await single(client.from("pandora_operator_actions").update({ status: "completed", result, warnings: built.warnings, completed_at: completedAt, updated_at: completedAt }).eq("user_id", input.userId).eq("id", input.actionId).select("*").single()); | ||
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "completed", message: "Approved read-only execution completed. No core memory mutation was performed.", metadata: result }); | ||
| return completed ?? { ...executing, status: "completed", result, warnings: built.warnings, completed_at: completedAt, updated_at: completedAt }; | ||
| } catch (error) { | ||
| const failedAt = new Date().toISOString(); | ||
| const warnings = [error instanceof Error ? error.message : "Read-only execution failed"]; | ||
| const result = envelope({ ...executing, status: "failed" }, { error: warnings[0] }, warnings); | ||
| const failed = await single(client.from("pandora_operator_actions").update({ status: "failed", result, warnings, failed_at: failedAt, updated_at: failedAt }).eq("user_id", input.userId).eq("id", input.actionId).select("*").single()); | ||
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "failed", message: "Approved read-only execution failed; no completion was fabricated.", metadata: result }); | ||
| return failed ?? { ...executing, status: "failed", result, warnings, failed_at: failedAt, updated_at: failedAt }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep outline lib/services/pandora-operator-action-service.ts --view expandedRepository: besfeng23/Memory
Length of output: 5931
🏁 Script executed:
rg -n "function single|const single|async function single|export .*single|\.eq\\(\"status\"" lib -SRepository: besfeng23/Memory
Length of output: 16982
🏁 Script executed:
sed -n '80,150p' lib/services/pandora-operator-action-service.ts | cat -nRepository: besfeng23/Memory
Length of output: 5641
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
text = Path("lib/services/pandora-operator-action-service.ts").read_text()
for needle in [
'.eq("user_id", input.userId).eq("id", input.actionId)',
'.eq("status", "approved")',
'.eq("status", "executing")',
'.eq("status", "proposed")',
'.eq("status", "dry_ran")',
]:
print(needle, text.count(needle))
PYRepository: besfeng23/Memory
Length of output: 322
Guard each transition update on the expected current status.
assertTransition only checks the row that was read. The update(...) calls for approved → executing, executing → completed/failed, and cancelled still match only user_id/id, so two concurrent requests can both advance the same action and emit duplicate audit events. Add the expected status to each update and treat a null result as a lost race.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/services/pandora-operator-action-service.ts` around lines 99 - 122, Guard
the status transitions in executeApprovedOperatorAction by making each database
update conditional on the expected current status, not just userId and actionId.
Update the approved-to-executing and executing-to-completed/failed writes to
include the current status in the filter, and treat a null return from
single(...) as a lost race so the action is not advanced twice or double-logged.
1 participant
Motivation
user_id— all identities come from server-derived sessions.Description
proposed,dry_ran,approved,executing,completed,blocked,failed, andcancelled, and implementassertTransition/TERMINALprotection inlib/services/pandora-operator-action-service.ts.approveOperatorAction,executeApprovedOperatorAction,getOperatorAction, andlistOperatorActionEventsthat validate ownership, action type, and mode; write audited event rows for every transition; and always return read-only execution envelopes includingno_mutation_performed: trueand execution metadata.user_idoverrides:POST /api/pandora/operator-actions/[id]/approve,POST /api/pandora/operator-actions/[id]/execute, andGET /api/pandora/operator-actions/[id]/events, and harden existingdry-runandcancelroutes to produce events and respect terminal states.event_preview,event_count,approved_at,completed_at,failed_at, andcountsByStatus) without letting action-table read failures crash/pandora.OperatorActionControls(Prepare dry-run, Approve, Execute approved read-only action, Cancel),OperatorActionTimeline, and UI updates toOperatorActionEnvelope/OperatorActionList/OperatorActionCenterCardto show lifecycle, request_id, shortened idempotency, timestamps, warnings, andNo mutation performedvisibility.supabase/migrations/20260703010000_pandora_operator_action_runner_statuses.sqlto expand thestatuscheck constraint safely; do not change or drop any core memory tables or RLS policies.docs/pandora-operator-action-center.mdand adddocs/pandora-readonly-action-runner.mddescribing lifecycle, allowed read-only executions, safety boundaries, and smoke-test checklist.Testing
npm run typecheckwhich passed successfully with no blocking errors.npm run lintwhich completed; project had existing lint warnings unrelated to these changes but no new lint errors from the runner changes.npm run testand the test suite passed (96 files, 615 tests passed overall); unit tests added/updated for operator actions include service and UI tests covering approve/execute/event ownership, transition guards, idempotency, and no-mutation envelopes.npm run build(Next.js build) which succeeded with existing runtime/warnings unrelated to safety, and rannpm run env:policywhich passed Env Broker checks.Codex Task
Summary by CodeRabbit
New Features
Bug Fixes