Repository navigation
chore: remove unused identity, invitation and admin token routes - #1413
Conversation
…utes Remove the organization read, update, delete, settings, invitation and member change routes, the current user status, profile, contact email, invitation and token revocation routes, the token based invitation routes, the workspace read, update and delete routes, and the admin invitation delivery, user erasure, member role and access token management routes. Admin access tokens still authenticate on the remaining admin routes. Their validation path, permission checks and audit events are unchanged. Delete the code that only these routes used: handlers and request types, the invitation, contact email and erasure services and repositories, the email sender, the read only token issuance setting, unused organization, workspace and user service methods, OpenAPI and deprecation entries, and admin policy entries. The users response keeps its contact email fields; needs_contact_email is always false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Delete tests that only exercised removed routes: invitations, contact email verification, user erasure, organization deletion, admin member role changes, admin invitation deliveries and admin access token management. Tests of behavior that remains now seed their setup directly: admin tokens, contact email columns, organization settings and allowed models. The retired routes table asserts 404 for every removed route. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
henrypark133
left a comment
There was a problem hiding this comment.
Code Review (multi-agent)
Intent: Remove 34 unused organization, user, invitation, workspace and admin routes plus code only they used; no tables or migrations dropped.
Stats: 10 findings (from 16 raw, 16 after filter, 10 after dedup) across 77 files. Reviewers run: correctness, security, performance, design, coverage. Reviewers failed: none. Body-only: 3
No Critical or High findings. Performance lane reported nothing.
- Medium Cached API key rejection after organization deactivation lost its only regression test (
crates/api/src/middleware/auth.rs:874-875, confidence 70, candidate — validate claim) (no diff position — body only) — anchor: crates/api/src/middleware/auth.rs:874
The deleted test drove the real auth middleware with a warm API-key cache, deactivated the organization, and expected 401. The production check (parent workspace/org lookup after a cache hit) remains, but organizations can still be deactivated by other means and no remaining test sends an API key through the real middleware after deactivation. - Medium Admin access tokens still authenticate but can no longer be listed or revoked (
crates/api/src/lib.rs:2179-2182, confidence 65, candidate — validate claim) — anchor: crates/api/src/lib.rs:2179
The create/list/revoke admin token routes are removed, but admin_middleware still acceptsadm_tokens (up to 720h expiry) on the remaining admin routes, including the encryption job routes. A leaked token now has no API revocation path and tokens cannot be inventoried; revocation exists only as a repository method with no production caller. docs/admin-access-token-permissions.md still says revocation 'remains unchanged'. - Medium Member add kept but member removal and role change removed (candidate - validate claim) (
crates/api/src/routes/api.rs:54-62, confidence 50, candidate — validate claim) — anchor: crates/api/src/routes/api.rs:56
POST /organizations/{id}/members stays, while DELETE and PUT on members/{id} are removed. If no other component serves removal or role changes, a wrongly added or offboarded member keeps organization access (usage, balance, workspaces, API keys). Please confirm removal is served elsewhere. - Low Orphan 'Organization Invitations API Models' banner (
crates/api/src/models.rs:2993-2995, confidence 90) — anchor: crates/api/src/models.rs:2993
All invitation models were removed; the section banner now sits above the next banner. - Low Stale nextest override for removed contact_email serial test (
.config/nextest.toml:50-54, confidence 65, candidate — validate claim) (no diff position — body only) — anchor: .config/nextest.toml:53
The override matchescontact_email::serial_, but contact_email.rs has no such tests now and the comment describes a removed daily send cap. - Low Inactive-organization insert guards lost their tests (
crates/database/src/repositories/workspace.rs:56-66, confidence 55, candidate — validate claim) — anchor: crates/database/src/repositories/workspace.rs:62
Tests that created a workspace or added a member under an inactive/deleted organization were deleted. The guards (is_active = true ... FOR SHARE) remain in workspace create and member add, which are kept routes, with no remaining test. - Low needs_contact_email=false pinned only for a user that has a contact email (
crates/api/src/conversions.rs:280-300, confidence 55, candidate — validate claim) — anchor: crates/api/src/conversions.rs:280
The only assertion is on a user with a verified contact email, so it would pass under most implementations. No test covers a placeholder-email wallet user without a contact email, the population the behaviour change affects. - Low Revoke-all DELETE /v1/users/me/tokens removed while token creation remains (candidate - validate claim) (
crates/api/src/routes/api.rs:87-94, confidence 50, candidate — validate claim) (no diff position — body only) — anchor: crates/api/src/routes/api.rs:87
Only per-refresh-token revocation remains. After a suspected compromise a user cannot invalidate all tokens in one call. - Low Remaining system-prompt test cannot tell whether the stored prompt is applied (
crates/api/tests/e2e_all/org_system_prompt.rs:8-48, confidence 50, candidate — validate claim) — anchor: crates/api/tests/e2e_all/org_system_prompt.rs:8
The only remaining test asserts status 200/completed, which holds whether the prompt is applied, dropped, or applied to the wrong organization. - Nit Doc comment now sits on the wrong method (
crates/services/src/organization/mod.rs:349-351, confidence 60, candidate — validate claim) — anchor: crates/services/src/organization/mod.rs:349
'Add a member by user ID...' now precedesget_organization_settings;add_member_validatedbelow has no doc.
Review · Status🟩 CompletedIronLoop completed the review and posted it to GitHub. ResultRun detailsAutomatic trigger · attempt 1 of 3 · completed in 9m 14s |
- Restore the cached API key rejection test, deactivating the organization with SQL - Restore inactive-organization insert guard tests for workspaces and API keys - Cover needs_contact_email == false for a user without a contact email - Make the org system prompt test assert the stored prompt reaches the provider - Add e2e coverage for users without a contact email on GET /users/me - Remove the orphan invitations banner in api models - Remove the stale contact_email serial override from nextest config - Move the add_member_validated doc comment back to its method Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed in a281caf. Body-only items from the review:
Checks: fmt and clippy ( |
There was a problem hiding this comment.
Review · Summary
Found one medium-severity retained-data cleanup regression.
Findings: 🟠 Medium 1
Code-specific findings are attached to the diff.
Validation
- ✅ Static inspection — Reviewed route registration, service wiring, and retained-data handling across the change.
- ✅ Captured unit and lint checks — The captured checks report unit tests and lint passing.
Review details
- Run:
622bcb0c-cfba-41da-b00c-5212159294ea - Attempts: 1
Removes 34 organization, user, invitation, workspace and admin routes that are no longer served from this API, together with the code that only they used.
Removed routes
GET/PUT/DELETE /v1/organizations/{id},GET/PATCH .../settings,POST .../members/invite-by-email,GET .../members/invitations,DELETE .../members/invitations/{id},DELETE .../invitations/{id},PUT/DELETE .../members/{id}GET /v1/users/me/status,PUT /v1/users/me/profile,POST/DELETE /v1/users/me/contact-email,GET /v1/users/me/invitations,POST .../invitations/{id}/acceptand/decline,DELETE /v1/users/me/tokens,POST /v1/users/contact-email/verifyGET /v1/invitations/{id},POST /v1/invitations/{id}/acceptGET/PUT/DELETE /v1/workspaces/{id}GET /v1/admin/invitation-email-deliveries,POST .../{id}/resend,POST /v1/admin/users/{id}/erasure/preview,POST /v1/admin/users/{id}/erasure,POST /v1/admin/user-erasures/lookup,PUT /v1/admin/organizations/{id}/members/{id},POST/GET /v1/admin/access-tokens,DELETE /v1/admin/access-tokens/{id}admin_retired_routes.rsasserts each removed route returns 404.Kept
All
/v1/auth/*routes;GET /v1/users/me, refresh-token list/revoke andPOST /v1/users/me/access-tokens; organization list/create, member list/add, workspace list/create; workspace API key list/create/update/delete, spend limit and usage history; organization usage, credit transfer, staking sync andPOST /v1/billing/costs; admin organization metrics, infra summary and the database/object encryption admin routes;/v1/services, docs and all inference routes. Admin access tokens still authenticate on the remaining admin routes; only their management endpoints are removed. No tables or migrations are dropped.Behaviour and config changes
GET /v1/users/mekeeps its response shape;needs_contact_emailis now alwaysfalsebecause contact email verification is no longer served here.INVITATION_EMAIL_*,RESEND_*,CONTACT_EMAIL_*andAUTH_ADMIN_READ_ONLY_TOKENS_ENABLED(only token issuance read it).SessionOnlyadmin policy class is removed.Tests
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings: passcargo test -p api --lib: 537 passedadmin_retired_routescontact_email,org_system_prompt,admin_audit,workspace_change_permissions,auth_tokens,duplicate_names,error_msgandstaking_wallet_bindingdrop only the removed-route cases.Some repository methods with no production caller remain as test fixtures (organization member removal, admin access token create/list/revoke); removing them is a follow-up.
🤖 Generated with Claude Code