refactor(engine): consolidate duplicated error/serialization/attachment helpers

[willwashburn]

Summary

An architecture-focused pass over packages/engine that eliminates genuine duplication and fixes a couple of latent inconsistencies, without churning the already-clean hexagonal structure. Three independently-verified commits.

1. Single source of truth for coded errors

The engine already had a canonical codedError(message, code, status) helper (in lib/httpError.ts), and newer modules used it — but ~45 older call sites still hand-rolled new Error() + Object.assign(err, { code, status }) + throw err, and routing.ts/directory.ts each defined their own duplicate local createError. All consolidated onto the one helper. Behavior-identical; the two a2a.ts sites that attach extra fields (retryable, data) were intentionally left raw.

2. Centralized wire-serialization helpers

• Added lib/serialize.ts with a single toIso, replacing the duplicate copies in delivery.ts and routing.ts.
• Dropped action.ts's private toFleetWireJson, which was missing the Date → ISO branch the canonical deliveryWire.ts version has (a latent serialization bug), in favor of the complete shared implementation.
• Removed an unused ne import (clears the last lint warning — lint is now 0 warnings).

3. Consolidated attachment hydration

delivery.ts, dm.ts, groupDm.ts, and message.ts each carried a verbatim copy of fetchAttachmentsBatch + the AttachmentRow type, and the copies had drifted: delivery.ts lacked the position ordering, and message.ts lacked both the files.workspaceId scope and the ordering. Extracted a single engine/attachments.ts helper that always scopes by workspace and orders by (message_id, position), and pointed all four modules at it — aligning every read path on the safest, most-correct variant. Delivery payloads and channel/thread message lists now return attachments in deterministic position order.

Net: −214 lines of duplicated code, one latent Date-serialization bug fixed, and attachment ordering made deterministic everywhere.

Verification (run after each step)

• turbo build, turbo test (133 unit + conformance), turbo lint (now 0 warnings, was 1) — all green
• Full live e2e against a self-hosted Node+SQLite server: 107 passed, 0 failed
• Manual upload→send→list check confirming attachments serialize with correct fields and position ordering (the read path is not covered by existing tests)

Intentionally out of scope

• Splitting the 700–900-line domain files (action/directory/a2a/node) — cohesive, high-churn/low-value.
• Routes building drizzle queries directly — a pervasive, deliberate style (small membership/lookup reads in routes, business logic in engine/); fixing one site would be inconsistent churn. Noted as a possible follow-up.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WfmtpEM9NFvxEBxtsAAV1S

---

Generated by Claude Code

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 839cc83b-4e23-455e-8046-28f4f218362a

📥 Commits

Reviewing files that changed from the base of the PR and between bb01110 and 9a0f553.

📒 Files selected for processing (18)

• .gitignore
• packages/engine/src/engine/a2a.ts
• packages/engine/src/engine/action.ts
• packages/engine/src/engine/agent.ts
• packages/engine/src/engine/attachments.ts
• packages/engine/src/engine/channel.ts
• packages/engine/src/engine/delivery.ts
• packages/engine/src/engine/directory.ts
• packages/engine/src/engine/dm.ts
• packages/engine/src/engine/dmAll.ts
• packages/engine/src/engine/eventDelivery.ts
• packages/engine/src/engine/groupDm.ts
• packages/engine/src/engine/message.ts
• packages/engine/src/engine/routing.ts
• packages/engine/src/engine/thread.ts
• packages/engine/src/engine/tokenRotate.ts
• packages/engine/src/engine/workspace.ts
• packages/engine/src/lib/serialize.ts

---

📝 Walkthrough

Walkthrough

The PR centralizes two shared utilities: toIso (ISO date serialization) in serialize.ts and fetchAttachmentsBatch/AttachmentRow in a new attachments.ts. Local duplicates of these helpers are removed from delivery.ts, dm.ts, groupDm.ts, message.ts, and action.ts. Separately, all engine modules replace ad-hoc Error construction (with manual code/status assignment) with the existing codedError helper, removing local createError functions from directory.ts and routing.ts. Two .gitignore entries are also added.

Changes

Shared Helper Extraction and Engine-Wide Adoption

Layer / File(s)	Summary
New shared serialization and attachment-batch helpers packages/engine/src/lib/serialize.ts, packages/engine/src/engine/attachments.ts	toIso is added to serialize.ts to render nullable Date values as ISO-8601 strings. attachments.ts is created with the exported AttachmentRow type and fetchAttachmentsBatch function that joins messageAttachments/files, filters by workspace and message IDs, and returns a grouped Map.
Attachment-batch and serialization helper adoption packages/engine/src/engine/delivery.ts, packages/engine/src/engine/dm.ts, packages/engine/src/engine/groupDm.ts, packages/engine/src/engine/message.ts, packages/engine/src/engine/action.ts	Local inline implementations of fetchAttachmentsBatch, toIso, and toFleetWireJson are removed from each module and replaced with imports from the new shared helpers. message.ts and delivery.ts also drop their local AttachmentRow type definitions.
codedError adoption across all engine modules packages/engine/src/engine/a2a.ts, packages/engine/src/engine/agent.ts, packages/engine/src/engine/channel.ts, packages/engine/src/engine/directory.ts, packages/engine/src/engine/dm.ts, packages/engine/src/engine/dmAll.ts, packages/engine/src/engine/eventDelivery.ts, packages/engine/src/engine/groupDm.ts, packages/engine/src/engine/routing.ts, packages/engine/src/engine/thread.ts, packages/engine/src/engine/tokenRotate.ts, packages/engine/src/engine/workspace.ts	All engine modules replace manually constructed Error objects with codedError(message, code, status). Local createError helper functions in directory.ts and routing.ts are deleted. Error messages, codes, and HTTP statuses are preserved.
.gitignore additions .gitignore	Adds ignore entries for relaycast-files/ and *.db.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 Hopping through the code with glee,
No more errors built by hand — codedError sets them free!
Attachments batched in one cozy den,
toIso ticks the clock, again, again.
The warren's tidy, helpers shared,
A cleaner engine — the rabbit cared. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.17% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: consolidating duplicated error, serialization, and attachment helpers across the engine package.
Description check	✅ Passed	The description is directly related to the changeset, providing detailed context about the three refactoring efforts (error consolidation, serialization helpers, attachment hydration) with verification results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/architecture-refactor-lahm99

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: one or more packages not found in the registry.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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