feat(worker): Phase 3a — port asset write routes (CRUD + evidence attach)#39
feat(worker): Phase 3a — port asset write routes (CRUD + evidence attach)#39chitcommit wants to merge 1 commit into
Conversation
…ach) Ports 4 Express write routes to Hono/Drizzle/Hyperdrive: - POST /api/assets (create asset + 'acquisition' timeline event, transactional) - PUT /api/assets/:id (ownership-checked update, 404 not 403 on mismatch) - DELETE /api/assets/:id (ownership-checked, cascades timeline_events + evidence) - POST /api/assets/:assetId/evidence (parent-asset ownership check + 'evidence_added' timeline event, all in one transaction) Security boundary: insertAssetSchema.omit(SERVER_OWNED) strips client-supplied userId/chittyId/trustScore/verificationStatus/chittyChainStatus/blockchain* fields. Client cannot escalate. Same pattern for evidence inserts. Deferred (documented divergence from Express): ChittyID minting + ChittyTrust calculation on create. Express called services.id.generate() + services.trust synchronously; worker leaves chitty_id NULL and trust_score at schema default '0.0' for an async minter pass. Tests confirm defaults. NO MOCKS: 18 new integration tests exercise real Neon (test branch br-spring-star-aky6u1mc). Full worker suite: 47 tests, all pass. End-to-end INSERT validated via Neon MCP run_sql_transaction (asset row + timeline_events row + cleanup). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a25262235d
ℹ️ 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".
| .delete(evidence) | ||
| .where(and(eq(evidence.assetId, id), eq(evidence.userId, userId))); |
There was a problem hiding this comment.
Delete all dependent rows before removing an asset
The delete transaction removes timeline_events and evidence only, but assets.id is also referenced by warranties.asset_id and insurance_policies.asset_id, and evidence.id is referenced by ai_analysis_results.evidence_id (see shared/schema.ts / drizzle/0001_init.sql). For assets that already have warranties/insurance or analyzed evidence, DELETE /api/assets/:id will hit FK violations and return 500 instead of 204, so owners cannot delete valid assets with common dependent data.
Useful? React with 👍 / 👎.
| .delete(timelineEvents) | ||
| .where( | ||
| and( | ||
| eq(timelineEvents.assetId, id), | ||
| eq(timelineEvents.userId, userId), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Preserve immutable timeline events on asset deletion
This route hard-deletes timeline rows (timeline_events) when an asset is removed, which breaks the repo’s explicit append-only audit requirement in /workspace/chittyassets/AGENTS.md (“Do not modify timelineEvents records — this table is append-only”). In environments that rely on timeline history for compliance/audit, this permanently erases evidence of prior state transitions.
Useful? React with 👍 / 👎.
| .delete(evidence) | ||
| .where(and(eq(evidence.assetId, id), eq(evidence.userId, userId))); |
There was a problem hiding this comment.
Avoid deleting evidence rows in asset delete path
The asset delete transaction also deletes from evidence, but /workspace/chittyassets/AGENTS.md states evidence has a 7-year minimum retention and explicitly says not to add delete paths for evidence. As written, a normal DELETE /api/assets/:id request can irreversibly remove retained evidence records, violating the service’s retention policy.
Useful? React with 👍 / 👎.
| .delete(evidence) | ||
| .where(and(eq(evidence.assetId, id), eq(evidence.userId, userId))); |
There was a problem hiding this comment.
Delete dependent rows by asset ID, not by owner ID
The cleanup predicates include eq(...userId, claims.chitty_id), so dependent rows on the same asset_id but a different user_id are left behind and can still block the asset delete via FK constraints. This is a real migration risk because the previous Express evidence writer (server/routes.ts POST /api/assets/:assetId/evidence) did not verify asset ownership before insert, so legacy rows can exist with mismatched user ownership for the same asset.
Useful? React with 👍 / 👎.
Summary
Phase 3a of the Express→Hono migration: ports 4 write routes from
server/routes.tsto Cloudflare Worker + Hono + Drizzle/Hyperdrive.Stacks on #38 → #37 → #36 → #34 → #33.
Routes ported
/api/assetsroutes.ts:259timeline_eventsrow (acquisition), same txn/api/assets/:idroutes.ts:306/api/assets/:idroutes.ts:323timeline_events+evidencein txn/api/assets/:assetId/evidenceroutes.ts:346timeline_eventsrow (evidence_added,related_evidence_idset), same txnZod schemas (security boundary)
insertAssetSchema.omit(SERVER_OWNED)strips client-supplied:userId, chittyId, chittyIdV2, trustScore, blockchainHash, blockNumber, ipfsHash, freezeTimestamp, settlementTimestamp, mintingFee, verificationStatus, chittyChainStatus, deletedAt. PUT uses.partial()on the same shape. Evidence has analogous omit (userId, assetId, chittyId, blockchainHash, verificationStatus, aiAnalysis, deletedAt). Testclient-supplied server-owned fields are strippedproves boundary holds.Transactional side effects
Both POSTs use
db.transaction(async tx => ...). Evidence POST does the ownership SELECT inside the same transaction before insert. DELETE does ownership SELECT, then deletes dependents, then deletes asset — all in one txn — to avoid FK violations.Ownership policy
PUT/DELETE:
WHERE id=? AND user_id=?withRETURNING. Empty result → 404 (not 403, per task spec — no existence leak). Evidence POST: explicit SELECT on parent asset inside txn, 404 if not owned. Re-read confirms row unchanged after intruder PUT/DELETE attempts.Unilateral decision (flagged)
Deferred ChittyID + ChittyTrust on create. Express calls
services.id.generate('asset')andservices.trust.calculate(...)synchronously before insert. Replicating in worker would require HTTP calls to chittyid.chitty.cc + chittytrust.chitty.cc on every POST, which (a) violates the NO MOCKS rule in tests, (b) couples the write path to external service availability, and (c) Phase 2c established a pattern of deferring external-HTTP-on-write side effects. Worker leaveschitty_idNULL andtrust_scoreat schema default'0.0'on insert. Async minter pass will fill these later. Documented in route comments. TestverificationStatusdefaults to'pending'confirms baseline state.Validation evidence
Typecheck
(Pre-existing errors in
server/*.tsare unchanged.)Tests — 47 pass (29 prior + 18 new)
Wrangler dry-run
Real Neon INSERT (Phase 3a contract — NO MOCKS)
Executed against
br-spring-star-aky6u1mcvia Neon MCPrun_sql_transaction:Returned:
id=4da11beb-...,verification_status='pending',chitty_chain_status='draft',trust_score='0.0',chitty_id=NULL— exactly matches the worker contract (schema defaults applied, server-owned fields untouched).Then:
Returned
event_type='acquisition'— matchestimelineEventTypeEnumcanonical value.Validation rows cleaned up.
Canonical (P/L/T/E/A)
// @canon: chittycanon://core/services/chittyassetsheader on all new files.worker/src/env.ts(unchanged). Owner is Person (P), asset/evidence are Thing (T), timeline rows are Event (E). Authority (A) and Location (L) not exercised by these writes but the enum carries them.event_typevalues used ('acquisition','evidence_added') are canonical members oftimelineEventTypeEnumatshared/schema.ts:82.Test plan
npm run check— 0 new worker errors--dry-run --env productionsucceedsbr-spring-star-aky6u1mcassets.chitty.cconce deployed🤖 Generated with Claude Code