Skip to content

feat(worker): Phase 2a — port asset read routes to Hono + Drizzle/Hyperdrive#36

Open
chitcommit wants to merge 1 commit into
chore/schema-canonical-remediationfrom
feat/hono-phase-2a-asset-reads
Open

feat(worker): Phase 2a — port asset read routes to Hono + Drizzle/Hyperdrive#36
chitcommit wants to merge 1 commit into
chore/schema-canonical-remediationfrom
feat/hono-phase-2a-asset-reads

Conversation

@chitcommit
Copy link
Copy Markdown
Contributor

Summary

Phase 2a of the Express→Hono migration. Ports 5 read-only GET routes for the asset domain to the Cloudflare Worker (Hono + Drizzle/Hyperdrive).

Stacks on #34 (schema canonical remediation) → #33 (Phase 1 Hono skeleton).

Routes Ported

Route Express ref Status
GET /api/assets server/routes.ts:215 Migrated
GET /api/assets/stats server/routes.ts:234 Migrated
GET /api/assets/:id server/routes.ts:245 Migrated
GET /api/assets/:assetId/evidence server/routes.ts:335 Migrated
GET /api/assets/:assetId/timeline server/routes.ts:441 Migrated

Express server/routes.ts is untouched. New Hono routes mount BEFORE the 501 catch-all in worker/src/index.ts, so all other /api/* paths still return not_yet_migrated.

Decided Unilaterally

1. Ownership Key Change

Express used assets.userId === "chitty_" + req.auth.userId (Replit Auth era prefix). Hono uses assets.userId === claims.chitty_id (ChittyAuth era, canonical VV-G-LLL-SSSS-P-YM-C-X). Intentional semantic alignment, not a bug. Production data migrated to ChittyAuth will use chitty_id directly as user_id. Documented in worker/src/routes/assets.ts header.

2. Drizzle Adapter

drizzle-orm/postgres-js + postgres package — Cloudflare-recommended for Hyperdrive (TCP via proxy, no WebSocket shim needed). @neondatabase/serverless is not used in the Worker. Adds postgres@^3.4.9 to deps.

3. Test Auth Seam

registerAssetRoutes(app, authMiddleware) factory pattern lets integration tests inject a pass-through middleware instead of requireChittyAuth. Zero production code change for the auth bypass — production instantiation (export const assetRoutes) uses the real requireChittyAuth. Data path is fully real.

4. No Miniflare

Direct app.request() on the Hono app with env stub. Avoids the Hyperdrive-in-local-dev gap. Real DB calls go to Neon.

5. Test Branch Strategy

The Neon MCP create_branch only branches from the project default, which lacks the Phase 1.5 schema. Rather than re-applying the schema (drizzle-kit 0.18 lacks a push subcommand on this repo), tests run directly against the existing Phase 1.5 preview branch br-spring-star-aky6u1mc with unique chitty_id-scoped fixtures and full afterAll cleanup. Future Phase 2b can adopt full per-run branches once drizzle-kit is upgraded.

Validation Evidence

TypeCheck (npm run check)

Zero NEW errors in worker/src/** and the new test file. All 85 remaining errors are pre-existing in client/ and server/ (Replit-era code untouched by this PR).

$ grep -cE '^worker/' tsc.log
0

Integration Tests

All 12 tests pass against Neon branch br-spring-star-aky6u1mc:

✓ GET /api/assets — owner's assets (200)
✓ GET /api/assets — type=jewelry filter (200)
✓ GET /api/assets — intruder sees empty list (200, ownership filter)
✓ GET /api/assets/stats — correct aggregate shape (200)
✓ GET /api/assets/:id — returns asset for owner (200)
✓ GET /api/assets/:id — intruder gets 404 (no existence leak)
✓ GET /api/assets/:id — bad UUID returns 400
✓ GET /api/assets/:assetId/evidence — owner's evidence (200)
✓ GET /api/assets/:assetId/evidence — empty for intruder (200)
✓ GET /api/assets/:assetId/evidence — bad UUID returns 400
✓ GET /api/assets/:assetId/timeline — owner's timeline (200)
✓ GET /api/assets/:assetId/timeline — bad UUID returns 400

Test Files  1 passed (1)
     Tests  12 passed (12)
  Duration  8.38s

Sample SQL via Neon MCP run_sql (on br-spring-star-aky6u1mc)

The Drizzle-emitted shape for GET /api/assets ownership filter:

SELECT id, user_id, name, asset_type, current_value, verification_status
  FROM assets
 WHERE user_id = '01-A-CHT-ASST-P-5A-1-X'
 ORDER BY created_at DESC;

Result (after seeding one fixture row):

[
  {
    "id": "731f3ea9-08fe-40de-a934-45e728391023",
    "user_id": "01-A-CHT-ASST-P-5A-1-X",
    "name": "PR Evidence Sample — Patek Philippe",
    "asset_type": "jewelry",
    "current_value": "42500.00",
    "verification_status": "verified"
  }
]

Confirms the query parses, executes against Neon, and returns the expected row shape. Row was deleted after capture.

Wrangler Dry-Run

$ npx wrangler deploy --dry-run --env production
Total Upload: 573.21 KiB / gzip: 116.21 KiB
Your Worker has access to the following bindings:
  env.CHITTYASSETS_DB (Hyperdrive Config 4bd7964c46dd42be86e8a5e3dd0d7376)
  env.ASSETS (Assets)
  env.ENVIRONMENT, CHITTYAUTH_*, CHITTYMINT_URL, CHITTYCONNECT_URL, CHITTYLEDGER_URL
Tail Workers: chittytrack
--dry-run: exiting now.

Hyperdrive binding CHITTYASSETS_DB is wired and active in production env. No missing bindings.

Canonical Compliance

  • @canon: chittycanon://core/services/chittyassets on all new files
  • ENTITY_TYPES = ["P", "L", "T", "E", "A"] retained in env.ts — all five P/L/T/E/A present (no omission of Authority)
  • Ownership uses canonical ChittyID (Person entity) — never the Replit-era chitty_ prefix workaround

Stack

PR #36 (this) → #34#33 (base: chore/schema-canonical-remediation)

Test Plan

  • 200 for owner listing their assets
  • 200 with type filter
  • 200 empty list for non-owner (ownership filter)
  • 404 for other user's asset (no existence leak)
  • 400 for bad UUID format
  • stats shape correct (totals + by-type/status)
  • evidence list for owner's asset
  • timeline events ordered by eventDate DESC
  • typecheck clean (zero new errors)
  • dry-run deploy succeeds with Hyperdrive binding
  • Drizzle SQL validated end-to-end via Neon MCP

🤖 Generated with Claude Code

…erdrive

Routes ported (GET only):
- GET /api/assets (list with filters)
- GET /api/assets/stats (aggregates)
- GET /api/assets/:id (ownership-checked)
- GET /api/assets/:assetId/evidence
- GET /api/assets/:assetId/timeline

Ownership key decision: assets.userId === claims.chitty_id (ChittyAuth era).
Express used 'chitty_' + req.auth.userId (Replit Auth era). This is an
intentional semantic alignment, not a bug. Documented in PR for review.

DB adapter: drizzle-orm/postgres-js + Hyperdrive (per Cloudflare guidance).
Per-request instantiation; Hyperdrive pools externally.

Test seam: registerAssetRoutes(app, authMiddleware) factory pattern lets
integration tests inject a pass-through middleware (zero production code
change for the auth bypass). Data path is fully real against Neon.

Canonical: @canon annotations on all new files; P/L/T/E/A enumerated in env.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2316e354-dfb4-4e2f-844c-c689e008032f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/hono-phase-2a-asset-reads

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6cb5fd6c37

ℹ️ 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".

// -----------------------------------------------------------------
app.get("/assets", authMiddleware, async (c) => {
const claims = c.get("claims");
const userId = claims.chitty_id;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Query assets by legacy user key until ID migration completes

Using claims.chitty_id as assets.userId here breaks compatibility with the active write path: server/routes.ts still writes userId as chitty_${req.auth.userId} for asset/evidence/timeline records, and shared/schema.ts still defines assets.userId as FK to users.id (not users.chitty_id). In environments with existing legacy rows, these new read routes will return empty/404 for valid owner data until a full backfill/column migration is complete, so this route should either translate to the legacy key or join through users.chitty_id during the transition.

Useful? React with 👍 / 👎.

app.get("/assets", authMiddleware, async (c) => {
const claims = c.get("claims");
const userId = claims.chitty_id;
const db = getDb(c.env.CHITTYASSETS_DB.connectionString);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard missing Hyperdrive binding before dereferencing

All migrated handlers dereference c.env.CHITTYASSETS_DB.connectionString unconditionally, which will throw at runtime when the binding is not configured for the current worker environment (for example non-production/default envs in this repo config). That converts otherwise valid requests into 500s before any auth/query logic runs; add an explicit binding check and a controlled error response (or environment-specific fallback) before calling getDb().

Useful? React with 👍 / 👎.

Comment on lines +25 to +27
if (!TEST_DB_URL) {
throw new Error(
"TEST_DB_URL must be set to an ephemeral Neon branch connection string.",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Skip DB integration suite when TEST_DB_URL is absent

This file throws at module-load time when TEST_DB_URL is unset, so a normal npm test run fails before any tests execute in environments that do not provision a Neon branch URL. Because this suite is included by the repo-wide Vitest pattern, the hard throw makes unrelated test runs fail by default; gate the suite with describe.skipIf(...) (or similar) instead of throwing at import time.

Useful? React with 👍 / 👎.

if (ASSET_ID_1) {
await sql`DELETE FROM assets WHERE id = ${ASSET_ID_1}`;
}
await sql`DELETE FROM users WHERE id = ${OWNER_CHITTY_ID}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent teardown from deleting pre-existing shared user

The suite uses a fixed OWNER_CHITTY_ID and inserts with onConflictDoNothing(), so if that ID already exists in the shared Neon branch, the test reuses someone else’s row and then unconditionally deletes it in teardown. In shared/dev environments this can remove non-test data and break other tests or manual QA; generate a per-run owner ID (or track whether this test created the user) before issuing the final delete.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant