feat(gateway): compose authenticated Planning Today state - #186
Conversation
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughGateway Today 엔드포인트가 Identity 세션을 인증하고, 서명된 Planning 요청으로 Today aggregate를 조합합니다. 입력, 응답 크기, JSON 구조를 검증하며 오류를 문제 응답으로 변환합니다. 관련 단위 테스트와 통합 테스트도 추가했습니다. ChangesGateway Today 조합
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway as HealthController.today
participant Identity
participant Planning
Client->>Gateway: /v1/today?date 요청과 세션 쿠키
Gateway->>Identity: 세션 인증 요청
Identity-->>Gateway: workspace ID
Gateway->>Planning: 서명된 workspace 컨텍스트와 date
Planning-->>Gateway: 검증 대상 Today aggregate
Gateway-->>Client: Today 응답 또는 문제 응답
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/gateway/src/today-composition.ts (2)
241-252: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
actions요소 구조를 검증하지 않고 그대로 전달합니다.
record.actions는 배열 여부와 개수만 검사합니다. 요소 내용은unknown으로 클라이언트 응답에 그대로 들어갑니다. Planning이 필드를 추가하거나 형태를 바꾸면 게이트웨이는 검증 없이 통과시킵니다.전체 응답이 64KiB로 제한되므로 크기 위험은 없습니다. 그러나 가이드라인은 모든 외부 응답을 바운드하고 검증하도록 요구합니다. 최소한 각 요소가 배열이 아닌 객체인지, 그리고 필수 식별자 필드를 가지는지 검사하십시오.
가이드라인 근거: "Treat every external response, stored JSON value, environment value, model output, and connector result as untrusted until bounded and validated."
🤖 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 `@apps/gateway/src/today-composition.ts` around lines 241 - 252, Validate each element of record.actions before constructing the frozen response: require every element to be a non-null, non-array object and verify its required identifier fields are present and valid. Reject the entire response with unavailable() when any action fails validation, while preserving the existing array and maximum-count checks and the Object.freeze behavior.Source: Coding guidelines
203-214: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value성공 본문 미디어 타입을
application/json으로 좁히는 방안을 검토하십시오.현재 두 호출 지점(Line 318, Line 347)은 상태 코드 200 응답에만
readBoundedJson을 사용합니다. 그 경로에서application/problem+json은 유효한 성공 표현이 아닙니다. 허용 목록을 인자로 받으면 계약이 더 엄격해집니다.최종 결과는
requirePlanningToday가 거부하므로 현재 동작은 fail closed입니다. 우선순위는 낮습니다.🤖 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 `@apps/gateway/src/today-composition.ts` around lines 203 - 214, Update readBoundedJson to accept an allowed media-type list or equivalent parameter, and have both successful 200-response call sites use an allowlist containing only application/json. Keep application/problem+json permitted only where an error response requires it, while preserving requirePlanningToday’s fail-closed behavior.apps/gateway/src/app.module.ts (1)
56-67: 🧹 Nitpick | 🔵 Trivial실패 경로에 관측성을 추가하는 방안을 검토하십시오.
catch블록은 모든 예상치 못한 오류를 고정 503 문제 응답으로 흡수합니다. 로그와 메트릭 기록이 없습니다. 그래서 Identity 장애와 Planning 장애, 잘못된 업스트림 페이로드를 운영 중에 구분할 수 없습니다.
gatewayMetrics(Line 11)에 실패 코드별 카운터를 기록하십시오. 또한composePlanningToday가 생성하는 상관 ID를 호출자에게 반환하거나 구조화 로그에 남기는 방안을 검토하십시오. 자격증명, 원시 업스트림 응답, 스택 트레이스는 남기지 마십시오.가이드라인 근거: "Each service owns its migrations, runtime configuration, observability, tests, and shutdown behavior."
🤖 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 `@apps/gateway/src/app.module.ts` around lines 56 - 67, Update the error handling around composePlanningToday to record gatewayMetrics counters differentiated by GatewayTodayError codes and the generic today_composition_unavailable path before throwing the problem response. Preserve existing response behavior, and expose the correlation ID produced by composePlanningToday either in the caller response or in a structured log; omit credentials, raw upstream payloads, and stack traces.Source: Coding guidelines
apps/gateway/src/today-composition.test.ts (1)
101-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win주요 실패 경로에 대한 테스트를 추가하세요.
현재 테스트는 Identity 401, Planning 503, 잘못된
aggregateId만 검증합니다. 다음 경로도 추가하세요.
- Planning 404 →
today_not_foundfetcher예외 및 타임아웃- 잘못된
content-type또는content-lengthMAXIMUM_RESPONSE_BYTES초과 본문 및 잘못된 UTF-8- 유효하지 않은
workspaceIdMAXIMUM_TODAY_ACTIONS초과- 32바이트 미만 시크릿 및 잘못된 서비스 origin
🤖 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 `@apps/gateway/src/today-composition.test.ts` around lines 101 - 147, Expand the composePlanningToday test coverage for the listed failure paths: map Planning 404 to today_not_found, reject fetcher exceptions and timeouts, invalid content-type/content-length, oversized or invalid UTF-8 responses, invalid workspaceId, and more than MAXIMUM_TODAY_ACTIONS actions as unavailable. Add configuration tests for secrets shorter than 32 bytes and invalid service origins, asserting the established validation errors without changing successful behavior.Source: Coding guidelines
🤖 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 `@apps/gateway/src/app.module.test.ts`:
- Around line 32-36: Isolate Today unavailability tests from inherited
dependency environment variables. In apps/gateway/src/app.module.test.ts lines
32-36, delete IDENTITY_SERVICE_ORIGIN, PLANNING_SERVICE_ORIGIN, and
PLANNING_GATEWAY_CONTEXT_SECRET at test start and restore their original values
in finally. In apps/gateway/src/app.module.integration.test.ts lines 40-43,
apply the same deletion before harness creation and restoration after the test
using shared beforeEach/afterEach hooks; update HealthController.today-related
tests only, with no direct change needed elsewhere.
In `@apps/gateway/src/today-composition.ts`:
- Around line 310-345: Update the identityResponse and planningResponse error
branches in the surrounding today composition flow to explicitly cancel each
response body before throwing for status 401, 404, or other non-200 responses.
Preserve the existing GatewayTodayError and unavailable() behavior after
cancellation, while leaving successful response handling unchanged.
---
Nitpick comments:
In `@apps/gateway/src/app.module.ts`:
- Around line 56-67: Update the error handling around composePlanningToday to
record gatewayMetrics counters differentiated by GatewayTodayError codes and the
generic today_composition_unavailable path before throwing the problem response.
Preserve existing response behavior, and expose the correlation ID produced by
composePlanningToday either in the caller response or in a structured log; omit
credentials, raw upstream payloads, and stack traces.
In `@apps/gateway/src/today-composition.test.ts`:
- Around line 101-147: Expand the composePlanningToday test coverage for the
listed failure paths: map Planning 404 to today_not_found, reject fetcher
exceptions and timeouts, invalid content-type/content-length, oversized or
invalid UTF-8 responses, invalid workspaceId, and more than
MAXIMUM_TODAY_ACTIONS actions as unavailable. Add configuration tests for
secrets shorter than 32 bytes and invalid service origins, asserting the
established validation errors without changing successful behavior.
In `@apps/gateway/src/today-composition.ts`:
- Around line 241-252: Validate each element of record.actions before
constructing the frozen response: require every element to be a non-null,
non-array object and verify its required identifier fields are present and
valid. Reject the entire response with unavailable() when any action fails
validation, while preserving the existing array and maximum-count checks and the
Object.freeze behavior.
- Around line 203-214: Update readBoundedJson to accept an allowed media-type
list or equivalent parameter, and have both successful 200-response call sites
use an allowlist containing only application/json. Keep application/problem+json
permitted only where an error response requires it, while preserving
requirePlanningToday’s fail-closed behavior.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: 5f79246f-34e9-4f9f-9f8e-6d299d7f3f63
📒 Files selected for processing (5)
apps/gateway/src/app.module.integration.test.tsapps/gateway/src/app.module.test.tsapps/gateway/src/app.module.tsapps/gateway/src/today-composition.test.tsapps/gateway/src/today-composition.ts
2 participants
Buyer-visible outcome
Advance #163 from an unconditional synthetic-data-safe 503 to the first real authenticated Today composition slice.
GET /v1/today?date=YYYY-MM-DDnow derives workspace authority from Identity session introspection, mints the reviewed short-livedlife-os.workspace.v1HMAC context, and reads the durable Planning-owned Today aggregate through Planning's HTTP contract.The browser cookie is sent only to Identity and is never forwarded to Planning. Upstream origins, cookies, secrets, response media types, response bytes, status classes, UUID/date evidence, and timeouts are bounded and validated. Invalid/malformed or unavailable dependencies fail closed with fixed credential-free Problem Details.
Explicit partial state
This PR does not claim issue #163 complete. Habit composition remains explicit as
degraded: ["habits_not_composed"]; no habit, review, priority, or other synthetic product state is fabricated. The next compatible slice must compose real Habit state through the Habit service's signed workspace boundary and add end-to-end operational acceptance.Test-first evidence
The branch began with
today-composition.test.tsagainst a nonexistent production composition module, then added the narrow implementation and HTTP boundary coverage. Tests assert Identity-derived workspace authority, exact downstream HMAC headers, no cookie forwarding to Planning, authentication distinction, dependency failure behavior, malformed upstream rejection, required date handling, and no fallback placeholder payload.Merge gate
No historical evidence transfers. Require this unchanged exact head to pass gateway tests/typecheck/build, CI including current-live-base merge compatibility, AppGuardrail, SAST Semgrep, Security Scan, Commercial Readiness, current automated review, and zero actionable current-head findings under live repository policy.
Closes no issue; advances #163.
Summary by CodeRabbit
새 기능
버그 수정