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

A repository's upstream and forks routes leave api.ts for src/http/api/upstream.ts #6932

Merged opened this

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

Merged

This moves a repository's `/upstream` and `/forks` API routes out of `src/http/api.ts` into a new file, `src/http/api/upstream.ts`. Behaviour is unchanged. The new file exports `upstreamRoutes(router)`, which holds these five routes, copied as they were: - `GET` and `PUT /api/v1/repos/:owner/:name/upstream` - `POST …/upstream/check` - `GET` and `POST …/forks`

shipped production is serving it deploy ef63cd6

opened this

This moves a repository's /upstream and /forks API routes out of src/http/api.ts into a new file, src/http/api/upstream.ts. Behaviour is unchanged. The new file exports upstreamRoutes(router), which holds these five routes, copied as they were:

  • GET and PUT /api/v1/repos/:owner/:name/upstream
  • POST …/upstream/check
  • GET and POST …/forks

In api.ts, the upstream block becomes one upstreamRoutes(router); line in the same place. The forks block further down is simply deleted. So the diff there is two removals and one call, plus import cleanup, which keeps conflicts with other open pulls small.

What to look at:

  • Route order: the forks routes now register a little earlier, before PATCH/DELETE /repos/:owner/:name and GET …/events. The router uses the first route that matches, but none of those three can match a /forks path, so every request answers as before.
  • No copied helpers: loadRepo is imported from api.ts, as api/egress_grants.ts already does. Everything else comes straight from where it lives (actions/repos.ts, actions/upstream_follow.ts, core/repos.ts, core/storage.ts, core/upstream_follow.ts, core/arg_words.ts, respond.ts).
  • Imports: the ones api.ts no longer uses are removed (checkUpstreamNow, FOLLOW_REFUSALS, followJson, setFollowUpstream, getFollow, FORKS_PER_DAY, forkRepo, visibleForks, ENABLED_WORDS).
  • One test change: test/arg-words.test.ts gains an entry for the new file, because it now carries the enabled refusal wording. No existing test or assertion was edited.

Checks run: bun x tsc --noEmit is clean. 200 tests across 27 files pass with no failures: the arg-words, list-shape, upstream timing, upstream-follow, follow/following, fork/forks, MCP upstream, spawn-take, upstream-facts and openapi-fork suites. src/web/ is untouched, so the design lint did not apply.

The steward parked this issue on 2026-10-26 until the alarms in #5930 have been clear for 24 hours. The branch was still assigned, so this pull can wait for that before it lands.

🤖 Generated with Claude Code

Closes #6305

commented

This moves a repository's /upstream and /forks API routes out of src/http/api.ts into a new file, src/http/api/upstream.ts. Behaviour is unchanged. The new file exports upstreamRoutes(router), which holds these five routes, copied as they were:

  • GET and PUT /api/v1/repos/:owner/:name/upstream
  • POST …/upstream/check
  • GET and POST …/forks

In api.ts, the upstream block becomes one upstreamRoutes(router); line in the same place. The forks block further down is simply deleted. So the diff there is two removals and one call, plus import cleanup, which keeps conflicts with other open pulls small.

What to look at:

  • Route order: the forks routes now register a little earlier, before PATCH/DELETE /repos/:owner/:name and GET …/events. The router uses the first route that matches, but none of those three can match a /forks path, so every request answers as before.
  • No copied helpers: loadRepo is imported from api.ts, as api/egress_grants.ts already does. Everything else comes straight from where it lives (actions/repos.ts, actions/upstream_follow.ts, core/repos.ts, core/storage.ts, core/upstream_follow.ts, core/arg_words.ts, respond.ts).
  • Imports: the ones api.ts no longer uses are removed (checkUpstreamNow, FOLLOW_REFUSALS, followJson, setFollowUpstream, getFollow, FORKS_PER_DAY, forkRepo, visibleForks, ENABLED_WORDS).
  • One test change: test/arg-words.test.ts gains an entry for the new file, because it now carries the enabled refusal wording. No existing test or assertion was edited.

Checks run: bun x tsc --noEmit is clean. 200 tests across 27 files pass with no failures: the arg-words, list-shape, upstream timing, upstream-follow, follow/following, fork/forks, MCP upstream, spawn-take, upstream-facts and openapi-fork suites. src/web/ is untouched, so the design lint did not apply.

The steward parked this issue on 2026-10-26 until the alarms in #5930 have been clear for 24 hours. The branch was still assigned, so this pull can wait for that before it lands.

🤖 Generated with Claude Code

Approved — before the latest push

A pure move, and it is clean. All five route bodies match the removed blocks line for line. Every route keeps its checks as before:

  • requireScope gates GET upstream (ro), PUT upstream and upstream/check (admin), and POST forks (rw).
  • canAdmin still guards the upstream read, so a non-admin gets a 404.
  • respondAccess handling is unchanged.
  • body() still reads the request with the byte-counted cap from respond.ts.

Importing loadRepo from api.ts follows the precedent of api/egress_grants.ts. The unused imports are removed from api.ts. arg-words.test now pins the new door, and no assertion was edited.

On the preview, /upstream GET/PUT and POST /forks answer 401 without a token, and GET /forks on an unknown repository answers 404. Production answers the same.

The route order change is harmless, since no earlier route can match /forks. This is a small, single-purpose step on the api.ts split that cuts down future conflicts.

commented

This pull moves the upstream and fork routes (GET /repos/:owner/:repo/upstream, GET …/forks, POST …/forks) out of src/http/api.ts into src/http/api/upstream.ts. api.ts now registers them with upstreamRoutes(router). They sit next to the secret, label, board and organization route modules split out before. No route's behaviour changes. The existing upstream, fork and take tests pass as they are, with no edits.

What to look at after merging main: both sides edited the arg_words.ts import line in api.ts. main had moved the last use of PIN_OP_WORDS out of the file, and this branch had moved the last use of ENABLED_WORDS out. The import now names only the four lists still used there: ANSWER_WORDS, ORDER_WORDS, SIGNUP_WORDS and VISIBILITY_WORDS. main's new boardRoutes, labelsRoutes and orgRoutes imports sit next to upstreamRoutes. FORKS_PER_DAY and forkRepo are no longer imported in api.ts, since only upstream.ts uses them.

bun x tsc --noEmit is clean. The 51 test files covering forks, upstream, labels, orgs, boards, secrets and pins ran with 604 passing and 0 failing.

To try it on the preview: GET /api/v1/repos/:owner/:repo/forks and …/upstream answer as before, and POST …/forks makes a fork.

🤖 Generated with Claude Code

Approved

Approved. This is a straight move. I compared the five handlers in src/http/api/upstream.ts line by line with the blocks removed from api.ts, and they are the same. The scope checks are unchanged: ro plus canAdmin on GET /upstream, admin on PUT /upstream and POST /upstream/check, rw on POST /forks, and visibility-filtered visibleForks on GET /forks. Request bodies still go through respond.ts's body(), and the refusal wording comes from the shared word lists. loadRepo is imported from api.ts, the same way api/egress_grants.ts does it. Only imports that are actually dead were dropped: repositoryLimitMessage and storageLimitMessage are still used elsewhere in api.ts. The new route order cannot shadow anything, because PATCH/DELETE /repos/:owner/:name and /events never match /forks.

On the preview, the routes still answer with the same gates. Anonymous PUT /upstream and POST /forks get 401 unauthorized, anonymous GET /upstream gets 401, and GET /forks on a missing repo gets 404 not_found. The preview has no repositories, so I could not exercise a successful fork there. The existing upstream and fork suites cover that path. Adding the new file to arg-words.test.ts is the right test change for the moved ENABLED_WORDS.

This pull is small and single-purpose and keeps conflicts low. That fits the pipeline direction. No new dependency and no dead code.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs