Repository navigation
feat(coordination): dispatcher core — state store + orchestration (ADR-028 D3/D6) - #1606
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces a complete coordination dispatcher system enabling inter-team state management. It adds a DynamoDB-backed store with optimistic versioning, a dispatcher that orchestrates state initialization, operation validation/application, and persistence, and comprehensive test coverage validating all operations and conflict handling. ChangesCoordination Dispatcher and Store
Sequence DiagramsequenceDiagram
participant Caller
participant dispatchCoordinationOp
participant readCoordinationState
participant dispatchOp as SDK dispatchOp
participant writeCoordinationState
participant safeProjectForTeam as SDK safeProjectForTeam
Caller->>dispatchCoordinationOp: input
dispatchCoordinationOp->>readCoordinationState: tenantId, eventId
alt State exists
readCoordinationState-->>dispatchCoordinationOp: { state, version }
else State missing
readCoordinationState-->>dispatchCoordinationOp: undefined
dispatchCoordinationOp->>dispatchCoordinationOp: initialize plugin.initialState
end
dispatchCoordinationOp->>dispatchOp: state, operation
alt Operation valid
dispatchOp-->>dispatchCoordinationOp: newState
dispatchCoordinationOp->>writeCoordinationState: newState, expectedVersion
alt Write succeeds
writeCoordinationState-->>dispatchCoordinationOp: { kind: "ok" }
dispatchCoordinationOp->>safeProjectForTeam: newState
safeProjectForTeam-->>dispatchCoordinationOp: projection
dispatchCoordinationOp-->>Caller: { ok, projection }
else Version conflict
writeCoordinationState-->>dispatchCoordinationOp: { kind: "conflict" }
dispatchCoordinationOp-->>Caller: { conflict }
end
else Operation invalid
dispatchOp-->>dispatchCoordinationOp: error
dispatchCoordinationOp-->>Caller: { rejected, error }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
infrastructure/test/problem-deploy/coordination-dispatch.test.ts (1)
155-165: ⚡ Quick winAdd a regression test for the fail-safe projection path.
The new contract here is "return
fallbackProjectionifprojectForTeamthrows", but the suite only exercises successful projections. A tiny plugin override that throws fromprojectForTeamwould lock in that safety guarantee for both exported helpers.🤖 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 `@infrastructure/test/problem-deploy/coordination-dispatch.test.ts` around lines 155 - 165, Add a regression test that exercises the fail-safe projection path by mocking projectForTeam to throw and asserting projectCoordinationForTeam returns the fallback projection; specifically, in coordination-dispatch.test.ts create a test similar to the existing ones that uses fakeStore (e.g., { getItem: undefined }) and a plugin/counter override where projectForTeam throws, then call projectCoordinationForTeam(store, counter, base, /* fallbackProjection */ { count: 0 }) and assert the return equals the fallback ({ count: 0 }) and that no PutCommand was sent via send.mock.calls; locate code references to projectCoordinationForTeam and projectForTeam to implement the throw behavior and expectations.
🤖 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
`@infrastructure/lib/problem-deploy/handlers/participant-handler/coordination-dispatch.ts`:
- Around line 52-54: Before seeding fresh state with plugin.initialState,
validate that the incoming coordination context matches the event/team
identifiers you will persist: check that input.ctx exists, that
input.ctx.teamIds includes the teamId(s) expected for this event, and that
input.eventId matches any event identifier in ctx; if validation fails, reject
early. Add an explicit guard or small Zod schema right before the
readCoordinationState / plugin.initialState block (and replicate the same
validation before the later branch around lines 93–95) so you never call
plugin.initialState with a ctx that doesn't match the input.eventId or lacks the
required teamId(s).
---
Nitpick comments:
In `@infrastructure/test/problem-deploy/coordination-dispatch.test.ts`:
- Around line 155-165: Add a regression test that exercises the fail-safe
projection path by mocking projectForTeam to throw and asserting
projectCoordinationForTeam returns the fallback projection; specifically, in
coordination-dispatch.test.ts create a test similar to the existing ones that
uses fakeStore (e.g., { getItem: undefined }) and a plugin/counter override
where projectForTeam throws, then call projectCoordinationForTeam(store,
counter, base, /* fallbackProjection */ { count: 0 }) and assert the return
equals the fallback ({ count: 0 }) and that no PutCommand was sent via
send.mock.calls; locate code references to projectCoordinationForTeam and
projectForTeam to implement the throw behavior and expectations.
🪄 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
Run ID: 97eef2a0-ebaf-4347-b936-961cdf3b764d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
infrastructure/lib/problem-deploy/handlers/participant-handler/coordination-dispatch.tsinfrastructure/lib/problem-deploy/handlers/participant-handler/coordination-store.tsinfrastructure/package.jsoninfrastructure/test/problem-deploy/coordination-dispatch.test.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1606 +/- ##
==========================================
+ Coverage 93.25% 93.26% +0.01%
==========================================
Files 379 381 +2
Lines 10447 10472 +25
Branches 3187 3198 +11
==========================================
+ Hits 9742 9767 +25
Misses 217 217
Partials 488 488 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PR #1606 review (CodeRabbit Major): dispatch/project は eventId で永続化しつつ ctx で 初期化するため、 ctx.eventId が eventId とズレたり認証 team が ctx.teamIds に居ない場合、 別 event 用に組んだ state を保存 / 配信し得る。 read/write 前に fail-closed で弾く。 - `isContextConsistent(input)`: `ctx.eventId === eventId && ctx.teamIds.includes(teamId)`。 - dispatch: 不整合なら read/write せず `{ kind: "rejected", error: "context_mismatch" }`。 - project: 不整合なら fallbackProjection を返す (= 機密非漏洩)。 ## Regression analysis - 防御 guard の追加のみ (正常系の挙動は不変: 整合 ctx では従来通り dispatch/project)。 - 新規 3 test。 計 14 tests 緑、 coordination-* modules は branches/functions/lines 100% (22/22, 6/6, 25/25)。 infra tsc / biome 通過。 ## Physical impact - NO-OP on CFn / deployed artifacts。 未配線 handler logic の防御強化のみ。 Relates #1420 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…R-028 D3/D6) #1420 / ADR-028 D3+D6 の platform-side dispatcher の **純粋なコア** を実装する (TDD で logic を先に pin)。 副作用は DI、 意味論は plugin に委譲し、 platform は host に徹する。 - `coordination-store.ts` (D3): coordination の per-event 共有 state を **既存 Deployments テーブル** に保存する store。 cast-event と同方針で新規 table / IAM / CDK は不要 (PK=`COORD#<tenantId>#<eventId>` / SK=`STATE` / version 楽観ロック)。 read + version 条件付き write (conflict→caller が 409 退避)。 - `coordination-dispatch.ts` (D6): orchestration。 read-or-init → SDK `dispatchOp` (validate→apply) → 楽観ロック write → `safeProjectForTeam` (fail-safe projection)。 store/plugin を DI して 100% unit-test 可能。 - `@tenkacloud/coordination-plugin-sdk` を infrastructure の dependency に追加。 **self-merge しない (owner review 用)。** 残る配線 = participant-handler の route (`POST/GET /portal/me/coordination/<eventId>`) + plugin loader。 loader は ADR-028 D6 の bundle-glob vs S3 の選択 (= owner judgment) が要るため本 PR には含めない。 本 PR は loading 機構に依存しない tested コアを先に置く。 ## Regression analysis - **純加算 + 未配線**: 新規 module 2 + test 1。 まだ route が import しないので participant-handler の bundle / 挙動は不変 (= dead code までは行かないが live path には未接続)。 既存 test に影響なし。 - 新規 module は branches/functions/lines 100% (11 tests)。 infra tsc / biome / harness 通過。 既存 Deployments テーブルの key schema (`COORD#` PK は `DEPLOYMENT#` と衝突しない) を踏襲。 - `@tenkacloud/coordination-plugin-sdk` 依存追加 (既に他 workspace で使用、 GATED 100%)。 bun.lock 更新。 ## Physical impact - NO-OP on CFn / deployed artifacts。 新規 CDK construct なし (state は既存 Deployments テーブル に相乗り)。 module はまだどの Lambda bundle にも取り込まれない (route 未配線) ため、 participant-handler の deployed artifact も不変。 Relates #1420 Relates #1417 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PR #1606 review (CodeRabbit Major): dispatch/project は eventId で永続化しつつ ctx で 初期化するため、 ctx.eventId が eventId とズレたり認証 team が ctx.teamIds に居ない場合、 別 event 用に組んだ state を保存 / 配信し得る。 read/write 前に fail-closed で弾く。 - `isContextConsistent(input)`: `ctx.eventId === eventId && ctx.teamIds.includes(teamId)`。 - dispatch: 不整合なら read/write せず `{ kind: "rejected", error: "context_mismatch" }`。 - project: 不整合なら fallbackProjection を返す (= 機密非漏洩)。 ## Regression analysis - 防御 guard の追加のみ (正常系の挙動は不変: 整合 ctx では従来通り dispatch/project)。 - 新規 3 test。 計 14 tests 緑、 coordination-* modules は branches/functions/lines 100% (22/22, 6/6, 25/25)。 infra tsc / biome 通過。 ## Physical impact - NO-OP on CFn / deployed artifacts。 未配線 handler logic の防御強化のみ。 Relates #1420 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
614eac9 to
cfa4038
Compare
#1606 で dispatcher core (state store + dispatchCoordinationOp / projectCoordinationForTeam) はマージ済みだが、 問題が同梱する CoordinationPlugin を runtime に取得する loader が無かった。 ADR-028 D6 の「plugin module を S3 (ADR-008 payload 経路) から fetch + import() で動的 load」 の loader 部分を実装する。 静的レジストリではなく動的 import を採るのは、 community が coordination 付き問題を後から 追加するたびに platform を再デプロイせずに済むため (= 問題は plugin、 platform は host。 静的だと platform が各問題に結合し問題カタログの moat を殺す)。 coordination-plugin-loader.ts: - `isCoordinationPlugin` — 動的 load した未知 module が SDK 契約 (initialState + 3 必須 hook、 tick は optional) を満たすか構造判定する門番。 - `loadCoordinationPlugin(importer, ref)` — import 関数を注入 (本番は S3 materialize 後の module、 test は fake)。 import 失敗 / 契約不一致は null を返し caller が safe fallback。 - `loadAndDispatchCoordinationOp` / `loadAndProjectCoordinationForTeam` — 動的 load → 既存 #1606 dispatcher への委譲を 1 経路にした orchestration (load 不可は plugin_unavailable / fallbackProjection で participant API を壊さない)。 別 isolate sandbox は立てない (ADR-028 D6、 cost/複雑度): plugin の hook は SDK 契約上すべて 純関数で、 bug は当該 event の 1 row に閉じる (optimistic lock + DDB write fail で停止)。 ## Regression analysis - 純追加 (新 file + 新 test のみ、 既存 source 無改変)。 既存 2266 test は不変、 新 12 test 追加で infra 2278 緑。 loader file 100% (17/17 stmts, 16/16 branch, 4/4 func, 14/14 lines)。 - typecheck 0 / biome clean / make harness no findings (scaffolding 含む invariant 違反なし)。 - 実 importer (S3 から plugin を materialize) と HTTP route 配線は本 PR の scope 外 (= 次 increment)。 importer を DI 境界に置いたので、 loader の意味論は S3 経路の有無と独立に検証済み。 ## Physical impact - NO-OP on CFn / deployed artifacts。 participant-handler Lambda の source に純関数 module を 追加するのみ (新 IAM / table / env 無し)。 既存 handler の挙動は不変。 Relates #1420 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (#1619) #1606 (dispatcher core) + #1617 (動的 loader) を participant-portal の HTTP route に接続する。 problem は plugin、 platform は host の原則どおり、 platform は dispatch / projection だけを担い、 semantics (alliance / routing / shared queue 等) は問題同梱 plugin に閉じる。 - `coordination-handler.ts` — team-login-key 認証 → scope 解決 → loader+dispatcher 委譲。 outcome は ok / rejected / conflict / unavailable / not_configured。 scope resolver は active な deployment 行から tenant/event/team を引き、 その問題が interTeamCoordination を宣言していれば moduleRef を確定する。 `parseCoordinationConfig` は `PROBLEM_COORDINATION` env を parse。 - `index.ts` — `POST /portal/me/coordination/op` + `GET /portal/me/coordination/projection` を登録。 outcome→HTTP は StatusCodes 名で明示 (rejected=422 / conflict=409 / unavailable=503 / not_configured=404)。 - `schemas.ts` — `CoordinationOpBodySchema` ({ op })。 op の意味論は plugin の validateOp が判定。 **importer は seam (ADR-028 D6)**: 問題同梱 plugin を S3 (ADR-008 payload) から materialize して import する実装と、 `PROBLEM_COORDINATION` env の CDK 配線、 full event roster の ctx.teamIds 解決は 別 increment (owner 領域: 未信頼コード実行 + Lambda bundling/IAM)。 未配線の間は loader が null → route は unavailable / fallback projection で安全に応答する (= participant API を壊さない)。 ## Regression analysis - 既存 route は無改変。 新 route 2 本 + handler module の追加。 index.ts は宣言的 wiring のみ。 - infra 全 2291 test 緑 (従来 2278 + coordination-handler 13)。 handler module 100% (23/23 stmts, 20/20 branch, 5/5 func)。 typecheck 0 / biome clean / make harness 違反なし。 - `PROBLEM_COORDINATION` env は optional。 未設定時は parseCoordinationConfig→{}→全 route not_configured で、 既存挙動 (coordination 無効) と等価。 ## Physical impact - **NO-OP** on CFn / deployed artifacts。 participant-handler Lambda の source に route + handler を 追加するのみ (新 IAM / table 無し)。 optional env `PROBLEM_COORDINATION` は CDK 未配線でも absent=安全。 既存 Function URL / IAM / DDB は不変。 Relates #1420 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
infrastructureworkspace; per theinfra は PR を開くが self-merge しないdirective).Implements the pure core of #1420 / ADR-028's platform-side coordination dispatcher (D3 + D6), TDD-first — side effects DI'd, semantics delegated to the problem's plugin, platform stays a host.
coordination-store.ts(D3) — per-event shared coordination state on the existing Deployments table (same approach ascast-event: no new table / IAM / CDK).PK=COORD#<tenantId>#<eventId>/SK=STATE/versionoptimistic lock. Read + version-conditional write (conflict → caller returns 409).coordination-dispatch.ts(D6) — orchestration: read-or-init → SDKdispatchOp(validate→apply) → optimistic-lock write →safeProjectForTeam(fail-safe projection). Store + plugin are DI'd → 100% unit-testable.@tenkacloud/coordination-plugin-sdktoinfrastructuredeps.Test plan
infrastructuretsc --noEmitclean /biome checkclean /make harnessinvariant 違反 0.Regression analysis
COORD#PK doesn't collide with the table'sDEPLOYMENT#PK schema. SDK dep already used by other (gated 100%) workspaces.Physical impact
Relates #1420
Relates #1417
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests