feat: add PATCH /v1/web/settings with per-workspace retention#44
Conversation
Backlog item #2 (Phase 3 dashboard). Persist auto-deletion retention per workspace and let admins edit workspace name + retention from the dashboard. - workspaces gains auto_deletion_days (int, default 30, CHECK 1..90) via migration 0007; schema + snapshot updated. - GET /v1/web/settings now reads the persisted value instead of the global USAGE_POLICY default. - PATCH /v1/web/settings (workos_access_token, admin scope, idempotent, actor rate limit) updates name + retention through runCommand and writes a workspace.settings.updated audit event. Bounds (1..90 days, name 1..120) enforced at the Zod contract and the DB check constraint. - Repo: workspaces.update port + both adapters; updateWebSettings in RepositoryCore; toWebSettings dedups the GET/PATCH shape. - Contract + OpenAPI golden regenerated; api + db tests extended. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
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 (2)
WalkthroughThis PR adds a complete PATCH /v1/web/settings endpoint for updating workspace settings. It introduces an auto_deletion_days column to the workspaces table with a 1–90 day range derived from the USAGE_POLICY TTL bounds. The feature includes database migrations, Drizzle schema updates, Zod/OpenAPI contract definitions, Repository interface and RepositoryCore implementation with idempotency support, entity adapter wiring, an API handler with member authorization and request validation, and extensive test coverage for success cases, validation errors, idempotency, and scope requirements. Sequence DiagramsequenceDiagram
participant Client
participant Handler as webUpdateSettings
participant Validation as UpdateWebSettingsRequest
participant Idempotent as runIdempotent
participant Core as RepositoryCore
participant DB as Database
Client->>Handler: PATCH /v1/web/settings + Idempotency-Key
Handler->>Handler: verify member actor with admin scope
Handler->>Validation: safeParse(workspace_name, auto_deletion_days)
Handler->>Idempotent: call with idempotency key
Idempotent->>Core: updateWebSettings(actor, workspaceName, autoDeletionDays)
Core->>DB: update workspaces set name, auto_deletion_days, updated_at
Core->>DB: insert workspace.settings.updated operation event
Core-->>Idempotent: return WebSettings
Idempotent-->>Handler: return result (or replay if key reused)
Handler-->>Client: 200 WebSettingsResponse
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-44.isaac-a46.workers.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/contracts/openapi/api.json`:
- Around line 7598-8070: The web.settings.update operation is missing a 403
forbidden response for admin-scope denial; add a "403" response object to the
responses map for operationId "web.settings.update" that mirrors the existing
error-envelope shape (an object with required "error" containing
"code","message", optional "docs" and "request_id") and include a descriptive
"description" like "Forbidden — insufficient scope" so clients can
programmatically handle authorization failures.
In `@packages/db/src/repository/core.ts`:
- Around line 479-503: The updateWebSettings path must validate
input.autoDeletionDays in repository core before calling
entities.workspaces.update to prevent local-adapter persistence of out-of-range
values; add a guard in updateWebSettings (inside the uow.command callback,
before calling entities.workspaces.update) that checks input.autoDeletionDays
against the same min/max bounds used by the DB (or a shared constant like
MIN_AUTO_DELETION_DAYS / MAX_AUTO_DELETION_DAYS), and throw a clear error (e.g.,
`invalid_auto_deletion_days`) when outside bounds so invalid values are rejected
before persisting.
🪄 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: 9e2d7f58-6c0f-4c2d-ae4a-62e4611692ba
📒 Files selected for processing (21)
apps/api/src/index.test.tsapps/api/src/index.tspackages/contracts/openapi/api.jsonpackages/contracts/src/mvp-contracts.test.tspackages/contracts/src/openapi/api.tspackages/contracts/src/openapi/shared.tspackages/contracts/src/routes.tspackages/contracts/src/web.tspackages/db/migrations/0007_workspace_auto_deletion_days.sqlpackages/db/snapshot/schema.sqlpackages/db/src/index.test.tspackages/db/src/policy.tspackages/db/src/queries/workspaces.tspackages/db/src/repository/core.tspackages/db/src/repository/interface.tspackages/db/src/repository/local-entities.tspackages/db/src/repository/ports.tspackages/db/src/repository/postgres-entities.tspackages/db/src/schema.tspackages/db/src/types.tsscripts/local-mvp-server.mjs
Address CodeRabbit review on #44. The local adapter has no DB CHECK constraint, so a direct updateWebSettings call could persist an auto_deletion_days value Postgres would reject. Guard against it in RepositoryCore using the shared MIN/MAX_AUTO_DELETION_DAYS bounds and lock the behavior with a repo test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
agent-paste PR preview is ready. API: https://agent-paste-api-pr-44.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. |
) * docs: mark PATCH /v1/web/settings (#44) done in status + web todo Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat: add operator platform lockdown endpoints behind requireOperator() POST /v1/web/admin/lockdowns and DELETE /v1/web/admin/lockdowns/{scope}/{target_id} set and lift reversible platform lockdowns, per ADR 0040/0046/0057. - New `operator` auth requirement + resolver accepting exactly two identities: a WorkOS session whose verified email is in OPERATOR_EMAILS, or a Cloudflare Access service-token JWT (RS256, aud-checked, common_name required). Every auth failure (incl. API-key bearer, non-operator email) collapses to a generic not_found (404) so the surface stays non-enumerable. - New `platform` actor type across contracts/db/commands; migration 0008 widens both actor_type CHECKs and creates `platform_lockdowns` (partial unique index on effective rows, forced RLS via app.platform). - setLockdown/liftLockdown run through runCommand; KV denylist wsd:/ad: keys are written on set and deleted on lift, after the Postgres commit (best-effort). - Operator principals rate-limit by platform:{id} with no workspace dimension. GET list endpoint deferred (tracked in web-app-todo.md). pnpm verify 70/70, db:check + openapi:check green, smoke:local green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: handle lockdown set/lift races and require DENYLIST.delete Address CodeRabbit review on PR #45: - Make KVNamespace.delete required so lockdown reversibility cannot be silently dropped; deleteDenylistEntry guards on env.DENYLIST presence. - platformLockdowns.insert returns a boolean via ON CONFLICT DO NOTHING RETURNING; setLockdown treats a lost partial-unique race as a replay instead of aborting the transaction. - platformLockdowns.markLifted returns a boolean via RETURNING; liftLockdown throws not_found (and emits no audit event) when it loses a lift race. - Normalize Cf-Access common_name (trim + lowercase) before use. - Assert platform actor on the lift handler and lifted audit event. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: throw on inconsistent lockdown insert conflict CodeRabbit re-review: when setLockdown's insert is rejected by the partial unique index but no effective row can be found, throw instead of emitting a misleading platform.lockdown.set audit event for a row that was never written. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
Phase 3 backlog item #2 (
docs/ops/project-status.md,docs/ops/web-app-todo.md). Adds the dashboard settings write path and makes retention a real per-workspace value instead of the global default.workspacesgainsauto_deletion_days(int, default 30,CHECK between 1 and 90) via migration0007; Drizzle schema + snapshot updated,db:checkgreen.GET /v1/web/settingsnow reads the persisted per-workspace value (was deriving from the globalUSAGE_POLICYdefault).PATCH /v1/web/settings(workos access token,adminscope, idempotent, actor rate limit) updates workspace name + retention throughrunCommandand writes aworkspace.settings.updatedaudit event.Changes
UpdateWebSettingsRequest(bounds derived frommvpUsagePolicy.min/max_ttl_seconds= 1..90 days; name 1..120);web.settings.updateroute;HttpMethodgainsPATCH; OpenAPI path + golden regenerated; MVP route-registry test updated.auto_deletion_dayson schema/type/snapshot + migration0007;workspaces.updateport + Postgres/local adapters;updateWebSettingsinRepositoryCore;toWebSettingsdedups the GET/PATCH shape; both workspace-provisioning paths set the default.webUpdateSettingshandler + mount, mirroring theweb.apiKeys.createmutation pattern.Risk
Low. Additive column with a safe default (30) and an idempotent, re-runnable migration; pre-launch so no back-compat surface. Bounds enforced at both the Zod contract and the DB check constraint. New route is admin-scoped and goes through the existing idempotent command/audit path.
Test plan
pnpm verifygreen (70/70 turbo tasks): lint, typecheck,test,openapi:check,db:check.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Database
Tests