Public: anyone can read this; the crew can change it.

A repository's egress-grant routes leave api.ts for src/http/api/egress_grants.ts #6877

Merged masoncrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

The three `/api/v1/repos/:owner/:name/egress-grants` routes (GET, POST, DELETE) move out of `src/http/api.ts` into a new file, `src/http/api/egress_grants.ts`, as `egressGrantRoutes(router)`. No behaviour changes.

shipped production is serving it deploy 2fa0c24

masoncrew opened this 17d ago

The three /api/v1/repos/:owner/:name/egress-grants routes (GET, POST, DELETE) move out of src/http/api.ts into a new file, src/http/api/egress_grants.ts, as egressGrantRoutes(router). No behaviour changes.

What moved where

The routes: the handlers and their comment are byte-identical to the block that left api.ts; the only new lines are the function wrapper and the imports.
api.ts: it holds no egress-grants route now. apiRoutes calls egressGrantRoutes(router) in the block's old spot. The actions/egress_grants.ts import went with the routes, and one import line was added for the new module.
Shared helper: loadRepo is now exported from api.ts, where it already lived, and the new module imports it rather than copying it. That makes the two files import each other, which is safe because loadRepo is only called inside a request handler, never while the modules load. The other helpers come from respond.ts and core/route_timing.ts as before.

One test changed, because it was wrong

test/openapi.test.ts ("every described route is registered in the router") read a fixed list of files, so it failed on DELETE …/egress-grants once the route left api.ts. It now also reads every file in src/http/api/.
test/openapi-coverage.test.ts still passed but only scanned the top level of src/http/, so it would no longer have checked the moved routes. It now reads src/http/api/ as well. Later moves into that directory won't need these tests edited again.

Checks run

bun x tsc --noEmit is clean.
These pass (205 tests, none failing): app-egress-grants, api-egress-grants-timing, tui-egress, egress-grants, openapi, openapi-oa-crew, openapi-coverage, route-timing-coverage, origins-security-auth, hollow, api-list-shape, cookie-flags, mcp, mcp-size.
Nothing in src/web/ changed, so design-lint was not needed.

Not done: none of the other route families were moved, and AGENTS.md is unchanged because it has no section on file layout.

To try it: call GET /api/v1/repos/<owner>/<name>/egress-grants with an admin-scoped token on the preview. The response should match production.

🤖 Generated with Claude Code

Closes #6304

masoncrew commented 17d ago

What moved where

The routes: the handlers and their comment are byte-identical to the block that left api.ts; the only new lines are the function wrapper and the imports.
api.ts: it holds no egress-grants route now. apiRoutes calls egressGrantRoutes(router) in the block's old spot. The actions/egress_grants.ts import went with the routes, and one import line was added for the new module.
Shared helper: loadRepo is now exported from api.ts, where it already lived, and the new module imports it rather than copying it. That makes the two files import each other, which is safe because loadRepo is only called inside a request handler, never while the modules load. The other helpers come from respond.ts and core/route_timing.ts as before.

One test changed, because it was wrong

test/openapi.test.ts ("every described route is registered in the router") read a fixed list of files, so it failed on DELETE …/egress-grants once the route left api.ts. It now also reads every file in src/http/api/.
test/openapi-coverage.test.ts still passed but only scanned the top level of src/http/, so it would no longer have checked the moved routes. It now reads src/http/api/ as well. Later moves into that directory won't need these tests edited again.

Checks run

bun x tsc --noEmit is clean.
These pass (205 tests, none failing): app-egress-grants, api-egress-grants-timing, tui-egress, egress-grants, openapi, openapi-oa-crew, openapi-coverage, route-timing-coverage, origins-security-auth, hollow, api-list-shape, cookie-flags, mcp, mcp-size.
Nothing in src/web/ changed, so design-lint was not needed.

Not done: none of the other route families were moved, and AGENTS.md is unchanged because it has no section on file layout.

To try it: call GET /api/v1/repos/<owner>/<name>/egress-grants with an admin-scoped token on the preview. The response should match production.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago

Clean move that does what #6304 asks. The three egress-grant handlers are byte-identical to the block that left api.ts, and egressGrantRoutes(router) is called in the block's old spot. The auth path is unchanged: requireScope(ctx, "admin") comes before loadRepo (which checks canRead) and before the body is read, and the site-admin check is still in actions/egress_grants.ts. body(), str() and respondAccess come from respond.ts as before, so the body cap is still on bytes actually read.

The only new coupling is the circular import of loadRepo between api.ts and api/egress_grants.ts. It is safe because loadRepo is called only inside handlers, and it follows the issue's rule to export a helper from where it lives instead of copying it. The two openapi tests now scan src/http/api/, so they keep covering the moved routes and later moves too. The other route-scanning tests register routes through apiRoutes at runtime, so they keep their coverage.

What I checked on this head: tsc --noEmit is clean. These 8 files pass, 86 tests, 0 failures: openapi, openapi-coverage, app-egress-grants, api-egress-grants-timing, egress-grants, route-timing-coverage, origins-security-auth, api-list-shape. On the preview, a guest gets 401 on GET, POST and DELETE /api/v1/repos/trevato/forge/egress-grants, and / answers 200. Approved.