refactor: split src/pecans.ts into src/http route modules (Phase 7 PR C) - #45
Merged
Conversation
Pure move, third and final Phase 7 PR: handler bodies relocate verbatim
into src/http/{downloads,updates,api,notes}.ts against a narrow
PecansHttpContext, and the query-parsing helpers into src/http/query.ts
(re-exported from the package root, so consumer imports are unchanged).
Pecans stays the composition root: router wiring, the
beforeDownload/afterDownload events around serveAsset, and thin
delegating methods that keep the class surface (and host overrides of
public methods like queryReleases) working exactly as before. HTTP
behavior is unchanged; the contract suite passes untouched.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015zUyj4PpSog9RkrYxzFRhM
There was a problem hiding this comment.
Pull request overview
Refactors Pecans’ Express composition root by splitting the HTTP handler implementations out of src/pecans.ts into dedicated src/http/* modules while keeping the existing routing surface and delegation points on Pecans.
Changes:
- Introduces
PecansHttpContextand moves route handlers intosrc/http/{downloads,updates,api,notes}.tsas plain(ctx, req, res, next)functions. - Moves request query/param helpers into
src/http/query.tsand re-exports them fromsrc/pecans.tsto preserve existing import paths. - Keeps
Pecansas the composition root with thin delegate methods that call into the new handler modules.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pecans.ts | Converts handler bodies into delegates and re-exports query helpers; builds a shared HTTP context. |
| src/http/context.ts | Defines the narrow capability interface (PecansHttpContext) used by handlers. |
| src/http/query.ts | Houses route param + query parsing/validation helpers (moved from Pecans). |
| src/http/downloads.ts | Implements legacy and discrete download routes using the shared context. |
| src/http/updates.ts | Implements Squirrel.Mac and Squirrel.Windows update endpoints using the shared context. |
| src/http/api.ts | Implements /api/channels, /api/status, and /api/versions handlers. |
| src/http/notes.ts | Implements /notes{/:version} handler with formatting/validation preserved. |
The only non-move changes in this PR, driven by review: the deprecated /update redirect and the Squirrel.Mac feed url previously interpolated raw req.query values, so repeated params (arrays) produced malformed urls. The redirect now requires single-string platform/version and encodes them into the path; filetype coerces to a single string (default zip) and is encoded into the feed url. Typo fix in a moved comment (Copilot review x3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zUyj4PpSog9RkrYxzFRhM
Per maintainer preference: each src/http module exports create<Route>Handler(ctx) returning a standard (req, res, next) handler, so ctx binds once at composition time and the returned functions are plain express RequestHandlers. Pecans builds the handler set in its constructor and keeps the thin delegating methods, so the class surface and host overrides are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zUyj4PpSog9RkrYxzFRhM
Regression coverage for the d5ac57f guards (Copilot review): repeated /update query params 400 instead of forming a malformed redirect, the redirect path is encoded, an explicit ?filetype override lands in the Squirrel.Mac feed url, and repeated ?filetype params fall back to zip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zUyj4PpSog9RkrYxzFRhM
This was referenced Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third and final Phase 7 PR (backend abstraction ✅ → resolution pipeline ✅ → HTTP split), closing out the modernization roadmap. Handler bodies relocate verbatim; HTTP behavior is unchanged and the contract suite passes untouched.
Layout
src/pecans.ts(682 lines) splits into:src/http/query.tsgetStringParam,validateReqQuery*,get*FromQuery) — re-exported from the package root, so consumer imports are unchangedsrc/http/downloads.tscreateDownloadHandler,createDlHandler,createDlFilenameHandlersrc/http/updates.tscreateUpdateRedirectHandler,createUpdateOSXHandler(Squirrel.Mac),createUpdateWinHandler(Squirrel.Windows RELEASES)src/http/api.tscreateApiChannelsHandler,createApiStatusHandler,createApiVersionsHandlersrc/http/notes.tscreateNotesHandlersrc/http/context.tsPecansHttpContext— the narrow capability interface the handlers get from the composition rootsrc/pecans.ts(~300 lines)Pecansstays the composition root: router wiring,beforeDownload/afterDownloadevents aroundserveAsset, and thin delegating methodsHandler factories
Each module exports
create<Route>Handler(ctx)returning a standard(req, res, next)Express handler — the ecosystem's higher-order-handler idiom (cors(),multer(opts), …): ctx binds once at composition time and the product is a plainRequestHandler, independently mountable withoutPecans.Pecansbuilds the handler set in its constructor.Fidelity notes
handleDownload→this.handlers.download(...)), and the router binds the methods — preserving the public/protected surface and the subclass-override point; the unit suite passes without modification. (Dropping the delegates and mounting factory outputs directly is a possible follow-up if handler overriding is declared a non-goal.)dlstill flows throughctx.queryReleases → Pecans.queryReleases, exactly like the oldthis.queryReleasescall, so host overrides of that public method keep applying./download/versionbefore/download/:tag/:filenameshadowing fix) and the router-scopederrorHandler()are byte-identical.d5ac57f): the deprecated/updateredirect and the Squirrel.Mac feed url now guard non-string query values (single-string required + encoded) instead of interpolating rawreq.query.Verification
npm test: 31 files / 718 passed, unchanged🤖 Generated with Claude Code
https://claude.ai/code/session_015zUyj4PpSog9RkrYxzFRhM