refactor(hutch): remove the inbox surface now served by the inbox deployable (phase D: cleanup) by FagnerMartinsBrack · Pull Request #952 · Readplace/readplace.com

FagnerMartinsBrack · 2026-07-10T10:47:55Z

Supersedes #951 (auto-closed when its stacked base merged as #950).

Stacked on #950 — closes the double-delivery overlap window that PR opens. Do not merge until #950 is deployed green in both stages and the operator checklist below is clear. (After #950 merges, retarget this PR's base to main.)

Removes from hutch: /inbox pages, inbox domain handlers, the three pipeline mains, and their entire infra chains (queues, lambdas, SNS→SQS bridge, EventBridge rules, truncation-alert trio — 251 lines of infra/index.ts). The composition roots collapse five inbox deps into one injected provisionInboxAddress; signup-provisioning tests re-home to web/auth/.

Hutch keeps: SES receiving (InboxMail), the three DynamoDB tables, delete-account's purge reach, signup-time address provisioning (addresses grant + INBOX_ADDRESS_DOMAIN stay on the main lambda; emails/links grants+env removed), and all Pulumi config keys.

⚠️ Operator checklist BEFORE merging

feat(inbox): extract the inbox into its own deployable (phases A–C: pre-refactors, seam, additive project) #950 deployed green: staging + prod, /inbox serving from inbox-web-handler, new pipeline lambdas consuming.
Old DLQs empty: receive-email-dlq, extract-email-links-dlq, crawl-email-link-preview-dlq (deleting a queue destroys its messages — redrive first if non-empty).
Old source queues ~0 depth (they self-drain; anything lost at delete instant is a duplicate the inbox path already processed).
Export extract-email-links-truncated-alert if the 14-day truncation audit trail matters (advisory only).

After deploy

Single delivery restored: the SNS topic has one SQS subscription; the platform bus carries only inbox-* rules for these events. Rollback = revert this PR (recreates hutch's consumers; re-entering the overlap is safe by idempotency; any dropped notification is replayable from the raw-email bucket, which retains every .eml forever).

Full pnpm check green at every commit (41 projects).

FagnerMartinsBrack · 2026-07-10T10:56:22Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: refactor(hutch): remove the inbox surface now served by the inbox deployable (phase D: cleanup) #952
Commit: 464ff8b

claude · 2026-07-10T10:56:58Z

Claude finished @FagnerMartinsBrack's task in 5m 2s —— View job

Review complete — PR #952 (inbox surface removal, phase D cleanup)

Read review instructions (claude-PR-code-reviewer.md)
Gather PR diff and context
Analyze retained vs removed code (app.ts, server.ts, infra, composition roots)
Check for dangling references to deleted inbox modules
Verify against CLAUDE.md guidelines
Post review

Verdict: clean. No high or medium priority issues.

What I verified:

No dangling references — hutch has zero remaining receive-email / extract-email-links / crawl-email-link-preview / initInboxRoutes references; the projects/inbox/* hits are the new deployable.
Retained code is correct — InboxMail SES + its exported notificationTopicArn (infra/index.ts:1306), the three DynamoDB tables + Pulumi config keys, delete-account purge reach (delete-account.main.ts:83–91 reads the emails/links tables via the delete-account lambda env at infra/index.ts:602–603), and signup-time address provisioning all match the PR's "hutch keeps" list.
No unused imports left behind — initS3ReadContent / contentBucketName are still used at app.ts:195; the removed infra imagesCdnBaseUrl has no lingering reference.
The 263-line infra removal is self-contained and CI is green on this commit (run 29087446720, success).

One LOW observation (optional): the provisionInboxAddress closure is now duplicated verbatim across three composition roots (app.ts prod, app.ts dev, test-app.ts). Reasonable trade-off for the narrower server.ts seam, but extractable. Details in the formal review comment.

The formal review block is emitted to this run's output; the automation posts it as the canonical review comment.

FagnerMartinsBrack · 2026-07-10T11:02:21Z

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. `provisionInboxAddress` closure is duplicated across three composition roots

The seam introduced here — replacing five injected inbox deps with a single provisionInboxAddress: (userId: UserId) => Promise<void> — is a clean narrowing of AppDependencies, and server.ts no longer needs to know about InboxAddressStore, inboxAddressDomain, or DEFAULT_INBOX_ALIAS. That's a good change.

The cost is that the identical provisioning body now appears verbatim in three places:

projects/hutch/src/runtime/app.ts:346 (prod root)
projects/hutch/src/runtime/app.ts:609 (dev root)
projects/hutch/src/runtime/test-app.ts:177 (test root)

Each builds the same createAddress({ userId, domain: inboxAddressDomain, name: DEFAULT_INBOX_ALIAS }) call. Previously this decision lived once inside provisionInboxAddressOnSignup. Some wiring duplication is idiomatic for composition roots (each root constructs its own store — DynamoDB vs in-memory), so this is a judgment call rather than a defect. If you'd like to keep the alias/domain/shape decision in one place, a small shared helper — e.g. initProvisionInboxAddress({ inboxAddressStore, inboxAddressDomain }) returning the closure — would collapse the three copies while preserving the seam. Optional.

FagnerMartinsBrack · 2026-07-10T11:32:20Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: refactor(hutch): remove the inbox surface now served by the inbox deployable (phase D: cleanup) #952
Branch: inbox-split-cleanup

claude · 2026-07-10T11:33:00Z

Claude encountered an error —— View job

Resolving merge conflicts — PR #952

Read conflict-fixer instructions
Read PR commit history to understand intent
Rebase onto origin/main
Resolve any conflicts (cross-referencing intent)
Run pnpm check to verify
Push with --force-with-lease

claude · 2026-07-10T11:34:58Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:35:50Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

claude · 2026-07-10T11:36:37Z

Claude encountered an error —— View job

I'll analyze this and get back to you.

FagnerMartinsBrack · 2026-07-10T15:14:17Z

@claude Resolve conflicts. Use your Read tool to read the local file .github/workflows/claude-PR-conflict-fixer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: refactor(hutch): remove the inbox surface now served by the inbox deployable (phase D: cleanup) #952
Branch: inbox-split-cleanup

claude · 2026-07-10T15:14:57Z

Claude finished @FagnerMartinsBrack's task in 15m 26s —— View job

Conflicts resolved — PR #952 rebased onto `main` ✅

Read conflict-fixer instructions
Read PR commit history to understand intent
Rebase onto origin/main
Resolve conflicts (cross-referencing intent)
pnpm check green (42 projects, all tasks)
Push with --force-with-lease

Rebased inbox-split-cleanup from 464ff8b0 onto current main (78dfa44b) → new tip cfd99d24. Two conflicts, both from main moving under the PR:

1. projects/hutch/src/runtime/test-app.ts (content)
main added an analytics middleware import; this PR replaced the inbox deps with a single provisionInboxAddress closure. Both edits land in the same import block. Combined them — kept all three symbols, each used in the merged body:

import { type AnalyticsEvent, createAnalyticsMiddleware } from "@packages/web-analytics"; // main → used at createAnalyticsMiddleware(...)
import { DEFAULT_INBOX_ALIAS } from "@packages/domain/inbox";                              // PR → used in the provisionInboxAddress closure
import type { UserId } from "@packages/domain/user";                                       // PR → closure param type

2. web/pages/inbox/inbox.route.test.ts (modify/delete)
This PR deletes the entire /inbox surface; main had edited this file to drop the now-removed confirmPassword field from its two /signup tests. Kept the PR's deletion (git rm), and — because the PR re-homed those exact signup tests to web/auth/signup-inbox-provisioning.route.test.ts — carried main's change forward by removing confirmPassword from both POST bodies in the new file. This preserves both intents rather than reverting main's signup cleanup (the reset-password flow's confirmPassword is untouched).

One environment snag, not a code issue: main added "@packages/web-shell": "workspace:*" to the two extension package.jsons (+ lockfile). After the rebase the pre-rebase node_modules was missing that symlink, so chrome/firefox-extension:compile failed on Cannot find module '@packages/web-shell'. pnpm install created the symlink; the re-run is fully green (extensions at 100% coverage).

No PR logic was changed — resolution was confined to the conflicting import block, the deleted-file handling, and re-homing main's signup edit.
· branch inbox-split-cleanup

…loyable Closes the double-delivery overlap the inbox extraction opened: the /inbox pages, inbox domain handlers, the three email-pipeline mains, and their infra chains (queues, lambdas, SNS bridge, EventBridge rules, truncation alert trio) leave hutch; the inbox stack's inbox-prefixed equivalents are the sole consumers from this deploy on. Hutch keeps everything stateful and every cross-cutting concern: the SES receiving stack, the three DynamoDB tables, delete-account's purge reach into them, and signup-time address provisioning — the five inbox deps on the composition roots collapse into one injected provisionInboxAddress, and the provisioning tests re-home to web/auth/ where the behavior lives. The emails/links table grants and env leave the main web lambda; the addresses grant stays for signup. Merge only after the inbox deployable's first deploy is green in both stages, with the old DLQs drained (see the cutover run-book in the PR).

FagnerMartinsBrack · 2026-07-10T15:37:59Z

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

Repo: Readplace/readplace.com
PR: refactor(hutch): remove the inbox surface now served by the inbox deployable (phase D: cleanup) #952
Commit: cfd99d2

claude · 2026-07-10T15:38:37Z

Claude finished @FagnerMartinsBrack's task in 3m 54s —— View job

Review complete — PR #952 (inbox surface removal, phase D cleanup) · re-review of rebased tip `cfd99d24`

Read review instructions (claude-PR-code-reviewer.md)
Gather PR diff and context (base origin/main)
Analyze retained vs removed code (app.ts, server.ts, infra, composition roots)
Check for dangling references to deleted inbox modules
Verify against CLAUDE.md guidelines
Post review

Verdict: clean. No high or medium priority issues. The rebase onto main (cfd99d24) is sound.

What I verified on this commit:

No dangling references — git grep on HEAD finds zero remaining imports of the deleted inbox web pages, domain handlers (receive-email / extract-email-links / crawl-email-link-preview / store-email-body / email-content-id), pipeline mains, or the deleted s3-put-image-object provider. The only surviving InboxEmail* references are in the delete-account purge path (delete-account-handler.ts, delete-account.main.ts), which the PR explicitly retains.
Image-provider deletion is self-contained — hutch's deleted providers/article-store/s3-put-image-object.ts had exactly one importer in origin/main: hutch's crawl-email-link-preview.main.ts, deleted in the same PR. projects/inbox and projects/save-link keep their own separate copies, unaffected.
Retained code intact — InboxMail SES + inboxNotificationTopicArn export (infra/index.ts:1306), all three DynamoDB tables (inboxAddressesTable / inboxEmailsTable / inboxEmailLinksTable still in storage, with emails/links env vars kept on the delete-account lambda at infra:602–604), and signup-time address provisioning all match the "hutch keeps" list.
The provisionInboxAddress seam is correct — server.ts drops InboxAddressStore / InboxEmailStore / InboxEmailLinkStore / ContentProvider / DEFAULT_INBOX_ALIAS / initInboxRoutes, while UserId (closure param) and ReadArticleContent (server.ts:288) remain used. Both test overrides in the new test read their targets at call time, so they take effect.
Rebase conflict resolution is clean — all four merged imports in test-app.ts (AnalyticsEvent, createAnalyticsMiddleware, DEFAULT_INBOX_ALIAS, UserId) are used; the re-homed signup tests drop confirmPassword consistent with main's signup cleanup.

One LOW observation carries over from the prior review (unchanged by the rebase): the provisionInboxAddress closure is duplicated verbatim across the three composition roots. Optional; details in the formal review block.

The formal review block is emitted to this run's output; the automation posts it as the canonical review comment.
· branch inbox-split-cleanup

FagnerMartinsBrack · 2026-07-10T15:42:52Z

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. `provisionInboxAddress` closure is duplicated across three composition roots

The seam introduced here — replacing five injected inbox deps with a single provisionInboxAddress: (userId: UserId) => Promise<void> — is a clean narrowing of AppDependencies. After this change server.ts no longer imports InboxAddressStore, InboxEmailStore, InboxEmailLinkStore, ContentProvider, DEFAULT_INBOX_ALIAS, or initInboxRoutes, and the retained UserId / ReadArticleContent types are both still used. Good change.

The cost is that the identical provisioning body now appears verbatim in three places:

projects/hutch/src/runtime/app.ts:346 (prod root)
projects/hutch/src/runtime/app.ts:609 (dev root)
projects/hutch/src/runtime/test-app.ts:177 (test root)

Each builds the same createAddress({ userId, domain: inboxAddressDomain, name: DEFAULT_INBOX_ALIAS }) call, where a single provisionInboxAddressOnSignup used to hold that decision. Some wiring duplication is idiomatic for composition roots (each root already constructs its own store — DynamoDB vs in-memory), so this is a judgment call rather than a defect. If you'd like the alias/domain/shape decision in one place, a small shared helper — e.g. initProvisionInboxAddress({ inboxAddressStore, inboxAddressDomain }) returning the closure — would collapse the three copies while preserving the seam. This is unchanged from the pre-rebase review and remains optional. Fix this →

FagnerMartinsBrack temporarily deployed to staging July 10, 2026 10:48 — with GitHub Actions Inactive

FagnerMartinsBrack force-pushed the inbox-split-cleanup branch from 464ff8b to cfd99d2 Compare July 10, 2026 15:29

FagnerMartinsBrack temporarily deployed to staging July 10, 2026 15:29 — with GitHub Actions Inactive

Uh oh!

Conversation

FagnerMartinsBrack commented Jul 10, 2026

Contents

⚠️ Operator checklist BEFORE merging

After deploy

Uh oh!

FagnerMartinsBrack commented Jul 10, 2026

Context

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review complete — PR #952 (inbox surface removal, phase D cleanup)

Uh oh!

FagnerMartinsBrack commented Jul 10, 2026

High Priority Issues (Must Fix)

Medium Priority Issues

Low Priority Suggestions

1. provisionInboxAddress closure is duplicated across three composition roots

Uh oh!

FagnerMartinsBrack commented Jul 10, 2026

Context

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resolving merge conflicts — PR #952

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FagnerMartinsBrack commented Jul 10, 2026

Context

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts resolved — PR #952 rebased onto main ✅

Uh oh!

FagnerMartinsBrack commented Jul 10, 2026

Context

Uh oh!

claude Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review complete — PR #952 (inbox surface removal, phase D cleanup) · re-review of rebased tip cfd99d24

Uh oh!

FagnerMartinsBrack commented Jul 10, 2026

High Priority Issues (Must Fix)

Medium Priority Issues

Low Priority Suggestions

1. provisionInboxAddress closure is duplicated across three composition roots

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jul 10, 2026 •

edited

Loading

1. `provisionInboxAddress` closure is duplicated across three composition roots

claude Bot commented Jul 10, 2026 •

edited

Loading

claude Bot commented Jul 10, 2026 •

edited

Loading

claude Bot commented Jul 10, 2026 •

edited

Loading

claude Bot commented Jul 10, 2026 •

edited

Loading

claude Bot commented Jul 10, 2026 •

edited

Loading

Conflicts resolved — PR #952 rebased onto `main` ✅

claude Bot commented Jul 10, 2026 •

edited

Loading

Review complete — PR #952 (inbox surface removal, phase D cleanup) · re-review of rebased tip `cfd99d24`

1. `provisionInboxAddress` closure is duplicated across three composition roots