Skip to content

fix: define transactional outbox store contract - #1130

Merged
kang-heewon merged 8 commits into
trunkfrom
fix/outbox-core-store
Jun 30, 2026
Merged

fix: define transactional outbox store contract#1130
kang-heewon merged 8 commits into
trunkfrom
fix/outbox-core-store

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Jun 30, 2026

Copy link
Copy Markdown
Member

Fixes #1092

Summary

  • Defines @croco/outbox-core with the provider-neutral TransactionalOutboxStore contract, outbox record/claim/failure types, and Unit of Work context boundary.
  • Adds an in-memory fixture and conformance suite covering idempotency, tenant isolation, UoW commit/rollback/context validation, leases, stale completion, retry metadata, retry exhaustion, and terminal failures.
  • Registers package catalog/docs, problem codes, public API snapshot, API-doc trigger, lockfile, and changeset.

Verification

  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/outbox-core test
  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/outbox-core typecheck
  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/outbox-core build
  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/docs docs:build
  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm check
  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm typecheck
  • COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm changeset-required:check -- --base origin/trunk --head HEAD
  • git diff --check and git diff --cached --check

Local full pnpm test is blocked in this worktree by @croco/tx-drizzle RealDb tests failing to load a local better-sqlite3 native binding under the current Node/toolchain; focused outbox tests pass.

Summary by CodeRabbit

  • New Features
    • @croco/outbox-core 패키지가 추가되어 트랜잭셔널 아웃박스 저장소의 provider-neutral 계약과 메모리 기반 구현을 제공합니다.
    • 레코드/클레임/디스패치/실패 및 관련 Problem 코드(디스패치 실패, 레코드 ID 충돌, UoW 컨텍스트 오류 등) 지원
    • 계약 적합성 테스트 스위트 제공
  • Documentation
    • outbox-core API 문서 및 문제 코드 안내, 패키지 카탈로그/가이드 수치가 새 패키지 기준으로 업데이트
  • Tests
    • 트랜잭션 아웃박스 계약 테스트 및 실행 구성이 추가되었습니다.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: fc776a68-32f7-495e-a038-0e355f58c38e

📥 Commits

Reviewing files that changed from the base of the PR and between 61c39a3 and 196ea65.

⛔ Files ignored due to path filters (2)
  • packages/problems-core/src/generated/problem-code-registry.ts is excluded by !**/generated/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • docs/problem-code-registry.json
  • packages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.md
  • packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
  • packages/outbox-core/src/libs/conformance.ts
  • packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts

📝 Walkthrough

Walkthrough

@croco/outbox-core 패키지가 추가된다. provider-neutral TransactionalOutboxStore 계약, outbox 문제 타입과 메타데이터 헬퍼, 인메모리 구현체, conformance 테스트, 패키지 설정, 공개 API 스냅샷, problem 레지스트리, 관련 문서와 카탈로그가 함께 갱신된다.

Changes

@croco/outbox-core 패키지

Layer / File(s) Summary
저장소 계약 타입 정의
packages/outbox-core/src/libs/types.ts, packages/docs/src/content/docs/api/outbox-core/...
outbox 레코드, 클레임, 재시도, 실패, 테넌트, 트레이스, 저장소 컨텍스트와 TransactionalOutboxStore 인터페이스 및 배치 옵션 타입이 추가되고 관련 API 문서가 생성된다.
Outbox 문제 타입과 메타데이터 헬퍼
packages/outbox-core/src/libs/problems/OutboxProblems.ts, packages/docs/src/content/docs/api/outbox-core/..., packages/docs/src/content/docs/api/problems-core/src/classes/Problem.md
outbox 문제 코드 상수, 문제 클래스, 실패 메타데이터 확장 생성/파싱 헬퍼가 추가되고 관련 문제 문서와 Problem 문서가 갱신된다.
인메모리 저장소 구현
packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts, packages/docs/src/content/docs/api/outbox-core/src/classes/InMemoryTransactionalOutboxStore.md
클론/정규화 유틸, 유닛 오브 워크 상태 관리, 레코드 생성, 클레임, 디스패치/실패 전이, 조회, 상태 판정 로직이 InMemoryTransactionalOutboxStore에 구현되고 API 문서가 추가된다.
컨트랙트 수트와 테스트
packages/outbox-core/src/libs/conformance.ts, packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts, packages/docs/src/content/docs/api/outbox-core/src/functions/createTransactionalOutboxStoreContractSuite.md, packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContract*.md
TransactionalOutboxStore 계약 케이스/옵션/스위트 타입과 동작 검증 시나리오, 그리고 인메모리 저장소를 이용한 테스트 실행이 추가되고 관련 문서가 생성된다.
패키지 배럴과 빌드 설정
packages/outbox-core/src/index.ts, packages/outbox-core/package.json, packages/outbox-core/tsconfig.json, packages/outbox-core/vitest.config.ts, .github/workflows/ci.yml, .changeset/outbox-core-contract.md
outbox-core 패키지 엔트리포인트, TypeScript/Vitest 설정, changeset, 그리고 CI의 API 소스 필터가 추가된다.
공개 API와 problem 레지스트리 갱신
public-api-surface.snapshot.json, docs/problem-code-registry.json, scripts/problem-registry.mts, packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
공개 API 스냅샷과 problem 코드 레지스트리에 @croco/outbox-core의 런타임/타입 심볼과 신규 문제 코드 4종이 등록되고, problem recovery cookbook이 갱신된다.
문서와 카탈로그 업데이트
packages/docs/src/content/docs/en/..., packages/docs/astro.config.mjs, packages/docs/tsconfig.typedoc.json, packages/docs/package.json, docs/package-catalog.json, docs/package-docs-report.md, README.md, packages/outbox-core/README.md
outbox-core 타입/클래스/함수 문서, 패키지 카탈로그 수치, 가이드, Typedoc 입력, README가 갱신된다.

추정 코드 리뷰 노력

🎯 5 (Critical) | ⏱️ ~90+ minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 트랜잭셔널 outbox 저장소 계약 정의라는 핵심 변경을 간결하게 요약합니다.
Linked Issues check ✅ Passed 구현과 테스트가 계약 API, UoW 롤백, 안전한 클레임, 문제 기반 실패 처리, 요구된 계약 테스트를 모두 충족합니다.
Out of Scope Changes check ✅ Passed 문서, 변경집, 스냅샷, 카탈로그, API 생성 설정 변경은 모두 새 outbox-core 계약과 이를 지원하는 범위 안에 있습니다.
✨ 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 fix/outbox-core-store

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.

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

📊 Benchmark Results

❌ Some benchmarks failed

Gate failures

  • CrocoApp constructor: p75 8.4μs exceeds baseline 3.4μs by more than 20%
  • CrocoApp lambdaHandler (10 controllers): p75 298.5μs exceeds baseline 33.3μs by more than 20%
  • Lambda cold-start simulation: p75 418.4μs exceeds baseline 70.2μs by more than 20%
  • Lambda cold-start with headers: p75 375.3μs exceeds baseline 66.7μs by more than 20%
  • Lambda cold-start with binary body: p75 341.9μs exceeds baseline 63.2μs by more than 20%
  • Lambda cold-start with query params: p75 304.4μs exceeds baseline 63.9μs by more than 20%
  • Lambda cold-start with authorizer context: p75 304.1μs exceeds baseline 59.8μs by more than 20%
  • Lambda cold-start realistic scenario: p75 303.4μs exceeds baseline 60.2μs by more than 20%
  • EventBusConfig.start (10 handlers): p75 1.4μs exceeds baseline 0.9μs by more than 20%
  • EventPublisher.publishNow single event: p75 1.7μs exceeds baseline 1.1μs by more than 20%
  • DefaultHandlerResolver.resolve × 10: p75 0.1μs exceeds baseline 0.0μs by more than 20%
  • Container.get singleton (cold): p75 69.8μs exceeds baseline 0.6μs by more than 20%
  • Container.register × 50 components: p75 3.2ms exceeds baseline 12.1μs by more than 20%
  • Container.validate (50 components): p75 3.5ms exceeds baseline 29.7μs by more than 20%
  • Container.get singleton (warm): p75 1.7μs exceeds baseline 0.3μs by more than 20%
  • lambdaPreset config creation: p75 1.4μs exceeds baseline 1.0μs by more than 20%
Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 8.4μs 30.0ms 3.4μs +148.8% -
CrocoApp lambdaHandler (10 controllers) 298.5μs 50.0ms 33.3μs +796.5% -
Lambda cold-start simulation 418.4μs 80.0ms 70.2μs +496.3% -
Lambda cold-start with headers 375.3μs 80.0ms 66.7μs +462.3% -
Lambda cold-start with binary body 341.9μs 80.0ms 63.2μs +441.3% -
Lambda cold-start with query params 304.4μs 80.0ms 63.9μs +376.6% -
Lambda cold-start with authorizer context 304.1μs 80.0ms 59.8μs +408.6% -
Lambda cold-start realistic scenario 303.4μs 80.0ms 60.2μs +404.2% -
EventBusConfig.start (10 handlers) 1.4μs 10.0ms 0.9μs +63.8% -
EventPublisher.publishNow single event 1.7μs 2.0ms 1.1μs +54.5% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.0μs +69.0% -
Container.get singleton (cold) 69.8μs 5.0ms 0.6μs +11074.4% -
Container.register × 50 components 3.2ms 10.0ms 12.1μs +26441.7% -
Container.validate (50 components) 3.5ms 20.0ms 29.7μs +11699.3% -
Container.get singleton (warm) 1.7μs 500.0μs 0.3μs +466.1% -
TelemetryRuntime.init (lambda preset) 1.1ms 200.0ms 69.3ms -98.4% -
lambdaPreset config creation 1.4μs 2.0ms 1.0μs +41.3% -

Updated: 2026-06-30T22:36:13.293Z · Commit: ae6ed90

coderabbitai[bot]
coderabbitai Bot previously requested changes Jun 30, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🤖 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 `@docs/problem-code-registry.json`:
- Around line 7655-7684: The registry entries for
outbox-core/failure-metadata-missing and unit-of-work-context-invalid are
incorrectly marked as conditionally retryable 500s with retry-oriented recovery
text, which can mislead consumers. Update the problem definitions referenced by
OutboxProblems so these cases are clearly non-retryable invariant violations,
and revise the recovery metadata for both entries to use failure/abort-oriented
cause, userAction, and operatorAction wording instead of retry guidance.

In
`@packages/docs/src/content/docs/api/outbox-core/src/classes/InMemoryTransactionalOutboxStore.md`:
- Line 8: The class description for InMemoryTransactionalOutboxStore is using
wording that describes the TransactionalOutboxStore contract instead of the
concrete implementation. Update the generated doc comment so
InMemoryTransactionalOutboxStore is described as the in-memory transactional
outbox store implementation, and keep the provider-neutral contract wording only
on TransactionalOutboxStore to avoid confusing the API docs.

In
`@packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxDispatchProblemOptions.md`:
- Around line 8-10: The description for OutboxDispatchProblemOptions is a
copy-pasted storage-contract blurb and does not match the actual type; update
the markdown generated for this type alias to describe its real purpose as
options used when creating a Problem for dispatch failures. Use the source
definition in OutboxProblems and align the summary text with the
OutboxDispatchProblemOptions symbol so the generated docs reflect the actual API
meaning.

In
`@packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxDispatchResultMetadata.md`:
- Around line 8-11: The type description for OutboxDispatchResultMetadata is
incorrect and appears to be copied from a storage contract type. Update the
documentation text in OutboxDispatchResultMetadata so it describes this object
as the dispatch result metadata container for provider message IDs and related
metadata, not as a provider-neutral transactional outbox storage contract.

In
`@packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureMetadata.md`:
- Around line 8-11: The description for OutboxFailureMetadata is incorrect and
looks copied from a storage contract. Update the documentation for
OutboxFailureMetadata to describe the retry/failure metadata it actually
represents: retryable, terminal, attempt counts, max attempts, failed at, and
next visible at, rather than a provider-neutral storage contract.

In
`@packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureRecord.md`:
- Around line 8-11: The description for OutboxFailureRecord is incorrect and
appears to be copied from a storage-contract type. Update the markdown in the
OutboxFailureRecord type alias doc so it clearly describes this type as a
persisted failure record containing Problem.toJSON() output and retry metadata,
not as a provider-neutral storage contract. Use the OutboxFailureRecord
heading/context in the generated docs to replace the misleading summary with
accurate wording.

In `@packages/outbox-core/src/index.ts`:
- Around line 4-56: The barrel exports in index.ts are mixed with type exports,
making the public API harder to scan; reorganize the exports so runtime exports
are grouped by category first and all export type blocks are moved to the end.
Keep the existing symbols from OutboxProblems, conformance, and
InMemoryTransactionalOutboxStore, but reorder them consistently so the file
follows the index.ts guideline and has a clear runtime-then-types structure.

In `@packages/outbox-core/src/libs/conformance.ts`:
- Around line 147-158: Update the conformance checks in conformance.ts so they
verify the specific Problem subtype, not just that a rejection happened. Replace
the generic assertRejects usage in the malformed Unit of Work context and
failure metadata cases with a predicate/constructor-based assertion that expects
OutboxUnitOfWorkContextProblem and OutboxFailureMetadataProblem respectively,
using the existing test helpers around store.record and the failure-metadata
validation cases. Ensure these tests fail when a generic Error or the wrong
Problem subclass is thrown.

In `@packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts`:
- Around line 181-194: In runInUnitOfWork, the current commit path can overwrite
a previously committed rootState when two units start from the same snapshot.
Update InMemoryTransactionalOutboxStoreClient handling in
InMemoryTransactionalOutboxStore.runInUnitOfWork to either serialize commits or
add optimistic conflict detection before assigning this.rootState =
cloneState(client.state), so concurrent commits do not cause a lost update and
transaction boundaries remain consistent.
- Around line 218-242: The custom id path in InMemoryTransactionalOutboxStore is
overwriting existing records when options.id matches a different record, which
can corrupt the idempotency index. Update the record creation flow to detect a
record.id collision before state.records.set and reject it with a Problem
subclass instead of replacing the existing entry; make sure the check is done
alongside the existing scopedKey/idByIdempotencyScope handling in the same
insert logic.
- Around line 316-342: The retry exhaustion logic in
InMemoryTransactionalOutboxStore should not trust failure.maxAttempts from the
incoming failure metadata, since dispatcher-supplied Problem extensions can
bypass the record’s actual retry budget. Update the retry/terminal evaluation
and the record update in the failure handling path to use the stored retry state
from the existing record (for example record.retry.maxAttempts) as the source of
truth, while still preserving normalized failure details for retryable/terminal
flags and timestamps.
- Around line 45-47: The cloneRecord helper currently performs only a shallow
copy, so nested payload, metadata, and dispatchResult.metadata objects can still
be shared between stored outbox records and callers. Update cloneRecord in
InMemoryTransactionalOutboxStore to deep-clone those nested structures when
copying records, and make sure the store’s read/write paths that use cloneRecord
preserve isolation between the internal state and any returned intent or record
objects.

In `@packages/outbox-core/vitest.config.ts`:
- Around line 4-8: The Vitest configuration is too broad and does not enforce
the repo’s required test location convention. Update the test include setting in
vitest.config.ts so it only matches the mandatory src/tests/[ClassName].spec.ts
path pattern, and remove the looser src/**/*.test.ts / src/**/*.spec.ts
globbing. Keep the change focused on the test.include value in the test config
object so only the approved tests are picked up.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 204d3d69-4557-48bd-bcd9-ac1ecf8d5b83

📥 Commits

Reviewing files that changed from the base of the PR and between 14bd9f8 and 90eb975.

⛔ Files ignored due to path filters (2)
  • packages/problems-core/src/generated/problem-code-registry.ts is excluded by !**/generated/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (59)
  • .changeset/outbox-core-contract.md
  • .github/workflows/ci.yml
  • README.md
  • docs/package-catalog.json
  • docs/package-docs-report.md
  • docs/problem-code-registry.json
  • packages/docs/astro.config.mjs
  • packages/docs/package.json
  • packages/docs/src/content/docs/api/outbox-core/src/classes/InMemoryTransactionalOutboxStore.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxDispatchProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxFailureMetadataProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxUnitOfWorkContextProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/createOutboxFailureProblemExtensions.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/createTransactionalOutboxStoreContractSuite.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/readOutboxFailureMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/interfaces/TransactionalOutboxStore.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/ClaimBatchOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/ClaimedOutboxRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/DispatchResult.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/InMemoryTransactionalOutboxStoreClient.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/InMemoryTransactionalOutboxStoreState.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxClaim.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxDispatchProblemOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxDispatchResultMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureProblemExtensions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxIntent.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecordOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecordStatus.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRetryMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRetryOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxSourceReference.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxTenantBoundary.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxTraceContext.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContext.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContractCase.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContractOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContractSuite.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_DISPATCH_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_FAILURE_METADATA_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_UNIT_OF_WORK_CONTEXT_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/problems-core/src/classes/Problem.md
  • packages/docs/src/content/docs/en/guides/getting-started.mdx
  • packages/docs/src/content/docs/en/index.mdx
  • packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
  • packages/docs/tsconfig.typedoc.json
  • packages/outbox-core/README.md
  • packages/outbox-core/package.json
  • packages/outbox-core/src/index.ts
  • packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts
  • packages/outbox-core/src/libs/conformance.ts
  • packages/outbox-core/src/libs/problems/OutboxProblems.ts
  • packages/outbox-core/src/libs/types.ts
  • packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts
  • packages/outbox-core/tsconfig.json
  • packages/outbox-core/vitest.config.ts
  • public-api-surface.snapshot.json

Comment thread docs/problem-code-registry.json
Comment thread packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts Outdated
Comment thread packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts Outdated
Comment thread packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts
Comment thread packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts Outdated
Comment thread packages/outbox-core/vitest.config.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts (1)

184-219: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

runInUnitOfWork 커밋이 markDispatched/markFailed/clear()의 동시 변경을 덮어쓸 수 있습니다.

runInUnitOfWork는 시작 시점에 rootState를 복제해 client.state로 작업하다가, 끝날 때 this.rootState = cloneState(client.state)rootState 전체를 통째로 교체합니다. 반면 markDispatched, markFailed, clear()unitOfWorkQueue를 거치지 않고 this.rootState를 직접 읽고 씁니다.

따라서 UoW가 실행 중인 동안(예: fn 내부에서 await로 양보하는 사이) markDispatchedmarkFailedthis.rootState.records를 직접 변경하면, 해당 변경은 UoW 커밋 시점의 통째 교체(this.rootState = cloneState(client.state))에 의해 조용히 사라집니다(lost update). PR 목표인 "claiming must support safe concurrent dispatch semantics"와 충돌하는 동작입니다. 이는 이미 해결된 "두 UoW 간 lost update" 이슈와는 별개의, UoW와 비-UoW 변경 메서드 간의 새로운 race입니다.

모든 상태 변경 메서드(record/claimBatch/markDispatched/markFailed/clear)를 동일한 직렬화 큐를 통하도록 통일하는 것을 권장합니다.

🔒 제안 수정 방향 (모든 변경 작업을 동일 큐로 직렬화)
   async runInUnitOfWork<T>(
     fn: (
       context: TransactionalOutboxStoreContext<InMemoryTransactionalOutboxStoreClient>,
     ) => Promise<T>,
   ): Promise<T> {
-    let release: () => void = () => {};
-    const previous = this.unitOfWorkQueue;
-    this.unitOfWorkQueue = new Promise<void>((resolve) => {
-      release = resolve;
-    });
-
-    await previous;
-
-    const client: InMemoryTransactionalOutboxStoreClient = {
-      state: cloneState(this.rootState),
-    };
-    this.clients.add(client);
-
-    try {
-      const result = await fn({ client });
-      this.rootState = cloneState(client.state);
-      return result;
-    } finally {
-      this.clients.delete(client);
-      release();
-    }
+    return this.enqueue(async () => {
+      const client: InMemoryTransactionalOutboxStoreClient = {
+        state: cloneState(this.rootState),
+      };
+      this.clients.add(client);
+      try {
+        const result = await fn({ client });
+        this.rootState = cloneState(client.state);
+        return result;
+      } finally {
+        this.clients.delete(client);
+      }
+    });
   }
+
+  private enqueue<T>(task: () => Promise<T>): Promise<T> {
+    const run = this.unitOfWorkQueue.then(task, task);
+    this.unitOfWorkQueue = run.then(
+      () => undefined,
+      () => undefined,
+    );
+    return run;
+  }

   clear(): void {
-    this.rootState = createEmptyState();
+    void this.enqueue(async () => {
+      this.rootState = createEmptyState();
+    });
   }

markDispatched/markFailed도 동일하게 본문 전체를 this.enqueue(async () => { ... })로 감싸면 됩니다.

Also applies to: 309-325, 327-372

🤖 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 `@packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts` around
lines 184 - 219, `runInUnitOfWork` currently snapshots and later overwrites
`rootState`, while `markDispatched`, `markFailed`, and `clear()` mutate
`rootState` directly, so concurrent updates can be lost. Make the state-changing
methods in `InMemoryTransactionalOutboxStore` use the same serialized
queue/critical section as `runInUnitOfWork` so all writes are ordered
consistently. Update `record`, `claimBatch`, `markDispatched`, `markFailed`, and
`clear` to go through the shared enqueue path, and keep `rootState` updates
within that single serialization mechanism.
🤖 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.

Outside diff comments:
In `@packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts`:
- Around line 184-219: `runInUnitOfWork` currently snapshots and later
overwrites `rootState`, while `markDispatched`, `markFailed`, and `clear()`
mutate `rootState` directly, so concurrent updates can be lost. Make the
state-changing methods in `InMemoryTransactionalOutboxStore` use the same
serialized queue/critical section as `runInUnitOfWork` so all writes are ordered
consistently. Update `record`, `claimBatch`, `markDispatched`, `markFailed`, and
`clear` to go through the shared enqueue path, and keep `rootState` updates
within that single serialization mechanism.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 15d5343c-9ed6-4168-9ebf-463be215407e

📥 Commits

Reviewing files that changed from the base of the PR and between 90eb975 and 0b6ef9a.

⛔ Files ignored due to path filters (2)
  • packages/problems-core/src/generated/problem-code-registry.ts is excluded by !**/generated/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (48)
  • .github/workflows/ci.yml
  • README.md
  • docs/package-catalog.json
  • docs/package-docs-report.md
  • docs/problem-code-registry.json
  • packages/docs/astro.config.mjs
  • packages/docs/src/content/docs/api/outbox-core/src/classes/InMemoryTransactionalOutboxStore.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxDispatchProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxFailureMetadataProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxRecordIdConflictProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/classes/OutboxUnitOfWorkContextProblem.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/createOutboxFailureProblemExtensions.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/createTransactionalOutboxStoreContractSuite.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/readOutboxFailureMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/ClaimBatchOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/ClaimedOutboxRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/DispatchResult.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxClaim.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxDispatchProblemOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxDispatchResultMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxFailureRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxIntent.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecordOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecordStatus.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRetryMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRetryOptions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxSourceReference.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxTenantBoundary.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxTraceContext.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContext.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_DISPATCH_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_FAILURE_METADATA_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_RECORD_ID_CONFLICT_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/outbox-core/src/variables/OUTBOX_UNIT_OF_WORK_CONTEXT_PROBLEM_CODE.md
  • packages/docs/src/content/docs/api/problems-core/src/classes/Problem.md
  • packages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.md
  • packages/docs/src/content/docs/en/index.mdx
  • packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
  • packages/outbox-core/src/index.ts
  • packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts
  • packages/outbox-core/src/libs/conformance.ts
  • packages/outbox-core/src/libs/problems/OutboxProblems.ts
  • packages/outbox-core/src/libs/types.ts
  • packages/outbox-core/vitest.config.ts
  • public-api-surface.snapshot.json
  • scripts/problem-registry.mts
💤 Files with no reviewable changes (7)
  • packages/docs/src/content/docs/api/outbox-core/src/functions/createOutboxFailureProblemExtensions.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/ClaimedOutboxRecord.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRecordStatus.md
  • packages/docs/src/content/docs/api/outbox-core/src/functions/readOutboxFailureMetadata.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/TransactionalOutboxStoreContext.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/DispatchResult.md
  • packages/docs/src/content/docs/api/outbox-core/src/type-aliases/OutboxRetryOptions.md

@kang-heewon

Copy link
Copy Markdown
Member Author

Addressed the remaining lost-update review finding in 61c39a3 by routing root writes through the same mutation queue as Unit of Work commits and adding an explicit same-store UoW reentrancy guard to prevent self-deadlocks. Added regression coverage for queued root writes during an in-flight UoW, clear serialization, and no-context root mutations inside active UoW callbacks.\n\nValidation is green from the repo-owned gates: validate, docs-build, docs-links, docs-sync-check, benchmark, companion, changes, and WIP all pass on head 61c39a3. Local checks also passed: pnpm check, pnpm --filter @croco/outbox-core test, and pnpm --filter @croco/outbox-core typecheck.\n\nRemaining merge blocker is external to the repository changes: the required CodeRabbit status is failing with "Prepaid credits exhausted — enable usage-based reviews". GitHub also still reports the stale CodeRabbit requested-changes state from the previous review.

@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously requested changes Jun 30, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

♻️ Duplicate comments (1)
packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts (1)

140-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Deferred/createDeferred 중복 정의.

packages/outbox-core/src/libs/conformance.ts:755-774와 완전히 동일한 헬퍼가 이 파일에도 새로 추가되었습니다. 공유 테스트 유틸리티 모듈로 추출해 두 파일에서 import하는 것을 권장합니다(상세 제안은 conformance.ts 코멘트 참조).

🤖 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 `@packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts` around lines
140 - 159, Remove the duplicated Deferred/createDeferred helper from
TransactionalOutboxStore.spec and reuse the shared test utility instead; the
same helper already exists in conformance.ts, so extract or import it from a
common test-utils module and update the spec to reference that shared symbol
rather than defining it locally.
🤖 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 `@packages/outbox-core/src/libs/conformance.ts`:
- Around line 747-774: The `assertDefined` and `createDeferred` helpers in
`conformance.ts` are duplicated in the transactional outbox spec, so extract
them into a shared test utility module and import them from both places to keep
the implementations aligned. While doing so, update the guard failures in
`assertDefined` and `createDeferred` to throw a `Problem` subclass instead of
generic `Error`, and keep the existing symbols (`assertDefined`, `Deferred`,
`createDeferred`) as the main integration points for the refactor.

In `@packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts`:
- Line 123: This test assertion in TransactionalOutboxStore.spec should follow
the async error-checking guideline by using Vitest’s
rejects.toThrow(OutboxUnitOfWorkContextProblem) instead of
rejects.toBeInstanceOf(...). Update the failing expectation in the relevant
promise rejection test to assert the thrown error type with toThrow, keeping the
existing async/await pattern and matching the other error-case tests in this
suite.

---

Duplicate comments:
In `@packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts`:
- Around line 140-159: Remove the duplicated Deferred/createDeferred helper from
TransactionalOutboxStore.spec and reuse the shared test utility instead; the
same helper already exists in conformance.ts, so extract or import it from a
common test-utils module and update the spec to reference that shared symbol
rather than defining it locally.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7c07419f-540a-475a-a21e-c1a1c49be586

📥 Commits

Reviewing files that changed from the base of the PR and between 0b6ef9a and 61c39a3.

⛔ Files ignored due to path filters (1)
  • packages/problems-core/src/generated/problem-code-registry.ts is excluded by !**/generated/**
📒 Files selected for processing (8)
  • docs/problem-code-registry.json
  • packages/docs/src/content/docs/api/outbox-core/src/classes/InMemoryTransactionalOutboxStore.md
  • packages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.md
  • packages/docs/src/content/docs/en/reference/problem-recovery-cookbook.md
  • packages/outbox-core/src/libs/InMemoryTransactionalOutboxStore.ts
  • packages/outbox-core/src/libs/conformance.ts
  • packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts
  • public-api-surface.snapshot.json

Comment thread packages/outbox-core/src/libs/conformance.ts
Comment thread packages/outbox-core/src/tests/TransactionalOutboxStore.spec.ts Outdated
@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@kang-heewon
kang-heewon dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot] June 30, 2026 23:09

Superseded by later commits and current-head CodeRabbit review fc776a68 with no actionable comments.

@kang-heewon
kang-heewon merged commit 361789b into trunk Jun 30, 2026
9 checks passed
@kang-heewon
kang-heewon deleted the fix/outbox-core-store branch June 30, 2026 23:12
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.

[outbox-core] Define TransactionalOutboxStore contract

1 participant