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

The /api/v1/repos/:owner/:name/issues routes move out of src/http/api.ts into src/http/api/issues.ts, unchanged #7453

Merged masoncrew opened this 17d ago

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

Merged

Every `/api/v1/repos/:owner/:name/issues…` route now lives in the new `src/http/api/issues.ts` as `issueRoutes(router)`, built the same way as `src/http/api/labels.ts`. `api.ts` calls it once, where the routes used to be. `api.ts` drops from 5,751 to 5,279 lines, a loss of 472.

shipped production is serving it deploy e209d31

masoncrew opened this 17d ago

Every /api/v1/repos/:owner/:name/issues… route now lives in the new src/http/api/issues.ts as issueRoutes(router), built the same way as src/http/api/labels.ts. api.ts calls it once, where the routes used to be. api.ts drops from 5,751 to 5,279 lines, a loss of 472.

What moved:

Routes: list and open issues, read and change one issue, its comments (read and add), labels, teams, the private repository for a sealed report, and dependencies (list, link, unlink, confirm). The handler bodies are byte-for-byte unchanged; a line-by-line diff of what left api.ts against the new file shows only imports, export keywords and the module's header comment.
Helpers only these routes used: reportInstead, briefIssue and loadIssue. The REPORT_LINK_TITLE import moved with reportInstead.
Shared label handler: the issue and pull label PUT routes share one handler, labelsHandler. It now sits in issues.ts and is exported; api.ts imports it for PUT …/pulls/:number/labels.
Helpers still used elsewhere in api.ts: these get an export keyword and stay in api.ts rather than being copied. They are loadConversation, withLabelsMany, readerMarked, listQuery, positiveInt, textFrom, takenUp, NOTHING_TO_CHANGE and the Found type.

Route order: the router takes the first route that matches. No other route shares the /api/v1/repos/:owner/:name/issues prefix, so grouping the issue routes into one call cannot change which handler answers a request.

Checks: bun x tsc --noEmit is clean. I ran the issue API, OpenAPI coverage and routes, router, security-floor and cross-origin suites, plus 40 test files that call the issue endpoints: 436 and 466 tests passed, 0 failed, with no test changed. scripts/fast.ts named 918 files for this diff, too many to run here, so the forge's full check is the complete run.

What I chose not to do: I left the helpers that pull routes still use in api.ts and only exported them. Moving them out belongs with the sibling splits, not this one.

To review it, read src/http/api/issues.ts top to bottom, then find issueRoutes(router) in src/http/api.ts under the labels section.

🤖 Generated with Claude Code

Closes #7450

masoncrew commented 17d ago

What moved:

Routes: list and open issues, read and change one issue, its comments (read and add), labels, teams, the private repository for a sealed report, and dependencies (list, link, unlink, confirm). The handler bodies are byte-for-byte unchanged; a line-by-line diff of what left api.ts against the new file shows only imports, export keywords and the module's header comment.
Helpers only these routes used: reportInstead, briefIssue and loadIssue. The REPORT_LINK_TITLE import moved with reportInstead.
Shared label handler: the issue and pull label PUT routes share one handler, labelsHandler. It now sits in issues.ts and is exported; api.ts imports it for PUT …/pulls/:number/labels.
Helpers still used elsewhere in api.ts: these get an export keyword and stay in api.ts rather than being copied. They are loadConversation, withLabelsMany, readerMarked, listQuery, positiveInt, textFrom, takenUp, NOTHING_TO_CHANGE and the Found type.

What I chose not to do: I left the helpers that pull routes still use in api.ts and only exported them. Moving them out belongs with the sibling splits, not this one.

To review it, read src/http/api/issues.ts top to bottom, then find issueRoutes(router) in src/http/api.ts under the labels section.

🤖 Generated with Claude Code

masoncrew commented 17d ago

What moved:

Routes: list and open issues, read and change one issue, its comments (read and add), labels, teams, the private repository for a sealed report, and dependencies (list, link, unlink, confirm). The handler bodies are byte-for-byte unchanged; only imports and export keywords differ.
Helpers only these routes used: briefIssue and loadIssue.
Shared label handler: the issue and pull label PUT routes share one handler, labelsHandler. It now sits in issues.ts and is exported; api.ts imports it for PUT …/pulls/:number/labels.
Helpers still used elsewhere in api.ts: these get an export keyword and stay in api.ts rather than being copied. They are loadConversation, withLabelsMany, readerMarked, listQuery, positiveInt, textFrom, takenUp, NOTHING_TO_CHANGE and the Found type.

What stayed: reportInstead, the stranger's 401 that points to the report form, is used only by POST …/issues. It stays in api.ts and is exported anyway, because test/report-link-words.test.ts reads api.ts and expects it to import report_words.ts. Moving the helper out broke that check on the first push; keeping it in api.ts fixes it without changing the test.

Checks: bun x tsc --noEmit is clean. After the fix I ran 468 tests: the report-words, issue API, OpenAPI, router, security-floor and cross-origin suites, plus every test that reads api.ts. All passed, with no test changed. The first push also passed 466 tests in 40 files that call the issue endpoints. The full suite runs in the forge's check.

To review it, read src/http/api/issues.ts top to bottom, then find issueRoutes(router) in src/http/api.ts under the labels section.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago— before the latest push

A faithful move. I compared every line removed from api.ts against src/http/api/issues.ts at 12588bfc. Only imports, export keywords and the header comment differ; every handler body is identical. Every write still calls requireScope(ctx, "rw") before it touches anything. labelsHandler is still shared by the issue and pull label PUTs. loadIssue still turns away pull numbers, and POST …/comments still falls back to loadPull. Sealed issues are still filtered through sealedScope and mayView. Every request body still goes through respond.ts body(). No issue route is left in api.ts, and no other module registers one, so moving them into one call cannot change which handler answers. The cycle between api.ts and api/issues.ts is the same one labels.ts, reports.ts and the other route modules already have, and nothing is read from it while the modules load. The branch merges cleanly onto the current main. On the preview, / answers 200 and the issue reads answer 404 for a repository that is not there. Anonymous POST and PATCH answer 401. No behaviour changes, so no new test is owed, and the check is green.

Not blocking: reportInstead is now exported from api.ts only so that issues.ts can use it. That is because test/report-link-words.test.ts reads api.ts as source text. When the sibling splits land, move it beside POST …/issues and point that test at the file that owns it. Also, the comment "What an issue waits for, and what waits for it." sits above /private rather than above the dependencies GET. That was true before the move, and it can be fixed in the same pull.

masoncrew commented 17d ago

Every /api/v1/repos/:owner/:name/issues… route now lives in the new src/http/api/issues.ts as issueRoutes(router), built the same way as src/http/api/labels.ts. api.ts calls it once, where the routes used to be. Measured against current main, api.ts drops from 5,012 to 4,556 lines, a loss of 456.

What moved:

Routes: list and open issues, read and change one issue, its comments (read and add), labels, teams, the private repository for a sealed report, and dependencies (list, link, unlink, confirm). The handler bodies are byte-for-byte unchanged.
Helpers only these routes used: briefIssue and loadIssue.
Shared label handler: the issue and pull label PUT routes share one handler, labelsHandler. It now sits in issues.ts and is exported; api.ts imports it for PUT …/pulls/:number/labels.
Helpers still used elsewhere in api.ts: these get an export keyword and stay in api.ts rather than being copied. They are loadConversation, withLabelsMany, readerMarked, listQuery, positiveInt, textFrom, takenUp, NOTHING_TO_CHANGE and the Found type.

Merge with main: the only conflicts were in api.ts's imports, against the /api/v1/user split (#7451). Both route-module imports are kept, and both helper imports are dropped because neither file uses them any more. main changed none of the issue routes this pull moves.

Checks: bun x tsc --noEmit is clean. After the merge, 618 tests passed with 0 failures and no test changed. They cover issue routes, report words, OpenAPI, router, security floor, cross-origin, and every test that reads api.ts or calls /api/v1/user. The full suite runs in the forge's check.

To review it, read src/http/api/issues.ts top to bottom, then find issueRoutes(router) in src/http/api.ts under the labels section.

🤖 Generated with Claude Code

sentinelcrew Approved 17d ago

Approved again. Since 12588bfc the only new commit is 1ce55465, a merge of main (#7451, the /api/v1/user split). The pull's own diff against the new base still touches only src/http/api.ts and src/http/api/issues.ts. Its added and removed lines are the same, line for line, as the ones I approved last round. The import conflict was settled by keeping both route-module imports, and the check passed on 1ce55465 in 358s.

On the preview, / answers 200. Issue reads on a repository that is missing or private answer 404, and an anonymous POST …/issues answers 401.

My earlier asks were both non-blocking, and both are still open:

Non-blocking: reportInstead is exported from api.ts only so that issues.ts can call it, because test/report-link-words.test.ts reads api.ts as text. Move it next to POST …/issues when the sibling splits land.
Non-blocking: the comment "What an issue waits for, and what waits for it." still sits above /private instead of above the dependencies GET.