Add operator lockdown UI#70
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
WalkthroughThis PR implements a web operator /admin UI for lockdown management. It adds LockdownForm and LockdownList React components, server mutations setLockdownFn and liftLockdownFn validated by updated lockdown contracts, a route loader that fetches lockdowns and returns them as loader data, comprehensive tests and fixtures for mutations and components, and documentation updates across project status, changelog, coverage, backlog, and web-app TODO files. Sequence DiagramsequenceDiagram
participant Operator
participant AdminPage
participant LockdownForm
participant setLockdownFn
participant LockdownAPI
participant LockdownList
participant liftLockdownFn
Operator->>AdminPage: open /admin
AdminPage->>LockdownList: loader GET /v1/web/admin/lockdowns
LockdownList->>LockdownAPI: GET /v1/web/admin/lockdowns
Operator->>LockdownForm: submit scope,target_id,reason_code
LockdownForm->>setLockdownFn: validated input
setLockdownFn->>LockdownAPI: POST /v1/web/admin/lockdowns
setLockdownFn-->>LockdownForm: result
Operator->>LockdownList: click Lift
LockdownList->>liftLockdownFn: scope,target_id
liftLockdownFn->>LockdownAPI: DELETE /v1/web/admin/lockdowns/{scope}/{target_id}
liftLockdownFn-->>LockdownList: result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-70.isaac-a46.workers.dev |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@apps/web/src/components/admin/LockdownForm.tsx`:
- Around line 42-57: The try/finally around the setLockdownFn call doesn't catch
rejected promises, so wrap the mutation call to explicitly handle rejections
from setLockdownFn (transport/runtime failures) by adding a catch block that
logs/pushes a user-facing error via push(errorToast(...)) and returns early;
keep the existing result.error handling and the finally that calls
setPending(false). Reference setLockdownFn, push, errorToast, setPending, and
onSuccess when adding the rejection handler so rejected promises produce the
same actionable toast behavior as result.error.
In `@apps/web/src/components/admin/LockdownList.tsx`:
- Around line 27-41: The try/finally around liftLockdownFn doesn't catch
rejections, so thrown errors escape; wrap the await liftLockdownFn call in a
try/catch/finally (or add a catch) to handle thrown errors from liftLockdownFn,
call push(errorToast("Couldn't lift lockdown", err)) inside the catch (or a
generic message if err has no message), and return after pushing so onLift isn't
called; keep the existing finally block that updates setPendingIds(key) to
ensure pending cleanup always runs.
In `@apps/web/src/routes/_authed.admin.tsx`:
- Around line 36-44: handleSuccess currently does two refreshes: it calls
loadLockdownsFn(), sets local state with setLockdowns(updated), then calls
router.invalidate() which re-runs the loader and fetches again; fix by choosing
a single refresh path — prefer router.invalidate() as the single source of
truth. Update handleSuccess to remove the direct fetch and setLockdowns call
(remove loadLockdownsFn() and setLockdowns(updated)) and simply await
router.invalidate(); rely on Route.useLoaderData()/initialLockdowns to supply
the updated data after invalidation (or if you must keep local caching, remove
router.invalidate() instead and only update via setLockdowns). Ensure references
to handleSuccess, loadLockdownsFn, setLockdowns, and router.invalidate are
adjusted accordingly.
In `@docs/ops/web-app-todo.md`:
- Around line 58-59: Update the contradictory sentence that claims the "operator
lockdown list remains EmptyState" to reflect that the `/admin` operator lockdown
listing, set, and lift functionality has been implemented (per the Shipped/Done
bullets); replace that sentence with a clear single-source-of-truth line stating
that the `/admin` view now lists lockdowns and allows operators to set or lift
workspace/artifact lockdowns (remove references to "EmptyState" for the operator
lockdown list).
In `@packages/contracts/src/lockdown.ts`:
- Around line 15-19: The validators allow whitespace-only target_id values;
update the schema for LiftLockdownRequest (and align SetLockdownRequest and
LockdownDetail) to trim input before validating length so strings like " " are
rejected — replace the current z.string().min(1) usage for the target_id fields
with a trimmed-and-validated variant (e.g., use z.string().trim().min(1) or an
equivalent transform+min) for the named types LiftLockdownRequest,
SetLockdownRequest, and LockdownDetail so all three enforce non-whitespace IDs
consistently.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 60f5a952-6670-4a6e-8ce8-1aa2a9894bc7
📒 Files selected for processing (14)
apps/web/src/components/admin/LockdownForm.tsxapps/web/src/components/admin/LockdownList.tsxapps/web/src/routes/_authed.admin.tsxapps/web/src/server/web-mutations.tsapps/web/test/fixtures.tsapps/web/test/routes.test.tsxapps/web/test/web-mutations.test.tsdocs/ops/project-status.mddocs/ops/status/changelog.mddocs/ops/status/coverage.mddocs/ops/status/implementation.mddocs/ops/status/phase-backlog.mddocs/ops/web-app-todo.mdpackages/contracts/src/lockdown.ts
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-70.isaac-a46.workers.dev |
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-70.isaac-a46.workers.dev |
|
agent-paste PR preview resources were cleaned up. The pr-preview-${context.issue.number} environment is left in place; remove it from the GitHub UI if desired. |
Summary
Builds the Phase 3 operator lockdown screen so allowed operators can manage active workspace and artifact lockdowns from the web app instead of relying only on the API.
Changes
/adminloader wiring for active lockdowns through the existing operator API.Risk: HIGH
apps/webadmin route/components/server mutations,packages/contracts, ops status docs./admin; no DB/query changes.Test plan
bun run ci:checkSummary by CodeRabbit