feat(engine): module substrate wave 3 — assistant auth, drafts API, draft-approval (HT-70) - #80
Merged
Merged
Conversation
…raft-approval orchestration (HT-70) Adds the per-Assistant bearer-token credential class (ht_asst_<id>_<secret>, constant-time verification alongside the service Bearer token), the Assistants admin API, the drafts API (create/list/approve/discard), and the draft-approval orchestration that mints a reply token for the draft's existing thread, derives the envelope exactly as sendReply does, applies HT-32 pixel injection when configured, and hands off to the unchanged delivery path. Wires author-identity forward-carry into replies/notes and emits draft.created/draft.resolved from the wave-1 event outbox inside the appendDraft/resolveDraft transactions. A wire-level equivalence fixture (src/mail/draft-equivalence.test.ts) proves assistant-draft-then-approve produces byte-identical RFC 5322 output to sendReply, modulo the Message-ID token, in both pixel-on and pixel-off configs — the mail-semantics acceptance bar for this change (CHARTER.md invariant #5). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- resolveDraft's approve branch reopens a closed conversation to active in the same transaction (spec §6's reply-reopen invariant; pending stays pending, spam unreachable via the API's 409). Tests: closed reopens, active untouched, pending stays. - Reply handler rejects caller Idempotency-Keys spelled draft:* (400), closing the cross-namespace collision; store comment now states the actual guarantee. Test for the 400. Gates re-verified on this tree: typecheck 0, lint 0, test 0 (1208/1208). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… (HT-70 review) - resolveDraft's approve branch now locks the parent conversation row (FOR UPDATE OF) inside the same transaction as the thread write, and is the AUTHORITATIVE check: 'deleted'/'spam' refuse outright (draft row untouched, returned as a typed sentinel the API maps to its existing 404/409 shapes), 'closed' still reopens. The API's own preflight read is now documented as a fast path only, since a concurrent delete/spam-mark between that read and the write could otherwise let mail be armed from a conversation an Agent already pulled out of the deliverable set. Tests simulate the race with a stale pre-mutation snapshot at both the approveDraft and full-API layers, plus store-level tests for the two new refusals. - approve-with-edits' bodyText override now gets the same 1-5000 length bound parseDraftBody/parseReplyBody already enforce (bodyHtml stays type-only, matching those same two paths). Tests: empty string 400, oversize 400, boundary 5000 accepted. Gates on this tree: typecheck 0, lint 0; targeted suites (conversations, approve-draft, drafts, draft-equivalence, send) 162/162. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… (HT-70) Rebased onto main (waves 1+2 merged): union-merged the add-add conflicts in router/index/composition (function-ownership kept conversations.ts conflict-free), and each wave's test fixtures gain the OTHER wave's now- required InboxApiDeps member (dummy, never-invoked posture matching index.test.ts's precedent). Comment paths follow the Modules vocabulary. Gates on the integrated tree: typecheck 0, lint 0, test 0 (1339/1339). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR adds Assistant token authentication and admin APIs, introduces draft creation and review workflows, updates conversation/thread contracts for authorship and draft status, and makes draft persistence, approval, delivery, events, previews, and counts transactional and lifecycle-aware. ChangesAssistant credentials and administration
Routing and API integration
Conversation and mail contracts
Draft persistence and delivery
Validation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/assistant-auth.ts`:
- Around line 45-49: Update the assistant authentication flow in createInboxApi
to catch failures from the assistant-store awaits, including store.get and
store.getTokenHash, and convert them into the API’s controlled server-error
response. Preserve the existing null return for missing or inactive assistants
and ensure authentication never rejects under the function’s “Never throws”
contract.
- Around line 45-52: Update the authentication flow around assistant lookup and
token validation to fetch the Assistant’s active status and token hash
atomically in a single store query. Replace the separate store.get and
store.getTokenHash calls while preserving rejection of missing or inactive
Assistants, then compare the provided hash against the hash returned by that
combined query.
In `@src/api/assistants.test.ts`:
- Line 10: Fix the Biome formatting violations in assistants.test.ts by
organizing the Vitest imports according to repository conventions and applying
the formatter to the webhooks fixture layout around the referenced fixture
section. Make only formatting and import-order changes.
In `@src/api/drafts.test.ts`:
- Line 92: Reformat the inline webhooks object in the test fixture to Biome’s
multi-line style, including its store and queue properties while preserving the
WebhooksApiDeps satisfies constraint. Apply the same formatting change to the
webhooks object inside racyApi at the other occurrence.
- Around line 25-26: Reorder the imports in drafts.test.ts so the createInboxApi
import from ./index.js precedes the type-only WebhooksApiDeps import from
./webhooks.js, matching Biome organizeImports ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a9ed2abf-f280-436a-9981-bb725b2481f8
📒 Files selected for processing (28)
specs/api/agent-inbox-v1.mdsrc/api/agents.test.tssrc/api/assistant-auth.test.tssrc/api/assistant-auth.tssrc/api/assistants.test.tssrc/api/assistants.tssrc/api/conversations.tssrc/api/cursor.tssrc/api/drafts.test.tssrc/api/drafts.tssrc/api/index.test.tssrc/api/index.tssrc/api/router.test.tssrc/api/router.tssrc/api/webhooks.test.tssrc/auth/assistant-token.test.tssrc/auth/assistant-token.tssrc/composition/root.tssrc/mail/approve-draft.test.tssrc/mail/approve-draft.tssrc/mail/draft-equivalence.test.tssrc/mail/reply-headers.test.tssrc/mail/reply-headers.tssrc/mail/send.tssrc/store/assistants.test.tssrc/store/assistants.tssrc/store/conversations.test.tssrc/store/conversations.ts
Comment on lines
+45
to
+49
| const assistant = await store.get(parsed.assistantId) | ||
| if (assistant === null || assistant.status !== 'active') return null | ||
|
|
||
| const tokenHash = await store.getTokenHash(parsed.assistantId) | ||
| if (tokenHash === null) return null |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Contain Assistant-store failures.
These awaits can reject. createInboxApi performs Assistant authentication before its response-shaping try, so a database failure escapes as an uncontrolled 500 despite this function’s “Never throws” contract. Handle the failure at the API boundary as a controlled server error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/assistant-auth.ts` around lines 45 - 49, Update the assistant
authentication flow in createInboxApi to catch failures from the assistant-store
awaits, including store.get and store.getTokenHash, and convert them into the
API’s controlled server-error response. Preserve the existing null return for
missing or inactive assistants and ensure authentication never rejects under the
function’s “Never throws” contract.
…iome fixes (HT-70 review) CodeRabbit round 1 on #80 (5 actionables): - getForAuth replaces getTokenHash: status + token_hash from ONE row read, so disable/rotation can never interleave between separate queries and validate stale credentials - assistant auth's pre-try await is contained at the call site — a store failure returns the controlled server_error envelope (with no-store), never an uncontrolled host-runtime 500; new end-to-end test proves it - biome import-order/format violations from the rebase integration patch fixed (the Quality gate failure; local lint had been misread through a piped exit code — re-verified bare) Gates: typecheck 0, lint 0, test 0 (1340/1340). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zaridan
added a commit
that referenced
this pull request
Jul 19, 2026
Adds docs/modules/{README,webhooks,assistants-and-drafts}.md, written and
verified against the shipped code rather than the spec alone: curl examples
cross-checked against src/api/router.ts's actual patterns and validation
(src/api/webhooks.ts, src/store/webhook-endpoints.ts), and the signature
sample verified byte-for-byte against both the engine's signer
(src/webhooks/delivery.ts) and the reference module's verifier
(module-draft-assistant/src/verify.ts) via a throwaway script (exit 0).
Assistants/drafts sections document PR #80 (HT-70), not yet on main —
flagged at the top of assistants-and-drafts.md; merge this after #80.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zaridan
added a commit
that referenced
this pull request
Jul 19, 2026
- assistants-and-drafts.md: drop the pre-merge branch/PR-#80 status note now that #80 is merged and deployed; content documents merged main. - assistants-and-drafts.md: stop claiming every Assistant write is a draft — notes (POST .../notes) are direct writes with no draft/ approval step, only customer-facing replies are draft-gated. Verified against handlePostNote (src/api/conversations.ts on merged main). - README.md: "draft-writing Assistant", not "draft-writing AI" — vocabulary rule (Assistants, never generic "AI"). - webhooks.md: the signature-verification sample now requires an exact 64-hex-char match before Buffer.from(..., 'hex') decoding — that call otherwise silently truncates at the first non-hex character instead of rejecting trailing garbage after a valid-length prefix. Re-verified with an extended throwaway script (adds the trailing-garbage negative case to the existing engine-signer/reference-module cross-check); exit 0. - webhooks.md: split the delivery-guarantees table's "anything else retries" row — an SsrfRefusedError dead-letters immediately on the first attempt and is never retried, unlike an ordinary HTTP failure/ timeout/connection error. Verified against src/webhooks/delivery.ts's catch branch on merged main. Rebased onto origin/main (PR #80 merged as 475b60e) so both the docs and the verification below reflect merged main, not the pre-merge branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zaridan
added a commit
that referenced
this pull request
Jul 19, 2026
* docs(modules): operator guide for the module substrate (HT-71)
Adds docs/modules/{README,webhooks,assistants-and-drafts}.md, written and
verified against the shipped code rather than the spec alone: curl examples
cross-checked against src/api/router.ts's actual patterns and validation
(src/api/webhooks.ts, src/store/webhook-endpoints.ts), and the signature
sample verified byte-for-byte against both the engine's signer
(src/webhooks/delivery.ts) and the reference module's verifier
(module-draft-assistant/src/verify.ts) via a throwaway script (exit 0).
Assistants/drafts sections document PR #80 (HT-70), not yet on main —
flagged at the top of assistants-and-drafts.md; merge this after #80.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs(modules): address CodeRabbit review on PR #81
- assistants-and-drafts.md: drop the pre-merge branch/PR-#80 status note
now that #80 is merged and deployed; content documents merged main.
- assistants-and-drafts.md: stop claiming every Assistant write is a
draft — notes (POST .../notes) are direct writes with no draft/
approval step, only customer-facing replies are draft-gated. Verified
against handlePostNote (src/api/conversations.ts on merged main).
- README.md: "draft-writing Assistant", not "draft-writing AI" — vocabulary
rule (Assistants, never generic "AI").
- webhooks.md: the signature-verification sample now requires an exact
64-hex-char match before Buffer.from(..., 'hex') decoding — that call
otherwise silently truncates at the first non-hex character instead of
rejecting trailing garbage after a valid-length prefix. Re-verified with
an extended throwaway script (adds the trailing-garbage negative case
to the existing engine-signer/reference-module cross-check); exit 0.
- webhooks.md: split the delivery-guarantees table's "anything else
retries" row — an SsrfRefusedError dead-letters immediately on the
first attempt and is never retried, unlike an ordinary HTTP failure/
timeout/connection error. Verified against src/webhooks/delivery.ts's
catch branch on merged main.
Rebased onto origin/main (PR #80 merged as 475b60e) so both the docs and
the verification below reflect merged main, not the pre-merge branch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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
Wave 3 — the final substrate wave (HT-70; spec §3/§6/§7): assistant principals, the drafts API, and the approval path. Rebased onto main with waves 1+2 in.
ht_asst_<id>_<secret>tokens (SHA-256-hashed at rest — sound for server-generated 256-bit secrets; reviewer-endorsed), constant-time verification before routing, admin CRUD + rotate-token (shown once), fixed capability set enforced at ONE gate: read conversations, create drafts, create notes — nothing else, with soft-deleted indistinguishability on every assistant path.draft:-scoped Idempotency-Key; reply handler now rejectsdraft:*caller keys closing the cross-namespace hole), review queue (keyset, soft-deleted excluded), approve-with-edits (audited; body validated identically to the reply path), discard.reply-headers.tsextraction (sendReply behavior provably identical — its tests unmodified), HT-32 pixel semantics, conversation locked and status re-checked INSIDE the resolve transaction (closed → reopens per the reply rule; deleted/spam → refused even under race), then the unchanged delivery worker. Fixture-asserted: an unedited approved draft's RFC 5322 wire output is byte-identical tosendReply's (both pixel configs; only the Message-ID token and the per-call MIME boundary normalized).draft.created/draft.resolvedemitted inside the store transactions; author identity now recorded on replies/notes/resolutions via the acting-agent header (nullable degrade).agent-inbox-v1.mdland here: ThreadViewauthorKind/draftStatus, widened deliveryStatus invariant, preview/threadCount draft exclusions.Review trail (three independent layers)
Gates
typecheck 0 · lint 0 · test 0 — 1339 passed (full integrated suite, waves 1+2+3).
Note for reviewer: after merge this needs a MANUAL
vercel deploy --prod— the project's git auto-deploy is not firing (observed on #77 and #79; see session notes). No new migrations (all DDL shipped in #77, already on prod).🤖 Generated with Claude Code
Summary by CodeRabbit