Add Pandora shadow pack preflight v1 (deterministic diff, risk, review, preflight tables)#137
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds a "Shadow Pack Preflight" system for reviewing proposed shadow context pack changes before promotion. It introduces new database tables, a service module for computing diffs/risk and managing preflight records, five API routes, dashboard UI components, operator action integration, and documentation, all without mutating live memory data. ChangesShadow Pack Preflight
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Composer as OperatorActionComposer
participant ActionService as pandora-operator-action-service
participant PreflightService as pandora-shadow-pack-preflight-service
participant DB as Supabase
Operator->>Composer: propose prepare_shadow_pack_preflight
Composer->>ActionService: buildDryRunResult(payload.shadow_pack_id)
ActionService->>PreflightService: computeShadowPackPreflight(shadow, master)
PreflightService-->>ActionService: diff/risk summary
ActionService-->>Composer: dry-run result (no mutation)
Operator->>ActionService: approve execution
ActionService->>PreflightService: createOrRefreshShadowPackPreflight()
PreflightService->>DB: upsert preflight row + event
PreflightService-->>ActionService: preflight row
ActionService-->>Operator: execution result (no_promotion_performed=true)
sequenceDiagram
participant Reviewer
participant UI as ShadowPackPreflightList/ReviewForm/Controls
participant API as Preflight API routes
participant Service as pandora-shadow-pack-preflight-service
participant DB as Supabase
Reviewer->>UI: view preflight list
UI->>API: GET /shadow-pack-preflights
API->>Service: listShadowPackPreflights(userId)
Service->>DB: query preflights
DB-->>Service: rows
Service-->>API: preflights
API-->>UI: preflights JSON
Reviewer->>UI: submit review decision
UI->>API: POST /shadow-pack-preflights/{id}/review
API->>Service: updateShadowPackPreflightReview()
Service->>DB: update row + emit event
API-->>UI: updated preflight
Reviewer->>UI: click Archive
UI->>API: POST /shadow-pack-preflights/{id}/archive
API->>Service: archiveShadowPackPreflight()
Service->>DB: set status archived + emit event
API-->>UI: archived preflight
🚥 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: a4ec11daf5
ℹ️ 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".
| export type OperatorActionDbClient = { from: (table: string) => any }; | ||
|
|
||
| const ACTIONS = new Set<OperatorActionType>(["verify_namespace_invariants", "verify_pack_supersession", "check_retrieval_eval_status", "refresh_dashboard_snapshot", "prepare_distill_smoke_plan", "prepare_shadow_context_pack"]); | ||
| const ACTIONS = new Set<OperatorActionType>(["verify_namespace_invariants", "verify_pack_supersession", "check_retrieval_eval_status", "refresh_dashboard_snapshot", "prepare_distill_smoke_plan", "prepare_shadow_context_pack", "prepare_shadow_pack_preflight"]); |
There was a problem hiding this comment.
The service/UI now accepts prepare_shadow_pack_preflight, but the existing pandora_operator_actions.action_type check constraint is still defined by 20260703020000_pandora_shadow_context_pack_lab.sql without that value, and this commit's migration never drops/readds it. In any environment with that migration applied, proposing this action through POST /api/pandora/operator-actions reaches proposeOperatorAction and then the insert is rejected by the database constraint, so the advertised operator-action preflight workflow cannot be created.
Useful? React with 👍 / 👎.
| alter table public.pandora_shadow_pack_preflights enable row level security; | ||
| create policy "shadow pack preflights select own" on public.pandora_shadow_pack_preflights for select to authenticated using (user_id = auth.uid()); | ||
| create policy "shadow pack preflights insert own" on public.pandora_shadow_pack_preflights for insert to authenticated with check (user_id = auth.uid()); | ||
| create policy "shadow pack preflights update own" on public.pandora_shadow_pack_preflights for update to authenticated using (user_id = auth.uid()) with check (user_id = auth.uid()); |
There was a problem hiding this comment.
This unrestricted update policy lets the authenticated owner update every preflight column directly through Supabase, unlike the operator-action migration that limits update columns. For any browser session with the public Supabase client, this bypasses updateShadowPackPreflightReview/archive event creation and can forge status, reviewer_decision, diff_summary, risk_summary, or approved_for_promotion_at, making the reviewer/audit-backed preflight record unreliable for handoff or future promotion checks.
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: 11
🧹 Nitpick comments (3)
supabase/migrations/20260704010000_pandora_shadow_pack_preflight.sql (1)
23-23: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDrop the redundant lookup index.
unique (user_id, shadow_pack_id)already creates an index with the same key order, soidx_pandora_shadow_pack_preflights_user_shadowadds write/storage overhead without improving lookups.♻️ Suggested fix
-create index if not exists idx_pandora_shadow_pack_preflights_user_shadow on public.pandora_shadow_pack_preflights(user_id, shadow_pack_id);🤖 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 `@supabase/migrations/20260704010000_pandora_shadow_pack_preflight.sql` at line 23, Remove the redundant lookup index from the migration: `idx_pandora_shadow_pack_preflights_user_shadow` duplicates the automatic index created by the `unique (user_id, shadow_pack_id)` constraint. Update the `pandora_shadow_pack_preflights` migration to keep only the unique constraint and drop the extra `create index` statement so writes/storage are not wasted.components/pandora/ShadowPackPreflightReviewForm.tsx (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueButtons default to
type="submit".None of the three buttons declare
type="button", unlikeShadowPackPreflightControls's archive button. Not currently an issue since there's no enclosing<form>, but a defensive/consistency nit worth aligning.🤖 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/ShadowPackPreflightReviewForm.tsx` at line 5, The three action buttons in ShadowPackPreflightReviewForm currently rely on the default submit behavior, so make them explicit `type="button"` for consistency with ShadowPackPreflightControls and to avoid accidental form submission if the component is ever wrapped in a form. Update the button elements in ShadowPackPreflightReviewForm to set the button type directly while keeping the existing save(preflight.id, notes, ...) handlers unchanged.tests/unit/pandora-shadow-pack-preflight-service.test.ts (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding regression coverage for review lifecycle edge cases.
The suite covers risk tiers and cross-user blocking well, but not: (1) refreshing an already-approved preflight (state consistency), or (2) reviewing an archived preflight. These correspond to the two logic gaps flagged in the service and would lock in the intended behavior once fixed.
🤖 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-shadow-pack-preflight-service.test.ts` at line 17, Add regression tests in the shadow pack preflight service suite to cover the missing review lifecycle edge cases: refreshing an already approved preflight and attempting to review an archived preflight. Use the existing helpers like createOrRefreshShadowPackPreflight, updateShadowPackPreflightReview, and archiveShadowPackPreflight to verify state remains consistent after refresh and that archived items cannot be reviewed. Keep the new coverage alongside the current review/archival flow tests so the intended service behavior is locked in.
🤖 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/shadow-pack-preflights/route.ts`:
- Line 6: The GET handler in shadow-pack-preflights should convert invalid
`namespace` or `status` query values into a 400 response instead of letting
`listShadowPackPreflights` throw and bubble up as a 500. Wrap the call path
after `resolvePandoraServerSession` in a `try/catch`, catch validation errors
from `assertNamespace`/`assertStatus`, and return `NextResponse.json({ ok:false,
blockers: ... }, { status: 400 })` for bad params while preserving the existing
401/other success paths in `GET`.
In `@components/pandora/OperatorActionComposer.tsx`:
- Around line 7-8: The OperatorActionComposer prepare flow always sends a fixed
payload, so prepare_shadow_pack_preflight cannot supply the required
shadow_pack_id. Update prepare() and the surrounding JSX to conditionally render
an input for shadow_pack_id when actionType is prepare_shadow_pack_preflight,
and include that value in the request payload only for that action. Also make
the fetch handling in OperatorActionComposer check for non-OK responses before
window.location.reload() so failures surface instead of reloading silently.
In `@components/pandora/ShadowPackPreflightControls.tsx`:
- Line 3: The archive flow in archive should not reload unconditionally after
fetch; it currently hides failures because it never checks the response status.
Update archive(id: string) in ShadowPackPreflightControls to capture the fetch
result, verify res.ok before calling window.location.reload(), and handle non-OK
responses the same way the review form does so a failed archive is surfaced
instead of looking successful.
In `@components/pandora/ShadowPackPreflightReviewForm.tsx`:
- Line 4: The save function in ShadowPackPreflightReviewForm submits the review
and always reloads the page, so failures are hidden. Update save to check the
fetch result from the /api/pandora/shadow-pack-preflights/${id}/review request,
handle non-2xx responses and network errors explicitly, and only call
window.location.reload() after a confirmed success. Use the save function and
its fetch call as the main place to add error handling and surface a clear
failure state to the reviewer.
In `@docs/pandora-operator-action-center.md`:
- Around line 53-55: The Shadow pack preflight action description is inaccurate
because `prepare_shadow_pack_preflight` dry-runs also compute risk summaries,
not just the diff. Update the wording in the docs entry for
`prepare_shadow_pack_preflight` to reflect that the dry-run path computes both
diff and risk summary, while approved execution still writes only preflight
metadata and does not promote context packs.
In `@lib/services/pandora-dashboard-service.ts`:
- Around line 49-51: The dashboard preflight failure is being swallowed, so the
warning never reaches the UI and the health state can stay green. Update
`listShadowPackPreflights` in `pandora-dashboard-service.ts` to surface Supabase
read errors instead of always returning an empty array, or wire
`preflightWarnings` into the `live` calculation so failed preflight loads affect
the dashboard status. Make the fix in the `listShadowPackPreflights`/`live` flow
and ensure the “preflight tables unavailable” warning is actually propagated.
In `@lib/services/pandora-operator-action-service.ts`:
- Line 119: The executing event in pandora-operator-action-service.ts mislabels
prepare_shadow_pack_preflight as read-only and non-mutating. Update the
createActionEvent call in the action execution flow to explicitly handle
prepare_shadow_pack_preflight alongside prepare_shadow_context_pack, using the
correct staging/preflight message and setting metadata.no_mutation_performed to
false for this branch while keeping no_core_memory_mutation_performed true.
Ensure the branching logic around action.action_type reflects all three action
types so the event metadata matches the actual behavior of
createOrRefreshShadowPackPreflight.
In `@lib/services/pandora-shadow-pack-preflight-service.ts`:
- Line 42: The updateShadowPackPreflightReview flow currently allows archived
preflights to be reviewed and moved back into active statuses. Add a
terminal-state guard in updateShadowPackPreflightReview, using
getShadowPackPreflight (or a follow-up check on the returned row) to reject rows
whose status is archived before building the patch/update. Make sure the guard
prevents any update/ event creation for archived records and returns a clear
error for that case.
- Line 41: In createOrRefreshShadowPackPreflight, the update path in the base
payload leaves reviewer state from the previous review intact while recomputing
status, which can create contradictory rows. Reset reviewer_decision,
reviewer_notes, reviewed_at, approved_for_promotion_at, and blocked_at when
refreshing an existing preflight, or add a guard in
getShadowPackPreflightByShadowPack/createOrRefreshShadowPackPreflight to prevent
refresh after a decision is recorded. Keep the behavior aligned with the
existing insert path and the event creation that follows.
- Line 41: The createOrRefreshShadowPackPreflight flow does a non-atomic
get-then-insert/update split that can race on the unique (user_id,
shadow_pack_id) constraint. Refactor the createOrRefreshShadowPackPreflight
logic to use a single atomic upsert on pandora_shadow_pack_preflights,
preserving the current base fields and insert-only defaults while still
returning the row for the later createShadowPackPreflightEvent call.
In `@supabase/migrations/20260704010000_pandora_shadow_pack_preflight.sql`:
- Around line 42-44: The `pandora_shadow_pack_preflight_events` RLS setup
currently includes an `UPDATE` policy that allows the owning user to modify
existing audit rows, which conflicts with the intended append-only behavior.
Remove the update permission from the policy block tied to
`pandora_shadow_pack_preflight_events` and keep only the `SELECT` and `INSERT`
policies so that `user_id = auth.uid()` can still read and append new events but
cannot rewrite prior ones.
---
Nitpick comments:
In `@components/pandora/ShadowPackPreflightReviewForm.tsx`:
- Line 5: The three action buttons in ShadowPackPreflightReviewForm currently
rely on the default submit behavior, so make them explicit `type="button"` for
consistency with ShadowPackPreflightControls and to avoid accidental form
submission if the component is ever wrapped in a form. Update the button
elements in ShadowPackPreflightReviewForm to set the button type directly while
keeping the existing save(preflight.id, notes, ...) handlers unchanged.
In `@supabase/migrations/20260704010000_pandora_shadow_pack_preflight.sql`:
- Line 23: Remove the redundant lookup index from the migration:
`idx_pandora_shadow_pack_preflights_user_shadow` duplicates the automatic index
created by the `unique (user_id, shadow_pack_id)` constraint. Update the
`pandora_shadow_pack_preflights` migration to keep only the unique constraint
and drop the extra `create index` statement so writes/storage are not wasted.
In `@tests/unit/pandora-shadow-pack-preflight-service.test.ts`:
- Line 17: Add regression tests in the shadow pack preflight service suite to
cover the missing review lifecycle edge cases: refreshing an already approved
preflight and attempting to review an archived preflight. Use the existing
helpers like createOrRefreshShadowPackPreflight,
updateShadowPackPreflightReview, and archiveShadowPackPreflight to verify state
remains consistent after refresh and that archived items cannot be reviewed.
Keep the new coverage alongside the current review/archival flow tests so the
intended service behavior is locked in.
🪄 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: 589dcc16-0e6a-451b-b6c3-7abc18a7cf46
📒 Files selected for processing (25)
app/api/pandora/shadow-context-packs/[id]/preflight/route.tsapp/api/pandora/shadow-pack-preflights/[id]/archive/route.tsapp/api/pandora/shadow-pack-preflights/[id]/review/route.tsapp/api/pandora/shadow-pack-preflights/[id]/route.tsapp/api/pandora/shadow-pack-preflights/route.tscomponents/pandora/OperatorActionComposer.tsxcomponents/pandora/PandoraDashboard.tsxcomponents/pandora/ShadowContextPackControls.tsxcomponents/pandora/ShadowPackDiffViewer.tsxcomponents/pandora/ShadowPackPreflightCard.tsxcomponents/pandora/ShadowPackPreflightControls.tsxcomponents/pandora/ShadowPackPreflightList.tsxcomponents/pandora/ShadowPackPreflightReviewForm.tsxcomponents/pandora/ShadowPackRiskSummary.tsxcomponents/pandora/mock-data.tscomponents/pandora/types.tsdocs/pandora-operator-action-center.mddocs/pandora-readonly-action-runner.mddocs/pandora-shadow-context-pack-lab.mddocs/pandora-shadow-pack-preflight.mdlib/services/pandora-dashboard-service.tslib/services/pandora-operator-action-service.tslib/services/pandora-shadow-pack-preflight-service.tssupabase/migrations/20260704010000_pandora_shadow_pack_preflight.sqltests/unit/pandora-shadow-pack-preflight-service.test.ts
| import { createSupabaseServerClient } from "@/lib/supabase/server"; | ||
| import { listShadowPackPreflights, type ShadowPackPreflightDbClient } from "@/lib/services/pandora-shadow-pack-preflight-service"; | ||
| export const dynamic = "force-dynamic"; | ||
| export async function GET(request: NextRequest) { const rejected = await assertNoClientUserIdOverride(request); 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 }); const url = new URL(request.url); const supabase = await createSupabaseServerClient(); const preflights = await listShadowPackPreflights(supabase as unknown as ShadowPackPreflightDbClient, { userId: session.session.userId, namespace: url.searchParams.get("namespace") ?? undefined, status: url.searchParams.get("status") ?? undefined, limit: 25 }); return NextResponse.json({ ok:true, preflights }); } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Invalid namespace/status query params return 500 instead of 400.
listShadowPackPreflights runs assertNamespace/assertStatus, which throw on unrecognized values sourced directly from url.searchParams. With no try/catch here, a request like ?namespace=foo surfaces as an unhandled 500 rather than a client-error 400.
🛡️ Suggested handling
-const preflights = await listShadowPackPreflights(supabase as unknown as ShadowPackPreflightDbClient, { userId: session.session.userId, namespace: url.searchParams.get("namespace") ?? undefined, status: url.searchParams.get("status") ?? undefined, limit: 25 }); return NextResponse.json({ ok:true, preflights });
+try { const preflights = await listShadowPackPreflights(supabase as unknown as ShadowPackPreflightDbClient, { userId: session.session.userId, namespace: url.searchParams.get("namespace") ?? undefined, status: url.searchParams.get("status") ?? undefined, limit: 25 }); return NextResponse.json({ ok:true, preflights }); } catch(e) { return NextResponse.json({ ok:false, error: e instanceof Error ? e.message : "Invalid query" }, { status: 400 }); }📝 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.
Suggested change
| export async function GET(request: NextRequest) { const rejected = await assertNoClientUserIdOverride(request); 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 }); const url = new URL(request.url); const supabase = await createSupabaseServerClient(); const preflights = await listShadowPackPreflights(supabase as unknown as ShadowPackPreflightDbClient, { userId: session.session.userId, namespace: url.searchParams.get("namespace") ?? undefined, status: url.searchParams.get("status") ?? undefined, limit: 25 }); return NextResponse.json({ ok:true, preflights }); } | |
| export async function GET(request: NextRequest) { | |
| const rejected = await assertNoClientUserIdOverride(request); | |
| 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 }); | |
| const url = new URL(request.url); | |
| const supabase = await createSupabaseServerClient(); | |
| try { | |
| const preflights = await listShadowPackPreflights(supabase as unknown as ShadowPackPreflightDbClient, { | |
| userId: session.session.userId, | |
| namespace: url.searchParams.get("namespace") ?? undefined, | |
| status: url.searchParams.get("status") ?? undefined, | |
| limit: 25, | |
| }); | |
| return NextResponse.json({ ok: true, preflights }); | |
| } catch (e) { | |
| return NextResponse.json( | |
| { ok: false, error: e instanceof Error ? e.message : "Invalid query" }, | |
| { status: 400 }, | |
| ); | |
| } | |
| } |
🤖 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/shadow-pack-preflights/route.ts` at line 6, The GET handler
in shadow-pack-preflights should convert invalid `namespace` or `status` query
values into a 400 response instead of letting `listShadowPackPreflights` throw
and bubble up as a 500. Wrap the call path after `resolvePandoraServerSession`
in a `try/catch`, catch validation errors from `assertNamespace`/`assertStatus`,
and return `NextResponse.json({ ok:false, blockers: ... }, { status: 400 })` for
bad params while preserving the existing 401/other success paths in `GET`.
Comment on lines
7
to
+8
| async function prepare() { await fetch("/api/pandora/operator-actions", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ action_type: actionType, namespace, mode: "dry_run", payload: { source: "operator_action_center" } }) }); window.location.reload(); } | ||
| return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}><option value="verify_namespace_invariants">verify_namespace_invariants</option><option value="verify_pack_supersession">verify_pack_supersession</option><option value="check_retrieval_eval_status">check_retrieval_eval_status</option><option value="refresh_dashboard_snapshot">refresh_dashboard_snapshot</option><option value="prepare_distill_smoke_plan">prepare_distill_smoke_plan</option><option value="prepare_shadow_context_pack">prepare_shadow_context_pack</option></select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label></div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">For prepare_shadow_context_pack: Creates shadow candidate only after approval/execution. No promotion. Only dry-run or queued-only proposals are available. No core memory mutation is available from this card.</p></div>; | ||
| return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}><option value="verify_namespace_invariants">verify_namespace_invariants</option><option value="verify_pack_supersession">verify_pack_supersession</option><option value="check_retrieval_eval_status">check_retrieval_eval_status</option><option value="refresh_dashboard_snapshot">refresh_dashboard_snapshot</option><option value="prepare_distill_smoke_plan">prepare_distill_smoke_plan</option><option value="prepare_shadow_context_pack">prepare_shadow_context_pack</option><option value="prepare_shadow_pack_preflight">prepare_shadow_pack_preflight</option></select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label></div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">For prepare_shadow_context_pack: creates shadow candidate only after approval/execution. For prepare_shadow_pack_preflight, payload.shadow_pack_id is required by the API/service; use a direct action payload only when the target shadow pack is known. No promotion. Only dry-run or queued-only proposals are available. No core memory mutation is available from this card.</p></div>; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
New prepare_shadow_pack_preflight option is unusable — no field to supply the required shadow_pack_id.
The service throws if payload.shadow_pack_id is missing for this action type: if (action.action_type === "prepare_shadow_pack_preflight") { const shadowPackId = typeof action.payload?.shadow_pack_id === "string" ? action.payload.shadow_pack_id : ""; if (!shadowPackId) throw new Error("prepare_shadow_pack_preflight requires payload.shadow_pack_id");
But prepare() always submits payload: { source: "operator_action_center" } (Line 7) with no way to enter a shadow pack id, for every action type including this new one. Selecting prepare_shadow_pack_preflight and clicking "Prepare dry-run" will always fail server-side, and since prepare() doesn't check the fetch response before reloading, the user gets no feedback that it failed.
Add a conditional input for shadow_pack_id when this action type is selected, and include it in the payload.
🛠️ Proposed fix
export function OperatorActionComposer() {
const [actionType, setActionType] = useState("verify_namespace_invariants");
const [namespace, setNamespace] = useState("real_life");
- async function prepare() { await fetch("/api/pandora/operator-actions", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ action_type: actionType, namespace, mode: "dry_run", payload: { source: "operator_action_center" } }) }); window.location.reload(); }
- return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}>...</select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label></div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">...</p></div>;
+ const [shadowPackId, setShadowPackId] = useState("");
+ async function prepare() {
+ const payload = actionType === "prepare_shadow_pack_preflight" ? { shadow_pack_id: shadowPackId } : { source: "operator_action_center" };
+ await fetch("/api/pandora/operator-actions", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ action_type: actionType, namespace, mode: "dry_run", payload }) });
+ window.location.reload();
+ }
+ return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}>...</select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label>{actionType === "prepare_shadow_pack_preflight" && <label className="pd-mini"><span>Shadow pack id</span><input value={shadowPackId} onChange={(e) => setShadowPackId(e.target.value)} /></label>}</div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">...</p></div>;📝 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.
Suggested change
| async function prepare() { await fetch("/api/pandora/operator-actions", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ action_type: actionType, namespace, mode: "dry_run", payload: { source: "operator_action_center" } }) }); window.location.reload(); } | |
| return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}><option value="verify_namespace_invariants">verify_namespace_invariants</option><option value="verify_pack_supersession">verify_pack_supersession</option><option value="check_retrieval_eval_status">check_retrieval_eval_status</option><option value="refresh_dashboard_snapshot">refresh_dashboard_snapshot</option><option value="prepare_distill_smoke_plan">prepare_distill_smoke_plan</option><option value="prepare_shadow_context_pack">prepare_shadow_context_pack</option></select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label></div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">For prepare_shadow_context_pack: Creates shadow candidate only after approval/execution. No promotion. Only dry-run or queued-only proposals are available. No core memory mutation is available from this card.</p></div>; | |
| return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}><option value="verify_namespace_invariants">verify_namespace_invariants</option><option value="verify_pack_supersession">verify_pack_supersession</option><option value="check_retrieval_eval_status">check_retrieval_eval_status</option><option value="refresh_dashboard_snapshot">refresh_dashboard_snapshot</option><option value="prepare_distill_smoke_plan">prepare_distill_smoke_plan</option><option value="prepare_shadow_context_pack">prepare_shadow_context_pack</option><option value="prepare_shadow_pack_preflight">prepare_shadow_pack_preflight</option></select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label></div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">For prepare_shadow_context_pack: creates shadow candidate only after approval/execution. For prepare_shadow_pack_preflight, payload.shadow_pack_id is required by the API/service; use a direct action payload only when the target shadow pack is known. No promotion. Only dry-run or queued-only proposals are available. No core memory mutation is available from this card.</p></div>; | |
| const [shadowPackId, setShadowPackId] = useState(""); | |
| async function prepare() { | |
| const payload = actionType === "prepare_shadow_pack_preflight" ? { shadow_pack_id: shadowPackId } : { source: "operator_action_center" }; | |
| await fetch("/api/pandora/operator-actions", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ action_type: actionType, namespace, mode: "dry_run", payload }) }); | |
| window.location.reload(); | |
| } | |
| return <div className="pd-composer"><div className="pd-mini-grid"><label className="pd-mini"><span>Action type</span><select value={actionType} onChange={(e) => setActionType(e.target.value)}><option value="verify_namespace_invariants">verify_namespace_invariants</option><option value="verify_pack_supersession">verify_pack_supersession</option><option value="check_retrieval_eval_status">check_retrieval_eval_status</option><option value="refresh_dashboard_snapshot">refresh_dashboard_snapshot</option><option value="prepare_distill_smoke_plan">prepare_distill_smoke_plan</option><option value="prepare_shadow_context_pack">prepare_shadow_context_pack</option><option value="prepare_shadow_pack_preflight">prepare_shadow_pack_preflight</option></select></label><label className="pd-mini"><span>Namespace</span><select value={namespace} onChange={(e) => setNamespace(e.target.value)}><option value="real_life">real_life</option><option value="au">au</option></select></label>{actionType === "prepare_shadow_pack_preflight" && <label className="pd-mini"><span>Shadow pack id</span><input value={shadowPackId} onChange={(e) => setShadowPackId(e.target.value)} /></label>}</div><button className="button-link button-link--primary" type="button" onClick={prepare}>Prepare dry-run</button><p className="pd-muted">For prepare_shadow_context_pack: creates shadow candidate only after approval/execution. For prepare_shadow_pack_preflight, payload.shadow_pack_id is required by the API/service; use a direct action payload only when the target shadow pack is known. No promotion. Only dry-run or queued-only proposals are available. No core memory mutation is available from this card.</p></div>; |
🤖 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/OperatorActionComposer.tsx` around lines 7 - 8, The
OperatorActionComposer prepare flow always sends a fixed payload, so
prepare_shadow_pack_preflight cannot supply the required shadow_pack_id. Update
prepare() and the surrounding JSX to conditionally render an input for
shadow_pack_id when actionType is prepare_shadow_pack_preflight, and include
that value in the request payload only for that action. Also make the fetch
handling in OperatorActionComposer check for non-OK responses before
window.location.reload() so failures surface instead of reloading silently.
| @@ -0,0 +1,4 @@ | |||
| "use client"; | |||
| import type { ShadowPackPreflightSummary } from "./types"; | |||
| async function archive(id: string) { await fetch(`/api/pandora/shadow-pack-preflights/${id}/archive`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) }); window.location.reload(); } | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Same silent-failure pattern as the review form.
archive reloads unconditionally without checking res.ok, so a failed archive (e.g., stale session, server error) is indistinguishable from success.
🛠️ Proposed fix
-async function archive(id: string) { await fetch(`/api/pandora/shadow-pack-preflights/${id}/archive`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) }); window.location.reload(); }
+async function archive(id: string) {
+ const res = await fetch(`/api/pandora/shadow-pack-preflights/${id}/archive`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) });
+ if (!res.ok) {
+ const body = await res.json().catch(() => ({}));
+ alert(body.error ?? "Failed to archive preflight.");
+ return;
+ }
+ window.location.reload();
+}📝 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.
Suggested change
| async function archive(id: string) { await fetch(`/api/pandora/shadow-pack-preflights/${id}/archive`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) }); window.location.reload(); } | |
| async function archive(id: string) { | |
| const res = await fetch(`/api/pandora/shadow-pack-preflights/${id}/archive`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({}) }); | |
| if (!res.ok) { | |
| const body = await res.json().catch(() => ({})); | |
| alert(body.error ?? "Failed to archive preflight."); | |
| 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/ShadowPackPreflightControls.tsx` at line 3, The archive
flow in archive should not reload unconditionally after fetch; it currently
hides failures because it never checks the response status. Update archive(id:
string) in ShadowPackPreflightControls to capture the fetch result, verify
res.ok before calling window.location.reload(), and handle non-OK responses the
same way the review form does so a failed archive is surfaced instead of looking
successful.
| "use client"; | ||
| import { useState } from "react"; | ||
| import type { ShadowPackPreflightSummary } from "./types"; | ||
| async function save(id: string, reviewer_notes: string, reviewer_decision: string) { await fetch(`/api/pandora/shadow-pack-preflights/${id}/review`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ reviewer_notes, reviewer_decision }) }); window.location.reload(); } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
No error handling around the review submission.
save reloads the page unconditionally after fetch, even on non-2xx responses or network failures, so a failed reviewer decision (e.g. auth expired, validation error) looks identical to success — the operator has no way to know the decision wasn't recorded.
🛠️ Proposed fix
-async function save(id: string, reviewer_notes: string, reviewer_decision: string) { await fetch(`/api/pandora/shadow-pack-preflights/${id}/review`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ reviewer_notes, reviewer_decision }) }); window.location.reload(); }
+async function save(id: string, reviewer_notes: string, reviewer_decision: string) {
+ const res = await fetch(`/api/pandora/shadow-pack-preflights/${id}/review`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ reviewer_notes, reviewer_decision }) });
+ if (!res.ok) {
+ const body = await res.json().catch(() => ({}));
+ alert(body.error ?? "Failed to save reviewer decision.");
+ return;
+ }
+ window.location.reload();
+}📝 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.
Suggested change
| async function save(id: string, reviewer_notes: string, reviewer_decision: string) { await fetch(`/api/pandora/shadow-pack-preflights/${id}/review`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ reviewer_notes, reviewer_decision }) }); window.location.reload(); } | |
| async function save(id: string, reviewer_notes: string, reviewer_decision: string) { | |
| const res = await fetch(`/api/pandora/shadow-pack-preflights/${id}/review`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ reviewer_notes, reviewer_decision }) }); | |
| if (!res.ok) { | |
| const body = await res.json().catch(() => ({})); | |
| alert(body.error ?? "Failed to save reviewer decision."); | |
| 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/ShadowPackPreflightReviewForm.tsx` at line 4, The save
function in ShadowPackPreflightReviewForm submits the review and always reloads
the page, so failures are hidden. Update save to check the fetch result from the
/api/pandora/shadow-pack-preflights/${id}/review request, handle non-2xx
responses and network errors explicitly, and only call window.location.reload()
after a confirmed success. Use the save function and its fetch call as the main
place to add error handling and surface a clear failure state to the reviewer.
Comment on lines
+53
to
+55
| ## Shadow pack preflight action | ||
|
|
||
| Operators may propose `prepare_shadow_pack_preflight` when they provide a known `payload.shadow_pack_id`. Dry-run computes the diff only; approved execution writes preflight metadata only and does not promote context packs. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the risk summary in the dry-run path.
prepare_shadow_pack_preflight dry-runs compute both diff and risk summaries, so “diff only” is inaccurate and conflicts with the runner/service behavior.
Suggested wording
-Operators may propose `prepare_shadow_pack_preflight` when they provide a known `payload.shadow_pack_id`. Dry-run computes the diff only; approved execution writes preflight metadata only and does not promote context packs.
+Operators may propose `prepare_shadow_pack_preflight` when they provide a known `payload.shadow_pack_id`. Dry-run computes deterministic diff and risk summaries; approved execution writes preflight metadata only and does not promote context packs.📝 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.
Suggested change
| ## Shadow pack preflight action | |
| Operators may propose `prepare_shadow_pack_preflight` when they provide a known `payload.shadow_pack_id`. Dry-run computes the diff only; approved execution writes preflight metadata only and does not promote context packs. | |
| ## Shadow pack preflight action | |
| Operators may propose `prepare_shadow_pack_preflight` when they provide a known `payload.shadow_pack_id`. Dry-run computes deterministic diff and risk summaries; approved execution writes preflight metadata only and does not promote context packs. |
🤖 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 53 - 55, The Shadow pack
preflight action description is inaccurate because
`prepare_shadow_pack_preflight` dry-runs also compute risk summaries, not just
the diff. Update the wording in the docs entry for
`prepare_shadow_pack_preflight` to reflect that the dry-run path computes both
diff and risk summary, while approved execution still writes only preflight
metadata and does not promote context packs.
Comment on lines
+49
to
+51
| const preflightWarnings: string[] = []; | ||
| let preflights = [] as Row[]; | ||
| try { preflights = await listShadowPackPreflights(client, { userId: input.userId, limit: 8 }) as Row[]; } catch { preflightWarnings.push("shadow pack preflight tables unavailable; preflight UI is showing an empty state."); } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect listShadowPackPreflights implementation and callers/tests for error handling contract.
fd pandora-shadow-pack-preflight-service.ts
rg -n -A5 -B5 'function listShadowPackPreflights' lib/services/pandora-shadow-pack-preflight-service.ts
rg -n 'listShadowPackPreflights' tests/unit/pandora-shadow-pack-preflight-service.test.tsRepository: besfeng23/Memory
Length of output: 5707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the dashboard service around the reported lines and any later warning handling.
wc -l lib/services/pandora-dashboard-service.ts
sed -n '1,180p' lib/services/pandora-dashboard-service.ts
# Check for any other preflight warning propagation in the repo.
rg -n 'preflightWarnings|warnings.length === 0|shadow pack preflight tables unavailable|listShadowContextPacks' lib testsRepository: besfeng23/Memory
Length of output: 20254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l lib/services/pandora-dashboard-service.ts
sed -n '1,180p' lib/services/pandora-dashboard-service.ts
rg -n 'preflightWarnings|warnings.length === 0|shadow pack preflight tables unavailable|listShadowContextPacks' lib testsRepository: besfeng23/Memory
Length of output: 20254
Propagate preflight load failures into the dashboard warnings
listShadowPackPreflights(...) swallows Supabase errors by returning [], so the catch here never emits the “preflight tables unavailable” warning. live also ignores preflightWarnings, so the dashboard can still report healthy when preflight reads failed. Let the helper surface an error, or include preflightWarnings in the live calculation.
🤖 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 49 - 51, The
dashboard preflight failure is being swallowed, so the warning never reaches the
UI and the health state can stay green. Update `listShadowPackPreflights` in
`pandora-dashboard-service.ts` to surface Supabase read errors instead of always
returning an empty array, or wire `preflightWarnings` into the `live`
calculation so failed preflight loads affect the dashboard status. Make the fix
in the `listShadowPackPreflights`/`live` flow and ensure the “preflight tables
unavailable” warning is actually propagated.
| @@ -112,6 +119,13 @@ export async function executeApprovedOperatorAction(client: OperatorActionDbClie | |||
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "executing", message: action.action_type === "prepare_shadow_context_pack" ? "Approved shadow staging execution started." : "Approved read-only verification execution started.", metadata: { no_mutation_performed: action.action_type !== "prepare_shadow_context_pack", no_core_memory_mutation_performed: true } }); | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
no_mutation_performed mislabels the preflight execution as read-only.
This condition only special-cases prepare_shadow_context_pack. The new prepare_shadow_pack_preflight branch also performs a write (createOrRefreshShadowPackPreflight inserts/updates pandora_shadow_pack_preflights and inserts a preflight event), so the "executing" event will falsely report no_mutation_performed: true and the generic "read-only verification" message for this action, even though a staging write is about to happen.
🛠️ Proposed fix
- await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "executing", message: action.action_type === "prepare_shadow_context_pack" ? "Approved shadow staging execution started." : "Approved read-only verification execution started.", metadata: { no_mutation_performed: action.action_type !== "prepare_shadow_context_pack", no_core_memory_mutation_performed: true } });
+ const performsStagingWrite = action.action_type === "prepare_shadow_context_pack" || action.action_type === "prepare_shadow_pack_preflight";
+ await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "executing", message: performsStagingWrite ? "Approved staging execution started." : "Approved read-only verification execution started.", metadata: { no_mutation_performed: !performsStagingWrite, no_core_memory_mutation_performed: true } });📝 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.
Suggested change
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "executing", message: action.action_type === "prepare_shadow_context_pack" ? "Approved shadow staging execution started." : "Approved read-only verification execution started.", metadata: { no_mutation_performed: action.action_type !== "prepare_shadow_context_pack", no_core_memory_mutation_performed: true } }); | |
| const performsStagingWrite = action.action_type === "prepare_shadow_context_pack" || action.action_type === "prepare_shadow_pack_preflight"; | |
| await createActionEvent(client, { userId: input.userId, actionId: input.actionId, eventType: "executing", message: performsStagingWrite ? "Approved staging execution started." : "Approved read-only verification execution started.", metadata: { no_mutation_performed: !performsStagingWrite, no_core_memory_mutation_performed: true } }); |
🤖 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` at line 119, The executing
event in pandora-operator-action-service.ts mislabels
prepare_shadow_pack_preflight as read-only and non-mutating. Update the
createActionEvent call in the action execution flow to explicitly handle
prepare_shadow_pack_preflight alongside prepare_shadow_context_pack, using the
correct staging/preflight message and setting metadata.no_mutation_performed to
false for this branch while keeping no_core_memory_mutation_performed true.
Ensure the branching logic around action.action_type reflects all three action
types so the event metadata matches the actual behavior of
createOrRefreshShadowPackPreflight.
| export async function getShadowPackPreflight(client: ShadowPackPreflightDbClient, input:{userId:string; preflightId:string}): Promise<ShadowPackPreflightRow>{ const row=await single(client.from("pandora_shadow_pack_preflights").select("*").eq("user_id",input.userId).eq("id",input.preflightId).limit(1)); if(!row) throw new Error("Shadow pack preflight not found for current user"); return row; } | ||
| export async function getShadowPackPreflightByShadowPack(client: ShadowPackPreflightDbClient, input:{userId:string; shadowPackId:string}){ return single(client.from("pandora_shadow_pack_preflights").select("*").eq("user_id",input.userId).eq("shadow_pack_id",input.shadowPackId).limit(1)); } | ||
| export async function createShadowPackPreflightEvent(client: ShadowPackPreflightDbClient, input:{userId:string; preflightId:string; shadowPackId:string; eventType:string; message:string; metadata?:Record<string,unknown>}): Promise<ShadowPackPreflightEventRow|null>{ const row={id:randomUUID(),user_id:input.userId,preflight_id:input.preflightId,shadow_pack_id:input.shadowPackId,event_type:input.eventType,message:input.message,metadata:input.metadata??{},created_at:new Date().toISOString()}; return single(client.from("pandora_shadow_pack_preflight_events").insert(row).select("*").single()); } | ||
| export async function createOrRefreshShadowPackPreflight(client: ShadowPackPreflightDbClient, input:{userId:string; shadowPackId:string; requestId?:string}): Promise<ShadowPackPreflightRow>{ const built=await computeShadowPackPreflight(client,input); const existing=await getShadowPackPreflightByShadowPack(client,{userId:input.userId,shadowPackId:input.shadowPackId}); const now=new Date().toISOString(); const base={user_id:input.userId,shadow_pack_id:input.shadowPackId,namespace:built.shadow.namespace,active_master_pack_id:built.activeMaster?.id??null,request_id:built.request_id,status:built.status,diff_summary:built.diff_summary,risk_summary:built.risk_summary,warnings:built.warnings,updated_at:now}; const row=existing? await single(client.from("pandora_shadow_pack_preflights").update(base).eq("user_id",input.userId).eq("id",existing.id).select("*").single()) : await single(client.from("pandora_shadow_pack_preflights").insert({id:randomUUID(),...base,reviewer_notes:"",reviewer_decision:null,created_at:now,reviewed_at:null,approved_for_promotion_at:null,blocked_at:built.status==="blocked"?now:null}).select("*").single()); if(!row) throw new Error("Unable to create or refresh shadow pack preflight"); await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:row.id,shadowPackId:input.shadowPackId,eventType:existing?"preflight_refreshed":"preflight_created",message:"Shadow pack promotion preflight prepared. No production context pack mutation or promotion performed.",metadata:{risk_status:built.risk_summary.status,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); return row; } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refresh leaves reviewer state inconsistent.
On the update path, base overwrites status with the freshly computed status (e.g. ready_for_review/draft) but does not reset reviewer_decision, reviewer_notes, reviewed_at, approved_for_promotion_at, or blocked_at. Refreshing a previously approved preflight yields status = "ready_for_review" while reviewer_decision = "approved_for_promotion" and approved_for_promotion_at remain set — a contradictory record that downstream promotion checks may misread. Reset the reviewer fields when recomputing, or block refresh once a decision is recorded.
🛠️ Illustrative fix
-const base={user_id:input.userId,shadow_pack_id:input.shadowPackId,namespace:built.shadow.namespace,active_master_pack_id:built.activeMaster?.id??null,request_id:built.request_id,status:built.status,diff_summary:built.diff_summary,risk_summary:built.risk_summary,warnings:built.warnings,updated_at:now};
+const base={user_id:input.userId,shadow_pack_id:input.shadowPackId,namespace:built.shadow.namespace,active_master_pack_id:built.activeMaster?.id??null,request_id:built.request_id,status:built.status,diff_summary:built.diff_summary,risk_summary:built.risk_summary,warnings:built.warnings,updated_at:now,reviewer_decision:null,reviewer_notes:"",reviewed_at:null,approved_for_promotion_at:null,blocked_at:built.status==="blocked"?now:null};📝 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.
Suggested change
| export async function createOrRefreshShadowPackPreflight(client: ShadowPackPreflightDbClient, input:{userId:string; shadowPackId:string; requestId?:string}): Promise<ShadowPackPreflightRow>{ const built=await computeShadowPackPreflight(client,input); const existing=await getShadowPackPreflightByShadowPack(client,{userId:input.userId,shadowPackId:input.shadowPackId}); const now=new Date().toISOString(); const base={user_id:input.userId,shadow_pack_id:input.shadowPackId,namespace:built.shadow.namespace,active_master_pack_id:built.activeMaster?.id??null,request_id:built.request_id,status:built.status,diff_summary:built.diff_summary,risk_summary:built.risk_summary,warnings:built.warnings,updated_at:now}; const row=existing? await single(client.from("pandora_shadow_pack_preflights").update(base).eq("user_id",input.userId).eq("id",existing.id).select("*").single()) : await single(client.from("pandora_shadow_pack_preflights").insert({id:randomUUID(),...base,reviewer_notes:"",reviewer_decision:null,created_at:now,reviewed_at:null,approved_for_promotion_at:null,blocked_at:built.status==="blocked"?now:null}).select("*").single()); if(!row) throw new Error("Unable to create or refresh shadow pack preflight"); await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:row.id,shadowPackId:input.shadowPackId,eventType:existing?"preflight_refreshed":"preflight_created",message:"Shadow pack promotion preflight prepared. No production context pack mutation or promotion performed.",metadata:{risk_status:built.risk_summary.status,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); return row; } | |
| export async function createOrRefreshShadowPackPreflight(client: ShadowPackPreflightDbClient, input:{userId:string; shadowPackId:string; requestId?:string}): Promise<ShadowPackPreflightRow>{ | |
| const built=await computeShadowPackPreflight(client,input); | |
| const existing=await getShadowPackPreflightByShadowPack(client,{userId:input.userId,shadowPackId:input.shadowPackId}); | |
| const now=new Date().toISOString(); | |
| const base={ | |
| user_id:input.userId, | |
| shadow_pack_id:input.shadowPackId, | |
| namespace:built.shadow.namespace, | |
| active_master_pack_id:built.activeMaster?.id??null, | |
| request_id:built.request_id, | |
| status:built.status, | |
| diff_summary:built.diff_summary, | |
| risk_summary:built.risk_summary, | |
| warnings:built.warnings, | |
| updated_at:now, | |
| reviewer_decision:null, | |
| reviewer_notes:"", | |
| reviewed_at:null, | |
| approved_for_promotion_at:null, | |
| blocked_at:built.status==="blocked"?now:null | |
| }; | |
| const row=existing? await single(client.from("pandora_shadow_pack_preflights").update(base).eq("user_id",input.userId).eq("id",existing.id).select("*").single()) : await single(client.from("pandora_shadow_pack_preflights").insert({id:randomUUID(),...base,reviewer_notes:"",reviewer_decision:null,created_at:now,reviewed_at:null,approved_for_promotion_at:null,blocked_at:built.status==="blocked"?now:null}).select("*").single()); | |
| if(!row) throw new Error("Unable to create or refresh shadow pack preflight"); | |
| await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:row.id,shadowPackId:input.shadowPackId,eventType:existing?"preflight_refreshed":"preflight_created",message:"Shadow pack promotion preflight prepared. No production context pack mutation or promotion performed.",metadata:{risk_status:built.risk_summary.status,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); | |
| return row; | |
| } |
🤖 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-shadow-pack-preflight-service.ts` at line 41, In
createOrRefreshShadowPackPreflight, the update path in the base payload leaves
reviewer state from the previous review intact while recomputing status, which
can create contradictory rows. Reset reviewer_decision, reviewer_notes,
reviewed_at, approved_for_promotion_at, and blocked_at when refreshing an
existing preflight, or add a guard in
getShadowPackPreflightByShadowPack/createOrRefreshShadowPackPreflight to prevent
refresh after a decision is recorded. Keep the behavior aligned with the
existing insert path and the event creation that follows.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -H '20260704010000_pandora_shadow_pack_preflight.sql' supabase/migrations --exec cat {} \; | rg -n -i 'unique|constraint|shadow_pack_id'Repository: besfeng23/Memory
Length of output: 724
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the service and related helpers
fd -H 'pandora-shadow-pack-preflight-service.ts|pandora-shadow-pack-preflight' lib supabase -t f
echo '--- service outline ---'
ast-grep outline lib/services/pandora-shadow-pack-preflight-service.ts --view expanded || true
echo '--- relevant lines ---'
nl -ba lib/services/pandora-shadow-pack-preflight-service.ts | sed -n '1,220p'
echo '--- search for helper definitions/usages ---'
rg -n "getShadowPackPreflightByShadowPack|createOrRefreshShadowPackPreflight|single\\(|upsert\\(|onConflict|pandora_shadow_pack_preflights" lib supabase -g '!**/node_modules/**'Repository: besfeng23/Memory
Length of output: 12293
Use upsert instead of the get-then-insert split. The unique constraint on (user_id, shadow_pack_id) prevents duplicates, but two concurrent calls can still both miss the row and one insert will fail on the constraint instead of refreshing the existing preflight. Making this atomic avoids transient errors on a hot path.
🤖 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-shadow-pack-preflight-service.ts` at line 41, The
createOrRefreshShadowPackPreflight flow does a non-atomic get-then-insert/update
split that can race on the unique (user_id, shadow_pack_id) constraint. Refactor
the createOrRefreshShadowPackPreflight logic to use a single atomic upsert on
pandora_shadow_pack_preflights, preserving the current base fields and
insert-only defaults while still returning the row for the later
createShadowPackPreflightEvent call.
Source: Linters/SAST tools
| export async function getShadowPackPreflightByShadowPack(client: ShadowPackPreflightDbClient, input:{userId:string; shadowPackId:string}){ return single(client.from("pandora_shadow_pack_preflights").select("*").eq("user_id",input.userId).eq("shadow_pack_id",input.shadowPackId).limit(1)); } | ||
| export async function createShadowPackPreflightEvent(client: ShadowPackPreflightDbClient, input:{userId:string; preflightId:string; shadowPackId:string; eventType:string; message:string; metadata?:Record<string,unknown>}): Promise<ShadowPackPreflightEventRow|null>{ const row={id:randomUUID(),user_id:input.userId,preflight_id:input.preflightId,shadow_pack_id:input.shadowPackId,event_type:input.eventType,message:input.message,metadata:input.metadata??{},created_at:new Date().toISOString()}; return single(client.from("pandora_shadow_pack_preflight_events").insert(row).select("*").single()); } | ||
| export async function createOrRefreshShadowPackPreflight(client: ShadowPackPreflightDbClient, input:{userId:string; shadowPackId:string; requestId?:string}): Promise<ShadowPackPreflightRow>{ const built=await computeShadowPackPreflight(client,input); const existing=await getShadowPackPreflightByShadowPack(client,{userId:input.userId,shadowPackId:input.shadowPackId}); const now=new Date().toISOString(); const base={user_id:input.userId,shadow_pack_id:input.shadowPackId,namespace:built.shadow.namespace,active_master_pack_id:built.activeMaster?.id??null,request_id:built.request_id,status:built.status,diff_summary:built.diff_summary,risk_summary:built.risk_summary,warnings:built.warnings,updated_at:now}; const row=existing? await single(client.from("pandora_shadow_pack_preflights").update(base).eq("user_id",input.userId).eq("id",existing.id).select("*").single()) : await single(client.from("pandora_shadow_pack_preflights").insert({id:randomUUID(),...base,reviewer_notes:"",reviewer_decision:null,created_at:now,reviewed_at:null,approved_for_promotion_at:null,blocked_at:built.status==="blocked"?now:null}).select("*").single()); if(!row) throw new Error("Unable to create or refresh shadow pack preflight"); await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:row.id,shadowPackId:input.shadowPackId,eventType:existing?"preflight_refreshed":"preflight_created",message:"Shadow pack promotion preflight prepared. No production context pack mutation or promotion performed.",metadata:{risk_status:built.risk_summary.status,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); return row; } | ||
| export async function updateShadowPackPreflightReview(client: ShadowPackPreflightDbClient, input:{userId:string; preflightId:string; reviewerNotes:string; reviewerDecision:string}): Promise<ShadowPackPreflightRow>{ assertDecision(input.reviewerDecision); const row=await getShadowPackPreflight(client,input); const now=new Date().toISOString(); const status:ShadowPackPreflightStatus=input.reviewerDecision==="approved_for_promotion"?"approved_for_promotion":input.reviewerDecision==="blocked"?"blocked":"reviewed"; const patch:any={reviewer_notes:input.reviewerNotes??"",reviewer_decision:input.reviewerDecision,status,reviewed_at:now,updated_at:now}; if(status==="approved_for_promotion") patch.approved_for_promotion_at=now; if(status==="blocked") patch.blocked_at=now; const updated=await single(client.from("pandora_shadow_pack_preflights").update(patch).eq("user_id",input.userId).eq("id",input.preflightId).select("*").single()); if(!updated) throw new Error("Unable to update preflight review"); await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:input.preflightId,shadowPackId:row.shadow_pack_id,eventType:"preflight_reviewed",message:"Reviewer decision saved for future promotion consideration only; no promotion performed.",metadata:{reviewer_decision:input.reviewerDecision,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); return updated; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Review can resurrect an archived preflight.
getShadowPackPreflight returns the row regardless of status, so posting a review against an archived preflight silently transitions it back to reviewed/approved_for_promotion/blocked. Guard against reviewing terminal-state records.
🛡️ Suggested guard
const row=await getShadowPackPreflight(client,input);
+if(row.status==="archived") throw new Error("Cannot review an archived preflight");📝 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.
Suggested change
| export async function updateShadowPackPreflightReview(client: ShadowPackPreflightDbClient, input:{userId:string; preflightId:string; reviewerNotes:string; reviewerDecision:string}): Promise<ShadowPackPreflightRow>{ assertDecision(input.reviewerDecision); const row=await getShadowPackPreflight(client,input); const now=new Date().toISOString(); const status:ShadowPackPreflightStatus=input.reviewerDecision==="approved_for_promotion"?"approved_for_promotion":input.reviewerDecision==="blocked"?"blocked":"reviewed"; const patch:any={reviewer_notes:input.reviewerNotes??"",reviewer_decision:input.reviewerDecision,status,reviewed_at:now,updated_at:now}; if(status==="approved_for_promotion") patch.approved_for_promotion_at=now; if(status==="blocked") patch.blocked_at=now; const updated=await single(client.from("pandora_shadow_pack_preflights").update(patch).eq("user_id",input.userId).eq("id",input.preflightId).select("*").single()); if(!updated) throw new Error("Unable to update preflight review"); await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:input.preflightId,shadowPackId:row.shadow_pack_id,eventType:"preflight_reviewed",message:"Reviewer decision saved for future promotion consideration only; no promotion performed.",metadata:{reviewer_decision:input.reviewerDecision,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); return updated; } | |
| export async function updateShadowPackPreflightReview(client: ShadowPackPreflightDbClient, input:{userId:string; preflightId:string; reviewerNotes:string; reviewerDecision:string}): Promise<ShadowPackPreflightRow>{ assertDecision(input.reviewerDecision); const row=await getShadowPackPreflight(client,input); if(row.status==="archived") throw new Error("Cannot review an archived preflight"); const now=new Date().toISOString(); const status:ShadowPackPreflightStatus=input.reviewerDecision==="approved_for_promotion"?"approved_for_promotion":input.reviewerDecision==="blocked"?"blocked":"reviewed"; const patch:any={reviewer_notes:input.reviewerNotes??"",reviewer_decision:input.reviewerDecision,status,reviewed_at:now,updated_at:now}; if(status==="approved_for_promotion") patch.approved_for_promotion_at=now; if(status==="blocked") patch.blocked_at=now; const updated=await single(client.from("pandora_shadow_pack_preflights").update(patch).eq("user_id",input.userId).eq("id",input.preflightId).select("*").single()); if(!updated) throw new Error("Unable to update preflight review"); await createShadowPackPreflightEvent(client,{userId:input.userId,preflightId:input.preflightId,shadowPackId:row.shadow_pack_id,eventType:"preflight_reviewed",message:"Reviewer decision saved for future promotion consideration only; no promotion performed.",metadata:{reviewer_decision:input.reviewerDecision,no_core_memory_mutation_performed:true,no_promotion_performed:true}}); return updated; } |
🤖 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-shadow-pack-preflight-service.ts` at line 42, The
updateShadowPackPreflightReview flow currently allows archived preflights to be
reviewed and moved back into active statuses. Add a terminal-state guard in
updateShadowPackPreflightReview, using getShadowPackPreflight (or a follow-up
check on the returned row) to reject rows whose status is archived before
building the patch/update. Make sure the guard prevents any update/ event
creation for archived records and returns a clear error for that case.
Comment on lines
+42
to
+44
| create policy "shadow pack preflight events select own" on public.pandora_shadow_pack_preflight_events for select to authenticated using (user_id = auth.uid()); | ||
| create policy "shadow pack preflight events insert own" on public.pandora_shadow_pack_preflight_events for insert to authenticated with check (user_id = auth.uid()); | ||
| create policy "shadow pack preflight events update own" on public.pandora_shadow_pack_preflight_events for update to authenticated using (user_id = auth.uid()) with check (user_id = auth.uid()); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep preflight events append-only.
This UPDATE policy lets the owning user rewrite audit rows after insertion, which breaks the review trail. If a correction is needed, insert a compensating event instead. As per the service snippets, these rows are only ever appended today.
🛠️ Suggested fix
create policy "shadow pack preflight events select own" on public.pandora_shadow_pack_preflight_events for select to authenticated using (user_id = auth.uid());
create policy "shadow pack preflight events insert own" on public.pandora_shadow_pack_preflight_events for insert to authenticated with check (user_id = auth.uid());
-create policy "shadow pack preflight events update own" on public.pandora_shadow_pack_preflight_events for update to authenticated using (user_id = auth.uid()) with check (user_id = auth.uid());📝 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.
Suggested change
| create policy "shadow pack preflight events select own" on public.pandora_shadow_pack_preflight_events for select to authenticated using (user_id = auth.uid()); | |
| create policy "shadow pack preflight events insert own" on public.pandora_shadow_pack_preflight_events for insert to authenticated with check (user_id = auth.uid()); | |
| create policy "shadow pack preflight events update own" on public.pandora_shadow_pack_preflight_events for update to authenticated using (user_id = auth.uid()) with check (user_id = auth.uid()); | |
| create policy "shadow pack preflight events select own" on public.pandora_shadow_pack_preflight_events for select to authenticated using (user_id = auth.uid()); | |
| create policy "shadow pack preflight events insert own" on public.pandora_shadow_pack_preflight_events for insert to authenticated with check (user_id = auth.uid()); |
🤖 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 `@supabase/migrations/20260704010000_pandora_shadow_pack_preflight.sql` around
lines 42 - 44, The `pandora_shadow_pack_preflight_events` RLS setup currently
includes an `UPDATE` policy that allows the owning user to modify existing audit
rows, which conflicts with the intended append-only behavior. Remove the update
permission from the policy block tied to `pandora_shadow_pack_preflight_events`
and keep only the `SELECT` and `INSERT` policies so that `user_id = auth.uid()`
can still read and append new events but cannot rewrite prior ones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
pandora_shadow_pack_preflightsandpandora_shadow_pack_preflight_eventswith unique(user_id, shadow_pack_id), JSONdiff_summary/risk_summary, RLS policies scoped toauth.uid(), and supporting indexes.lib/services/pandora-shadow-pack-preflight-service.tsthat deterministically normalizes candidate and active master payloads, computesdiff_summaryandrisk_summary(blocked/high/medium/low rules), supports create/refresh, reviewer updates, archive, and emits preflight events; service never mutates core memory tables and always uses server-deriveduserId.prepare_shadow_pack_preflightinlib/services/pandora-operator-action-service.tswhere dry-run computes diff/risk only and approved execution creates/refreshes preflight rows/events only (responses includeno_core_memory_mutation_performed,no_promotion_performed, andpreflight_write_performed).app/api/pandora/shadow-pack-preflights/*andapp/api/pandora/shadow-context-packs/[id]/preflight).ShadowPackPreflightCard,ShadowPackDiffViewer,ShadowPackRiskSummary,ShadowPackPreflightList,ShadowPackPreflightReviewForm,ShadowPackPreflightControls), wire intoPandoraDashboardand the Shadow Context Pack Lab with a safe "Create/refresh preflight" control and a clear banner: “Preflight only — no production context pack mutation”.docs/pandora-shadow-pack-preflight.md, and update related docs describing thatapproved_for_promotionis not promotion.tests/unit/pandora-shadow-pack-preflight-service.test.tsand integrate operator-action/integration checks; guard tests ensure no core table mutation or service-role usage.Testing
npm run typecheck,npm run lint,npm run test,npm run build, andnpm run env:policy.631tests passing across98files, and the new preflight unit tests passed locally.Codex Task
Summary by CodeRabbit
New Features
Documentation
Tests