Skip to content

refactor(backend): move the API-key routes into their own feature module - #14456

Open
Pwuts wants to merge 2 commits into
devfrom
pwuts/split-v1-api-keys
Open

refactor(backend): move the API-key routes into their own feature module#14456
Pwuts wants to merge 2 commits into
devfrom
pwuts/split-v1-api-keys

Conversation

@Pwuts

@Pwuts Pwuts commented Sep 9, 2026

Copy link
Copy Markdown
Member

Moves the six /api/api-keys routes out of backend/api/features/v1.py into their own feature module, backend/api/features/api_keys/, with no change to the published API.

Why / What / How

Why. v1.py is a 3,022-line router module (2,902 after this PR) — the residue of the API layout that predates feature modules. Every one of its neighbours in backend/api/features/ (library/, store/, orgs/, search/, chat/, …) is already a package with its own routes module and router; v1.py is the part that was never split. It has eight distinct reasons to change, and the API-key routes are one of them.

What. The API KEY section — create_api_key, get_api_keys, get_api_key, delete_api_key, suspend_key, update_permissions — moves to backend/api/features/api_keys/routes.py. The three models it owns (CreateAPIKeyRequest, CreateAPIKeyResponse, UpdatePermissionsRequest) move from the shared backend/api/model.py to backend/api/features/api_keys/model.py; nothing else referenced them, and their APIKeyInfo/APIKeyPermission import goes with them.

How. The new router is mounted at prefix="/api/api-keys" with tags=["v1", "api-keys"], which is exactly the tag list the routes had before (mount-level ["v1"] plus route-level ["api-keys"]). That matters: custom_generate_unique_id builds each operationId from the first tag and the route summary, so both are load-bearing on the generated frontend client. The per-route dependencies=[Security(requires_user)] — identical on all six — is hoisted to the router; the dependency tree that FastAPI builds is unchanged, which the route-table diff below confirms.

The route docstrings stay, because FastAPI publishes them as each operation's description.

Changes 🏗️

  • New backend/api/features/api_keys/ package: routes.py (6 routes), model.py (3 models), routes_test.py.
  • backend/api/features/v1.py: −120 lines (the section plus its now-unused imports).
  • backend/api/model.py: −16 lines (the three models and their import).
  • backend/api/rest_api.py: +6 (import and mount).
  • backend/api/features/orgs/regression_test.py: test_create_api_key_sets_org_context reads the route's source with inspect.getsource, so its import moves to the new module. This was the only reference to a moved symbol anywhere in the repo.

Verified

The exported OpenAPI schema is byte-identical before and afterjson.dumps(before, sort_keys=True) == json.dumps(after, sort_keys=True) is True, and the committed frontend/src/app/api/openapi.json has an empty diff after running the export-api-schema hook. pnpm generate:api then produces no change to the generated client, and tsc --noEmit passes.

All 357 operations still resolve to the same handler. I dumped, for every path+method in the spec, which route Starlette's first-match-wins matcher actually picks, before and after. The only differences are the six API-key operations' module paths (backend.api.features.v1.*backend.api.features.api_keys.routes.*). Nothing else moved, and nothing became unresolvable — so no route shadows or is shadowed by the moved set. Registration order is also unchanged, because the API-key routes were last in v1_router and the new mount immediately follows it; the spec's four /api/api-keys* paths are the only ones that could collide, and there is no /api/{param} route anywhere.

Per-route, the app's route table is identical apart from those module names: tags, operationId, unique_id, summary, description, status code, response class, route dependencies and the full flattened dependency tree all match.

The new routes_test.py pins that surface for the seven splits still to come. I checked it can fail, with three mutations: renaming one summary (1 failed), hiding one route from the schema (2 failed), and changing the mount prefix to /api/apikeys (11 failed). All reverted, baseline green again.

Executed: backend/api in full (2333 passed), backend/util/architecture_test.py (3 passed), backend/blocks/test/test_block.py (1647 passed, 84 skipped), backend/api/features/orgs/regression_test.py (95 passed, 12 xfailed), backend/api/utils/api_key_auth_test.py + the two onboarding tests that import v1_router (50 passed). Two failures in the backend/api run are pre-existing and not from this change: ws_api_test.py::test_health_endpoint_returns_ok passes when run alone (suite interference), and search/content_handlers_integration_test.py::test_ensure_content_embedding_blocks fails identically on clean dev at 4ac3646361 with this commit absent. Not executed: anything outside backend/api, backend/util and backend/blocks.

Full evidence in a comment below.

Next in the sequence

Bottom-up through v1.py's own banner comments, so each PR is small and the API-surface check above is the acceptance test each time. Next is COPILOT SKILLS (4 routes + 3 models), then Schedules (5 routes + 1 model), then Graphs, Credits, Blocks, Onboarding, Auth. Bottom-up because the sections at the bottom are the smallest and the least entangled — Graphs, in the middle, is the one rest_api.py still imports five symbols from directly, so it wants the pattern to be settled first.

Agents and large language models used

Claude Code with Claude Opus 5

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • Exported the OpenAPI schema before and after and diffed it — byte-identical
    • Regenerated the frontend API client (pnpm generate:api) — no diff — and ran tsc --noEmit
    • Dumped which handler answers every one of the 357 spec operations, before and after — only the six moved endpoints' module paths differ
    • Diffed the app's full route table per route — tags, operation IDs, dependencies, response models all unchanged
    • Ran backend/api, architecture_test.py, test_block.py, and the org regression suite
    • Proved the new surface test can fail, with three mutations

🤖 Generated with Claude Code

…n feature module

v1.py is the 3,016-line residue of the pre-feature-module API layout. Its own
banner comments already mark eight sections; this moves the last of them —
the six /api/api-keys routes and the three request/response models they own —
into backend/api/features/api_keys/, alongside the other feature packages.

The published API is unchanged: the exported OpenAPI schema is byte-identical
before and after, and every one of the app's 357 operations still resolves to
the same handler.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: cfe00475-3401-4b9c-ae5e-9187967bda3f

📥 Commits

Reviewing files that changed from the base of the PR and between 4ac3646 and 801bc1e.

📒 Files selected for processing (8)
  • autogpt_platform/backend/backend/api/features/api_keys/__init__.py
  • autogpt_platform/backend/backend/api/features/api_keys/model.py
  • autogpt_platform/backend/backend/api/features/api_keys/routes.py
  • autogpt_platform/backend/backend/api/features/api_keys/routes_test.py
  • autogpt_platform/backend/backend/api/features/orgs/regression_test.py
  • autogpt_platform/backend/backend/api/features/v1.py
  • autogpt_platform/backend/backend/api/model.py
  • autogpt_platform/backend/backend/api/rest_api.py
💤 Files with no reviewable changes (2)
  • autogpt_platform/backend/backend/api/features/v1.py
  • autogpt_platform/backend/backend/api/model.py

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
Format Python code with `poetry run format`

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • autogpt_platform/backend/backend/api/rest_api.py
  • autogpt_platform/backend/backend/api/features/orgs/regression_test.py
  • autogpt_platform/backend/backend/api/features/api_keys/model.py
  • autogpt_platform/backend/backend/api/features/api_keys/routes.py
  • autogpt_platform/backend/backend/api/features/api_keys/routes_test.py
🔇 Additional comments (5)
autogpt_platform/backend/backend/api/features/api_keys/model.py (1)

1-20: LGTM!

autogpt_platform/backend/backend/api/features/api_keys/routes.py (1)

1-96: LGTM!

autogpt_platform/backend/backend/api/rest_api.py (1)

31-31: LGTM!

Also applies to: 383-387

autogpt_platform/backend/backend/api/features/orgs/regression_test.py (1)

3050-3050: LGTM!

Also applies to: 3052-3052

autogpt_platform/backend/backend/api/features/api_keys/routes_test.py (1)

1-49: LGTM!


Walkthrough

Changes

API key route extraction

Layer / File(s) Summary
API key contracts and handlers
autogpt_platform/backend/backend/api/features/api_keys/model.py, autogpt_platform/backend/backend/api/features/api_keys/routes.py
Adds request and response models and authenticated handlers for API key creation, listing, retrieval, revocation, suspension, and permission updates.
Router registration and legacy removal
autogpt_platform/backend/backend/api/rest_api.py, autogpt_platform/backend/backend/api/features/v1.py, autogpt_platform/backend/backend/api/model.py, autogpt_platform/backend/backend/api/features/orgs/regression_test.py
Mounts the dedicated router at /api/api-keys and removes the previous v1 handlers and model dependencies.
Published route contract tests
autogpt_platform/backend/backend/api/features/api_keys/routes_test.py
Verifies operation IDs, tags, the exact published route set, and the route handler module.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 801bc

API-key management endpoints are reorganized into a dedicated module while preserving their published paths, authentication behavior, schemas, and client contract. No merge-blocking production impact is identified.

Suggested reviewers: ntindle

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: moving the API-key routes into a dedicated feature module.
Description check ✅ Passed The description directly explains the route and model extraction, preserved API behavior, validation results, and related test updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch pwuts/split-v1-api-keys

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added cla: signed CLA signed by all contributors platform/backend AutoGPT Platform - Back end size/l labels Sep 9, 2026
@Pwuts

Pwuts commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

🤖 Evidence

The scripts below live outside the repo ($CLAUDE_JOB_DIR/tmp/); they are throwaway checks, not something to commit.

1. OpenAPI schema: byte-identical

$ poetry run export-api-schema --output openapi-before.json     # at 4ac3646361 (dev head)
$ poetry run export-api-schema --output openapi-after.json      # at e4264634ce
$ python3 -c "import json; a=json.load(open('openapi-before.json')); b=json.load(open('openapi-after.json')); \
              print('identical:', json.dumps(a,sort_keys=True)==json.dumps(b,sort_keys=True))"
identical: True

And through the real pre-commit path, against the committed spec:

$ poetry run export-api-schema --output ../frontend/src/app/api/openapi.json
$ (cd ../frontend && pnpm prettier --write ./src/app/api/openapi.json)
$ git diff --stat autogpt_platform/frontend/src/app/api/openapi.json
                                          # empty

$ (cd ../frontend && pnpm generate:api && npx tsc --noEmit)
🎉 autogpt_api_client - Your OpenAPI spec has been converted into ready to use orval!
$ git status --short autogpt_platform/frontend/src/app/api/
                                          # empty — generated client unchanged
                                          # tsc --noEmit: exit 0

2. Route resolution: all 357 operations, before vs after

For each path+method in the spec, walk app.routes in order and record the first Match.FULL — which is exactly what FastAPI does at request time.

357 operations resolved, 0 unresolved   (before)
357 operations resolved, 0 unresolved   (after)

The complete diff:

-  "DELETE /api/api-keys/{key_id}":            "backend.api.features.v1.delete_api_key",
+  "DELETE /api/api-keys/{key_id}":            "backend.api.features.api_keys.routes.delete_api_key",
-  "GET /api/api-keys":                        "backend.api.features.v1.get_api_keys",
+  "GET /api/api-keys":                        "backend.api.features.api_keys.routes.get_api_keys",
-  "GET /api/api-keys/{key_id}":               "backend.api.features.v1.get_api_key",
+  "GET /api/api-keys/{key_id}":               "backend.api.features.api_keys.routes.get_api_key",
-  "POST /api/api-keys":                       "backend.api.features.v1.create_api_key",
+  "POST /api/api-keys":                       "backend.api.features.api_keys.routes.create_api_key",
-  "POST /api/api-keys/{key_id}/suspend":      "backend.api.features.v1.suspend_key",
+  "POST /api/api-keys/{key_id}/suspend":      "backend.api.features.api_keys.routes.suspend_key",
-  "PUT /api/api-keys/{key_id}/permissions":   "backend.api.features.v1.update_permissions",
+  "PUT /api/api-keys/{key_id}/permissions":   "backend.api.features.api_keys.routes.update_permissions",

Nothing else changed handler, so nothing shadows or is shadowed by the moved set. Registration order is byte-identical too (diff order-before.txt order-after.txt → exit 0), because the six routes were last in v1_router and the new mount immediately follows it.

On shadowing specifically: the only paths under /api whose second segment is a path parameter or api-keys are the four /api/api-keys* paths themselves. There is no /api/{param} route anywhere in the spec.

3. Route table, per route

Dumped path, methods, name, endpoint, operation_id, unique_id, summary, description, tags, response_model, responses, status_code, deprecated, include_in_schema, response_class, route_dependencies and the full flattened dependency tree for all 365 routes; normalised memory addresses; keyed by methods + path so ordering cannot hide a change. The entire diff:

-  "endpoint": "backend.api.features.v1.delete_api_key",
+  "endpoint": "backend.api.features.api_keys.routes.delete_api_key",
-  "endpoint": "backend.api.features.v1.get_api_keys",
+  "endpoint": "backend.api.features.api_keys.routes.get_api_keys",
-  "endpoint": "backend.api.features.v1.get_api_key",
+  "endpoint": "backend.api.features.api_keys.routes.get_api_key",
-  "endpoint": "backend.api.features.v1.create_api_key",
+  "endpoint": "backend.api.features.api_keys.routes.create_api_key",
-  "response_model": "<class 'backend.api.model.CreateAPIKeyResponse'>",
+  "response_model": "<class 'backend.api.features.api_keys.model.CreateAPIKeyResponse'>",
-  "endpoint": "backend.api.features.v1.suspend_key",
+  "endpoint": "backend.api.features.api_keys.routes.suspend_key",
-  "endpoint": "backend.api.features.v1.update_permissions",
+  "endpoint": "backend.api.features.api_keys.routes.update_permissions",

route_dependencies and dependency_tree are unchanged on all six, which is what makes hoisting Security(requires_user) from the six decorators to the router safe rather than merely plausible.

4. The new test can fail

mutation result
summary="Suspend API key""Suspend an API key" 1 failed, 10 passed
include_in_schema=False on the suspend route 2 failed, 9 passed
mount prefix="/api/apikeys" in rest_api.py 11 failed
(all reverted) 11 passed

The first is the one worth having: operationId is f"{method}{Tag}{Summary}", so an innocuous summary rewrite during a later split silently renames the generated client's method. Two of the current IDs already show the quirk — getV1List user api keys, postV1Create new api key, spaces and all — because custom_generate_unique_id splits the summary on _ rather than whitespace. Pre-existing; noted here only because the next seven PRs will be moving summaries around.

5. Test runs

suite result
backend/api (full) 2333 passed, 12 xfailed, 2 failed + 1 error — both pre-existing, see below
backend/util/architecture_test.py 3 passed
backend/blocks/test/test_block.py 1647 passed, 84 skipped
backend/api/features/orgs/regression_test.py 95 passed, 12 xfailed
backend/api/features/api_keys/routes_test.py 11 passed
api_key_auth_test.py + onboarding_{profile,step}_test.py 50 passed
frontend tsc --noEmit exit 0

The two non-passes in the backend/api run:

  • ws_api_test.py::test_health_endpoint_returns_ok — a prisma.errors.DataError at setup. Passes when the file is run alone (20 passed), so it is suite interference within the directory, not this change.
  • search/content_handlers_integration_test.py::test_ensure_content_embedding_blocksAssertionError: Expected 'generate_embedding' to have been called once. Reproduced on clean dev (git checkout --detach HEAD~1, i.e. 4ac3646361, this commit absent): 1 failed, 6 passed, same test, same assertion. Pre-existing.

6. Reference sweep

Greps across autogpt_platform/ for every moved symbol, after the move:

  • features.v1 + any api-key symbol → no hits
  • create_api_key / get_api_keys / delete_api_key / suspend_key / update_permissions outside the new module → only backend.data.auth.api_key's own DB-layer create_api_key and its callers, which are untouched
  • CreateAPIKeyRequest / CreateAPIKeyResponse / UpdatePermissionsRequest → only the new module
  • from backend.api.model import … expecting an APIKey* name → no hits

The one real reference was orgs/regression_test.py:3050, which reads the route body with inspect.getsource and so is invisible to a grep for the route's path. It is fixed in this commit. There are no snapshot fixtures and no Playwright page objects touching this surface (no user-visible label changes).

…sence

backend/AGENTS.md bans getattr-based type dispatch; APIRoute is what the
assertion means, and it gives the type checker the narrowing for .endpoint.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.53968% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.49%. Comparing base (4ac3646) to head (801bc1e).
⚠️ Report is 3 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff            @@
##              dev   #14456    +/-   ##
========================================
  Coverage   81.48%   81.49%            
========================================
  Files        3553     3556     +3     
  Lines      265706   265731    +25     
  Branches    24618    24618            
========================================
+ Hits       216514   216555    +41     
+ Misses      43830    43727   -103     
- Partials     5362     5449    +87     
Flag Coverage Δ
platform-backend 86.46% <82.53%> (+<0.01%) ⬆️
platform-frontend-e2e 28.23% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Platform Backend 86.46% <82.53%> (+<0.01%) ⬆️
Platform Frontend 63.22% <ø> (+0.01%) ⬆️
AutoGPT Libs ∅ <ø> (∅)
Classic AutoGPT 28.43% <ø> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Pwuts
Pwuts marked this pull request as ready for review September 9, 2026 00:36
@Pwuts
Pwuts requested a review from a team as a code owner September 9, 2026 00:36
@Pwuts
Pwuts requested review from 0ubbe and Abhi1992002 and removed request for a team September 9, 2026 00:36
@Pwuts
Pwuts enabled auto-merge September 9, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: signed CLA signed by all contributors platform/backend AutoGPT Platform - Back end size/l

Projects

Status: 🆕 Needs initial review

Development

Successfully merging this pull request may close these issues.

1 participant