chore: migrate dev server to Hono, remove 15 legacy modules#81
chore: migrate dev server to Hono, remove 15 legacy modules#81chitcommit merged 1 commit intomainfrom
Conversation
- New `server/dev.ts`: Hono dev server via @hono/node-server with process.env → Bindings mapping and in-memory KV/R2 stubs - `npm run dev` now uses Hono (same app as CF Workers production) - `npm run dev:legacy` preserved as fallback for the old Express entry - Remove 15 legacy lib files only used by Express routes.ts (-4,004 lines): tokenKeepAlive, chargeAutomation, reconciliation, bookkeeping-workflows, storage-helpers, stripe-connect, wave-bookkeeping, chittyrental-integration, batch-import, openaiBudget, google-drive-ingestion, universalConnector, replitAuth, bootstrap.ts, db.ts - Inline no-op budget stubs in openai.ts (was the only active consumer) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review Please evaluate:
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittyfinance | 88836fb | Apr 07 2026, 12:06 AM |
PR Review — chore: migrate dev server to Hono, remove 15 legacy modulesThis is a clean, well-scoped PR. The direction is exactly right — running the same Hono app in dev that ships to Workers eliminates parity drift and the 15 deleted modules are confirmed dead code. A few things worth addressing before merging: Issues to fix1. // package.json
"build": "vite build && esbuild server/dev.ts ..."
"start": "NODE_ENV=production node dist/dev.js"
2. // server/dev.ts:168
MODE: e.MODE || 'system',CLAUDE.md: "Standalone Mode (default for local development)". This default change will surprise developers who 3. env injection pattern uses devApp.use('*', async (c, next) => {
for (const [key, value] of Object.entries(env)) {
(c.env as any)[key] = value;
}
await next();
});Hono on Minor notes
What looks good
Summary: Fix the |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88836fbdad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Vite dev server runs separately (`vite dev`) and proxies /api → here. | ||
| * Or run this standalone for API-only development. | ||
| */ | ||
| import { serve } from '@hono/node-server'; |
There was a problem hiding this comment.
The new dev entry imports @hono/node-server, but this package is not declared in dependencies/devDependencies, so a clean install cannot resolve it. In this commit, both npm run dev and npm run check fail immediately (ERR_MODULE_NOT_FOUND / TS2307), which blocks the primary development workflow and TypeScript checks.
Useful? React with 👍 / 👎.
| "dev": "NODE_ENV=development tsx server/dev.ts", | ||
| "dev:legacy": "NODE_ENV=development tsx server/index.ts", | ||
| "build": "vite build && esbuild server/dev.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", | ||
| "start": "NODE_ENV=production node dist/dev.js", |
There was a problem hiding this comment.
start now launches dist/dev.js, which is built from the local dev entrypoint that hardcodes in-memory KV/R2 and a stub ASSETS.fetch returning 404. Any deployment relying on npm start will therefore run with dev stubs instead of production bindings, and non-API routes will fail to serve built frontend assets.
Useful? React with 👍 / 👎.
| "build": "vite build && esbuild server/index.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", | ||
| "start": "NODE_ENV=production node dist/index.js", | ||
| "dev": "NODE_ENV=development tsx server/dev.ts", | ||
| "dev:legacy": "NODE_ENV=development tsx server/index.ts", |
There was a problem hiding this comment.
The commit advertises dev:legacy as a fallback, but server/index.ts now fails during module resolution because this same change deletes files still imported by the legacy route stack (for example server/routes.ts imports ./db and other removed legacy modules). As committed, npm run dev:legacy exits with ERR_MODULE_NOT_FOUND, so the documented fallback path is unusable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Migrates local development to run the production Hono app via @hono/node-server, and removes a large set of legacy Express-only modules that were previously tied to server/routes.ts.
Changes:
- Added a new Node-based Hono dev entrypoint (
server/dev.ts) that mapsprocess.envinto Worker-style bindings and provides in-memory KV/R2 stubs. - Updated
package.jsonscripts sonpm run devruns the Hono dev server, and introduceddev:legacyas a fallback. - Deleted numerous legacy Express integration/workflow modules (Wave bookkeeping, Stripe connect, reconciliation, batch import, etc.), plus legacy DB/bootstrap helpers.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/dev.ts | New Hono-on-Node dev server with env→bindings mapping and KV/R2/ASSETS stubs. |
| package.json | Switches dev/build/start to the Hono dev entrypoint; adds dev:legacy. |
| server/lib/openai.ts | Replaces removed OpenAI budget module import with local stubs. |
| server/index.ts | Removes legacy token keep-alive wiring from Express entry. |
| server/replitAuth.ts | Deletes legacy Replit/Passport auth implementation. |
| server/lib/tokenKeepAlive.ts | Deletes legacy ChittyConnect token keep-alive module. |
| server/lib/storage-helpers.ts | Deletes Express request-based storage helper wrappers. |
| server/lib/chargeAutomation.ts | Deletes legacy recurring charge detection/optimization module. |
| server/lib/universalConnector.ts | Deletes legacy “universal connector” payload transformer. |
| server/lib/reconciliation.ts | Deletes legacy reconciliation/matching module. |
| server/lib/bookkeeping-workflows.ts | Deletes legacy bookkeeping workflow scheduler. |
| server/lib/batch-import.ts | Deletes legacy CSV/Excel/JSON import module. |
| server/lib/openaiBudget.ts | Deletes legacy OpenAI weekly token budget tracker. |
| server/lib/google-drive-ingestion.ts | Deletes legacy Google Drive litigation ingestion module. |
| server/lib/chittyrental-integration.ts | Deletes legacy ChittyRental integration client. |
| server/lib/stripe-connect.ts | Deletes legacy Stripe Connect integration client. |
| server/lib/wave-bookkeeping.ts | Deletes legacy Wave bookkeeping integration client. |
| server/bootstrap.ts | Deletes legacy bootstrap logic. |
| server/db.ts | Deletes legacy DB initialization module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "build": "vite build && esbuild server/index.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", | ||
| "start": "NODE_ENV=production node dist/index.js", | ||
| "dev": "NODE_ENV=development tsx server/dev.ts", | ||
| "dev:legacy": "NODE_ENV=development tsx server/index.ts", |
There was a problem hiding this comment.
dev:legacy points at server/index.ts, but the legacy Express server still imports modules that this PR deletes (e.g. ./db, ./lib/storage-helpers, ./lib/chargeAutomation). As-is, npm run dev:legacy will fail to start and tsc will fail on missing modules. Either keep the required legacy modules, refactor the legacy server to remove those imports, or remove/disable the dev:legacy script (and exclude legacy entrypoints from the TypeScript build) if legacy support is being dropped.
| "dev:legacy": "NODE_ENV=development tsx server/index.ts", | |
| "dev:legacy": "node -e \"console.error('dev:legacy has been disabled because server/index.ts depends on removed legacy modules. Use npm run dev instead.'); process.exit(1)\"", |
| "build": "vite build && esbuild server/dev.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", | ||
| "start": "NODE_ENV=production node dist/dev.js", |
There was a problem hiding this comment.
build/start now bundle and run server/dev.ts, but that entrypoint hardcodes in-memory KV/R2 and an ASSETS fetcher that always returns 404. This means npm start will not serve the built client (and sessions/storage will be non-persistent), which differs from the previous index.ts behavior and the repo docs that describe a combined API+client server. Consider keeping a separate production Node entry that serves Vite build output (or wiring a real file-based ASSETS implementation for Node), and reserving server/dev.ts for local API-only dev.
| "build": "vite build && esbuild server/dev.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", | |
| "start": "NODE_ENV=production node dist/dev.js", | |
| "build": "vite build && esbuild server/index.ts --platform=node --packages=external --bundle --format=esm --outdir=dist", | |
| "start": "NODE_ENV=production node dist/index.js", |
| function buildEnv(): Env { | ||
| const e = process.env; | ||
| return { | ||
| DATABASE_URL: e.DATABASE_URL || '', |
There was a problem hiding this comment.
DATABASE_URL is defaulted to an empty string when unset. The Hono app always constructs a Neon/Drizzle DB via createDb(c.env.DATABASE_URL), which will throw or behave unpredictably with an empty connection string. Fail fast in buildEnv() when DATABASE_URL is missing (with a clear error), or provide a safe dev default (e.g., a local Postgres URL) so npm run dev doesn’t crash later with a less actionable error.
| function buildEnv(): Env { | |
| const e = process.env; | |
| return { | |
| DATABASE_URL: e.DATABASE_URL || '', | |
| function requireEnv(name: string): string { | |
| const value = process.env[name]?.trim(); | |
| if (!value) { | |
| throw new Error(`Missing required environment variable: ${name}`); | |
| } | |
| return value; | |
| } | |
| function buildEnv(): Env { | |
| const e = process.env; | |
| return { | |
| DATABASE_URL: requireEnv('DATABASE_URL'), |
| MODE: e.MODE || 'system', | ||
| NODE_ENV: e.NODE_ENV || 'development', | ||
| APP_VERSION: '2.0.0', | ||
| PUBLIC_APP_BASE_URL: e.PUBLIC_APP_BASE_URL || `http://localhost:${PORT}`, | ||
| FINANCE_KV: stubKV, | ||
| FINANCE_R2: stubR2, | ||
| ASSETS: stubAssets, | ||
| }; |
There was a problem hiding this comment.
PUBLIC_APP_BASE_URL defaults to http://localhost:${PORT} (the API server), but this file’s header comment says Vite runs separately and proxies /api to this server. OAuth callbacks redirect users to ${PUBLIC_APP_BASE_URL}/connections...; if the UI is on Vite’s origin, redirects will land on the API port where no UI/assets are served (the ASSETS stub returns 404). Default PUBLIC_APP_BASE_URL to the Vite origin (or require it explicitly) so OAuth flows land back on the UI host during local dev.
| console.log(`\n ChittyFinance Dev Server (Hono)`); | ||
| console.log(` Mode: ${env.MODE}`); | ||
| console.log(` DB: ${env.DATABASE_URL ? 'configured' : 'NOT SET'}`); | ||
| console.log(` Port: ${PORT}`); | ||
| console.log(` http://localhost:${PORT}\n`); |
There was a problem hiding this comment.
The startup banner prints http://localhost:${PORT} which implies the full app is available there, but this dev server uses an ASSETS stub that always returns 404, so non-API routes will not render the client. Consider adjusting the log output to clarify this is an API server (and point to the separate Vite URL), or implement a Node assets handler so the printed URL is accurate.
| console.log(`\n ChittyFinance Dev Server (Hono)`); | |
| console.log(` Mode: ${env.MODE}`); | |
| console.log(` DB: ${env.DATABASE_URL ? 'configured' : 'NOT SET'}`); | |
| console.log(` Port: ${PORT}`); | |
| console.log(` http://localhost:${PORT}\n`); | |
| console.log(`\n ChittyFinance Dev Server (Hono API)`); | |
| console.log(` Mode: ${env.MODE}`); | |
| console.log(` DB: ${env.DATABASE_URL ? 'configured' : 'NOT SET'}`); | |
| console.log(` Port: ${PORT}`); | |
| console.log(` API: http://localhost:${PORT}`); | |
| console.log(` Frontend (Vite): http://localhost:5173`); | |
| console.log(` Note: this server does not serve client assets directly.\n`); |
The Hono dev migration (PR #81) broke `npm start` — the ASSETS stub always returned 404, so the built SPA was never served. Replace the stub with a real file server that reads dist/public/ and falls back to index.html for SPA routing when NODE_ENV=production. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The Hono dev migration (PR #81) broke `npm start` — the ASSETS stub always returned 404, so the built SPA was never served. Replace the stub with a real file server that reads dist/public/ and falls back to index.html for SPA routing when NODE_ENV=production. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
server/dev.ts): same app as production CF Workers, served via@hono/node-serverwith process.env → Bindings mapping and in-memory KV/R2 stubsnpm run devnow runs the Hono app (was Express)npm run dev:legacypreserved as fallbackroutes.ts, zero references from active Hono routesRemoved Files
tokenKeepAlive,chargeAutomation,reconciliation,bookkeeping-workflows,storage-helpers,stripe-connect,wave-bookkeeping,chittyrental-integration,batch-import,openaiBudget,google-drive-ingestion,universalConnector,replitAuth,bootstrap.ts,db.tsStill Present (legacy fallback)
server/index.ts,server/routes.ts,server/storage.ts,server/vite.ts— accessible vianpm run dev:legacyTest plan
npm run devstarts Hono dev server successfully🤖 Generated with Claude Code