feat(planning): add durable Today synchronization - #127
Conversation
📝 WalkthroughWalkthroughDurable Today aggregate를 PostgreSQL에 저장하고, 인증된 BFF와 브라우저에서 명시적으로 조회·생성·갱신하도록 추가했다. revision, ETag, 멱등성, workspace 격리, 동시성 검증과 CI를 포함한다. 데이터 권리 요청 원장과 최근 인증 검증도 추가했다. ChangesDurable Today 동기화
데이터 권리 요청 원장
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
* test(identity): define durable data-rights request ledger * feat(identity): persist data-rights request state * feat(identity): add data-rights request ledger schema * test(identity): preserve data-rights receipts through erasure * fix(identity): retain data-rights receipts after erasure * chore(identity): sequence data-rights request migration * chore(identity): remove duplicate migration sequence * docs(identity): record durable data-rights ledger boundary * test(identity): expose request-id collision as domain conflict * fix(identity): normalize request ledger conflicts * test(identity): harden data-rights ledger integration harness * test(identity): cover dual request conflict evidence * docs(identity): align data-rights ledger implementation status * docs(changelog): record durable data-rights ledger * fix(identity): resolve ledger migration path portably * test(identity): require immutable terminal receipt storage * test(identity): model pg timestamp rows as Date values * test(identity): satisfy pg parameter mutability contract * fix(identity): enforce immutable terminal data-rights receipts * test(identity): assert immutable receipt no-op at database boundary * test(identity): bind migration fixture lock to one PostgreSQL session
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/components/today-workspace-sync-panel.tsx (1)
171-180: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win버튼이 사라지면서 키보드 포커스가 사라집니다.
저장 버튼을 누르면 상태가
checking이 되고canSave가 거짓이 됩니다. 그러면 포커스를 가진 버튼이 언마운트되고 포커스가body로 돌아갑니다. 키보드 사용자는 위치를 잃습니다. 같은 문제가useWorkspaceToday버튼에도 있습니다.버튼을 제거하는 대신
disabled로 유지하십시오. 조회 버튼(164-170행)은 이미 이 방식을 사용합니다.♻️ 제안 수정
- {canSave ? ( - <button type="button" onClick={() => void saveLocal()}> - {saveLabel} - </button> - ) : null} + <button + type="button" + disabled={!canSave} + onClick={() => void saveLocal()} + > + {saveLabel} + </button>🤖 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/web/app/components/today-workspace-sync-panel.tsx` around lines 171 - 180, Update the save and use-workspace button rendering in the component containing saveLocal and useWorkspace so the buttons remain mounted while unavailable, using disabled state instead of conditional unmounting. Preserve the existing visibility conditions where appropriate, and disable the save button when canSave is false and the workspace button when it is not actionable, matching the existing lookup-button pattern.
🧹 Nitpick comments (9)
apps/identity-service/tests/session-authentication-migration.integration.test.ts (1)
86-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value정리 순서는 정확합니다. 데이터베이스 이름 중복만 검토하십시오.
lock 보유 클라이언트에서 DROP과 unlock을 실행하고, 그 다음 클라이언트를 release합니다. 순서가 정확합니다. 다만 DDL 문자열은
life_os_identity_migration_test를 직접 씁니다.TEMPORARY_DATABASE_NAME상수를 변경하면 이 문자열은 낡은 값으로 남습니다. 식별자 검증 후 상수를 삽입하면 중복이 사라집니다. 같은 PR의apps/identity-service/src/data-rights-request-ledger.integration.test.ts13-20행이 이 패턴을 사용합니다.🤖 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/identity-service/tests/session-authentication-migration.integration.test.ts` around lines 86 - 99, Update the DROP DATABASE statement in the cleanup block guarded by lockHeld and adminClient to use the TEMPORARY_DATABASE_NAME constant instead of duplicating the literal database name. Validate or safely interpolate the identifier following the existing pattern in the data-rights request ledger integration test, while preserving the current DROP, unlock, release, and pool shutdown order.apps/identity-service/src/oauth-http-boundary.ts (1)
94-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value최근 인증 게이트를 전용 모듈로 분리하는 것을 검토하십시오.
requireRecentAuthentication은 OAuth HTTP 경계가 아니라 데이터 권리 경계에서만 사용됩니다. 소비자는data-rights-authenticated-application.ts이고, 테스트는oauth-http-boundary를 네임스페이스로 가져와 동적으로 심볼을 찾습니다. 예:recent-authentication.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 `@apps/identity-service/src/oauth-http-boundary.ts` around lines 94 - 101, Move requireRecentAuthentication into a dedicated recent-authentication module, such as recent-authentication.ts, and update data-rights-authenticated-application.ts and its tests to import the symbol from the new module. Preserve the function signature and behavior while removing its ownership from oauth-http-boundary.apps/identity-service/src/data-rights-recent-auth.test.ts (1)
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value두 테스트 파일이 대상 심볼을 정적 import 대신 동적으로 해석합니다. 공통 원인은 같습니다. 두 파일은 모듈을
unknown레코드로 캐스트하거나 동적import()로 가져온 뒤typeof단언으로 심볼을 확인합니다. 이 방식은 타입 검사를 우회하고, 모듈 로드 실패의 원인을 숨기며, 수동 타입 선언을 강요합니다.
apps/identity-service/src/data-rights-recent-auth.test.ts#L10-L16:recentAuthenticationGate헬퍼와RecentAuthenticationGate타입을 제거하고,requireRecentAuthentication을 명명 import로 가져오십시오.apps/identity-service/src/data-rights-authenticated-application.test.ts#L35-L42:applicationConstructor헬퍼와AuthenticatedApplicationConstructor타입을 제거하고,AuthenticatedDataRightsApplication을 명명 import로 가져오십시오.🤖 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/identity-service/src/data-rights-recent-auth.test.ts` around lines 10 - 16, Replace the dynamic module lookup in apps/identity-service/src/data-rights-recent-auth.test.ts lines 10-16 by directly importing requireRecentAuthentication, and remove recentAuthenticationGate and RecentAuthenticationGate. In apps/identity-service/src/data-rights-authenticated-application.test.ts lines 35-42, directly import AuthenticatedDataRightsApplication and remove applicationConstructor and AuthenticatedApplicationConstructor; update all call sites to use the named imports.apps/identity-service/src/data-rights-authenticated-application.ts (1)
41-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win거부 사유를 구분할 수 있는 오류 타입을 사용하십시오.
인증 실패와 최근 인증 만료는 모두 일반
Error로 전달됩니다. HTTP 경계는 401과 403을 구분해야 합니다. 현재 구조에서는 호출자가 오류 메시지 문자열을 비교해야 합니다. 문자열 비교는 메시지 변경 시 조용히 깨집니다. 전용 오류 클래스를 도입하면 매핑이 안정됩니다.♻️ 제안 변경
+/** Fail-closed error when no authenticated session backs the request. */ +export class AuthenticationRequiredError extends Error { + constructor() { + super('Authentication is required'); + this.name = 'AuthenticationRequiredError'; + } +} + async exportWorkspace(cookieHeader: string | undefined): Promise<unknown> { const session = await this.sessions.introspectSession(cookieHeader); if (session.statusCode !== 200) { - throw new Error('Authentication is required'); + throw new AuthenticationRequiredError(); }🤖 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/identity-service/src/data-rights-authenticated-application.ts` around lines 41 - 50, Replace the generic Error paths in exportWorkspace and requireRecentAuthentication with dedicated error classes that distinguish authentication failure from recent-authentication expiration. Preserve the existing rejection conditions and messages while exposing stable error types for the HTTP boundary to map authentication failures to 401 and expired recent authentication to 403, without requiring message comparisons.apps/identity-service/migrations/0006_data_rights_request_ledger.sql (1)
36-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win트리거 함수에 계약 설명 주석을 추가하고 DELETE 보호 여부를 명시하십시오.
identity.preserve_completed_data_rights_receipt()는 완료된 행의 단말 컬럼 변경을 예외 없이 조용히 폐기합니다. 호출자는rowCount = 0으로만 이 동작을 인지합니다. 새 기여자가 구현을 읽지 않고 계약을 이해하려면 함수 상단에 설명 주석이 필요합니다. 코딩 가이드라인은 프로덕션 선언에 설명 문서를 요구합니다.또한 이 트리거는 UPDATE만 가드합니다. 완료된 영수증의 DELETE는 차단되지 않습니다. 감사 증거 보존이 목표라면 DELETE 가드 또는 역할 기반 권한 제한을 함께 문서화하십시오.
As per coding guidelines: "Production declarations must include explanatory docstrings sufficient for a new contributor to understand the contract without reconstructing the implementation."
♻️ 제안 변경
+-- Preserves terminal data-rights evidence. When a row is already 'completed', +-- any update that would change request_status, receipt_digest, or completed_at +-- is suppressed by returning NULL. The caller observes rowCount = 0 and maps it +-- to a stable domain conflict. Updates that leave terminal columns unchanged +-- proceed unchanged. DELETE is not guarded by this trigger. CREATE FUNCTION identity.preserve_completed_data_rights_receipt() RETURNS trigger LANGUAGE plpgsql AS $$🤖 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/identity-service/migrations/0006_data_rights_request_ledger.sql` around lines 36 - 56, Add a descriptive SQL doc comment immediately above identity.preserve_completed_data_rights_receipt() documenting that updates to terminal columns on completed rows are silently ignored and that the trigger covers UPDATE only. Explicitly document the chosen DELETE protection contract—either add a DELETE guard or state that deletion is enforced through role-based permissions—and ensure the documentation matches the implemented behavior.Source: Coding guidelines
apps/planning-service/src/postgres-today-repository-input.test.ts (1)
29-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value이 파일의 두 테스트는 기존 테스트와 중복되며 검증 강도가 낮습니다.
apps/planning-service/src/postgres-today-repository.test.ts의rejects malformed lookup identifiers before PostgreSQL sees them테스트가 같은 두 입력('not-a-uuid','2026-02-30')을 이미 다루고,TodayPersistenceError타입까지 확인합니다. 여기서는rejects.toThrow()만 사용하므로 임의의 오류도 통과합니다.이 파일을 삭제하거나, 최소한
rejects.toBeInstanceOf(TodayPersistenceError)로 강화하십시오.🤖 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/planning-service/src/postgres-today-repository-input.test.ts` around lines 29 - 45, Remove the duplicate lookup-scope tests in the postgres-today-repository-input test file, since the existing rejects malformed lookup identifiers before PostgreSQL sees them test already covers both inputs and validates TodayPersistenceError. If retaining these tests, strengthen both rejection assertions to require TodayPersistenceError instead of accepting any thrown error.apps/planning-service/src/planning-runtime.test.ts (1)
17-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
failCommit옵션을 사용하는 테스트가 없습니다.
FakePlanningPool.connect는 항상 인자 없이FakePlanningConnection을 생성합니다. 따라서 COMMIT 실패 경로는 실행되지 않습니다. COMMIT 실패 시ROLLBACK이 실행되고 연결이 파기되지 않는지 검증하는 테스트를 추가하거나, 이 옵션을 제거하십시오.🤖 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/planning-service/src/planning-runtime.test.ts` around lines 17 - 34, FakePlanningPool.connect and the existing planning-runtime tests never exercise FakePlanningConnection’s failCommit option. Add a test that configures the fake connection to fail on COMMIT, verifies ROLLBACK is issued, and confirms the connection is not destroyed; alternatively remove failCommit and its unused branch if this scenario is not required.apps/planning-service/src/postgres-today-repository.test.ts (1)
68-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win행 fixture가
local_date를 문자열로만 제공합니다.
pg는date컬럼을Date객체로 반환합니다. 현재 fixture는 문자열만 사용하므로canonicalTodayDate의Date분기가 검증되지 않습니다. 이 분기는apps/planning-service/src/today-invariants.ts의 시간대 문제와 직접 연결됩니다.new Date(2026, 7, 9)형태의local_date를 사용하는 케이스를 추가하십시오.🤖 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/planning-service/src/postgres-today-repository.test.ts` around lines 68 - 80, Update the tests using aggregateRow to include a case whose local_date is a Date instance created with new Date(2026, 7, 9), rather than relying only on the string-based DATE fixture. Ensure this case exercises canonicalTodayDate’s Date handling while preserving the existing string-date coverage.apps/web/app/components/today-workspace-sync-panel.tsx (1)
65-66: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win렌더 중 ref를 쓰는 대신 커밋 이후에 갱신하십시오.
동시 렌더링에서
currentDraft.current = draft를 렌더 중에 mutating하면 중단된 렌더가 다시 실행될 때 이전ref.current가 남을 수 있습니다.useEffect에서 갱신하면 커밋된 값만 저장됩니다.♻️ 제안 수정
-import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; ... const currentDraft = useRef(draft); - currentDraft.current = draft; + useEffect(() => { + currentDraft.current = draft; + }, [draft]);🤖 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/web/app/components/today-workspace-sync-panel.tsx` around lines 65 - 66, Update the currentDraft ref synchronization so it no longer mutates currentDraft.current during render; move the draft assignment into a useEffect that runs after commit and depends on draft, preserving the ref’s role for accessing the latest committed draft.
🤖 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/identity-service/src/data-rights-authenticated-application.test.ts`:
- Around line 44-82: Extend the AuthenticatedDataRightsApplication tests with
failure cases for introspectSession responses where statusCode is not 200 and
where the authenticated session is expired beyond maximumAgeMs. Assert each
exportWorkspace call rejects with the expected authentication error and verify
dataRights.exportWorkspace is never invoked, while preserving the existing
successful-session assertions.
In `@apps/identity-service/src/data-rights-recent-auth.test.ts`:
- Around line 43-74: Extend the `fails closed on future, malformed, or invalid
policy timestamps` test to include a valid, non-canonical ISO timestamp without
milliseconds, such as the same historical instant as an existing case. Assert
that `requireRecentAuthentication` throws `Authentication provenance is
invalid`, isolating the `canonicalAuthenticatedAt !== input.authenticatedAt`
branch.
In `@apps/identity-service/src/data-rights-request-ledger.integration.test.ts`:
- Around line 13-21: Protect the fixed TEST_DATABASE_NAME lifecycle with a
PostgreSQL advisory lock, following the session-authentication migration
integration test pattern. In beforeAll, acquire the lock through a dedicated
PoolClient using pg_advisory_lock before DROP_TEST_DATABASE and
CREATE_TEST_DATABASE, and keep that client/session for the database-management
work and connection startup; do not acquire the lock via Pool.query, and release
the client during teardown.
- Around line 214-239: Update the test “enforces request kind, digest,
completion consistency, receipt digest, and time ordering constraints” so each
invalid row receives fresh UUID identifiers instead of reusing base, preventing
primary-key conflicts from masking CHECK violations. Assert that each insert
fails because of the expected named constraint, rather than only asserting a
generic throw, and map each invalid case to its corresponding constraint.
In `@apps/identity-service/src/data-rights-request-ledger.test.ts`:
- Around line 211-225: Extend the data-rights ledger tests around
PostgresDataRightsRequestLedger to cover malformed persisted rows that must
reject with DataRightsRequestPersistenceError, including completed status with
null receipt_digest, an invalid request_id UUID, and completed_at earlier than
requested_at. Assert each case fails closed and add
DataRightsRequestPersistenceError to the test imports while preserving full
coverage.
In `@apps/identity-service/src/data-rights-request-ledger.ts`:
- Around line 136-147: Update parseInstant to validate Date instances before
calling toISOString(), ensuring invalid dates invoke the provided invalid
callback and produce the domain persistence error. Preserve the existing string
validation and canonical timestamp checks for valid Date and string inputs.
In `@apps/planning-service/src/planning-runtime.ts`:
- Around line 19-34: 문서화되지 않은 공개 계약인 PlanningPoolConnection과 PlanningPool에 설명용
docstring을 추가하십시오. PlanningPoolConnection에서는 release(destroy?: boolean)의 연결 반환 및
destroy 동작을 설명하고, PlanningPool의 connect()에서는 호출자가 반환된 연결의 소유권을 가지며 반드시 release를
호출해야 한다는 규칙을 명시하십시오. 같은 파일의 PlanningSqlClient 문서화 스타일을 따르십시오.
- Around line 95-115: Update transaction() to apply transaction-local lock and
statement timeouts immediately after BEGIN and before operation() runs. Use SET
LOCAL lock_timeout and SET LOCAL statement_timeout with the service’s
established timeout values, preserving rollback and connection cleanup so
timeout failures propagate as retryable errors.
In `@apps/planning-service/src/today-invariants.test.ts`:
- Around line 15-65: Expand the tests around canonicalTodayUuidV4,
canonicalTodayDate, and canonicalTodayDraft to cover each missing invariant
failure: duplicate action IDs and priorities, overlapping open schedules, more
than MAXIMUM_ACTIONS actions, invalid startMinute/durationMinutes combinations
and day-boundary overflow, inconsistent done/completedAt states, title control
characters and code-point/byte limits, and unexpected keys on drafts or actions.
Also add coverage for canonicalTodayDate when given a Date object, asserting
both valid normalization and invalid-date rejection while preserving the
existing success cases.
In `@apps/planning-service/src/today-invariants.ts`:
- Around line 48-66: Update canonicalTodayDate in
apps/planning-service/src/today-invariants.ts:48-66 to derive Date-object values
with getFullYear, getMonth, and getDate instead of converting through
toISOString, preserving the local calendar date. Add an aggregateRow case using
a Date local_date in
apps/planning-service/src/postgres-today-repository.test.ts:68-80 and verify it
passes under a non-UTC TZ.
In `@apps/planning-service/tests/postgres-today-lock-order.integration.test.ts`:
- Around line 105-111: Update the cleanup query in the cleanups array to use
PostgreSQL’s forced database drop option, adding WITH (FORCE) to DROP DATABASE
IF EXISTS for life_os_today_lock_test. Keep the existing cleanup order and
connection-closing callbacks unchanged.
- Around line 119-134: Update the cleanup handling in the finally block of the
Today lock integration test so it only records cleanup failures and never throws
there; move propagation of cleanupError outside the finally block while
preserving the existing primaryFailure cause attachment and behavior when no
primary failure exists.
In `@docs/superpowers/plans/2026-08-04-data-rights-orchestration-slice.md`:
- Around line 40-47: Add an APA 7 references section to the plan document
supporting the retention/expiry, legal-hold, and backup-expiry policy claims in
items 3 and 5. Cite authoritative regulatory or standards publications, clearly
label any drafts or preprints as such, and distinguish them from official final
publications.
---
Outside diff comments:
In `@apps/web/app/components/today-workspace-sync-panel.tsx`:
- Around line 171-180: Update the save and use-workspace button rendering in the
component containing saveLocal and useWorkspace so the buttons remain mounted
while unavailable, using disabled state instead of conditional unmounting.
Preserve the existing visibility conditions where appropriate, and disable the
save button when canSave is false and the workspace button when it is not
actionable, matching the existing lookup-button pattern.
---
Nitpick comments:
In `@apps/identity-service/migrations/0006_data_rights_request_ledger.sql`:
- Around line 36-56: Add a descriptive SQL doc comment immediately above
identity.preserve_completed_data_rights_receipt() documenting that updates to
terminal columns on completed rows are silently ignored and that the trigger
covers UPDATE only. Explicitly document the chosen DELETE protection
contract—either add a DELETE guard or state that deletion is enforced through
role-based permissions—and ensure the documentation matches the implemented
behavior.
In `@apps/identity-service/src/data-rights-authenticated-application.ts`:
- Around line 41-50: Replace the generic Error paths in exportWorkspace and
requireRecentAuthentication with dedicated error classes that distinguish
authentication failure from recent-authentication expiration. Preserve the
existing rejection conditions and messages while exposing stable error types for
the HTTP boundary to map authentication failures to 401 and expired recent
authentication to 403, without requiring message comparisons.
In `@apps/identity-service/src/data-rights-recent-auth.test.ts`:
- Around line 10-16: Replace the dynamic module lookup in
apps/identity-service/src/data-rights-recent-auth.test.ts lines 10-16 by
directly importing requireRecentAuthentication, and remove
recentAuthenticationGate and RecentAuthenticationGate. In
apps/identity-service/src/data-rights-authenticated-application.test.ts lines
35-42, directly import AuthenticatedDataRightsApplication and remove
applicationConstructor and AuthenticatedApplicationConstructor; update all call
sites to use the named imports.
In `@apps/identity-service/src/oauth-http-boundary.ts`:
- Around line 94-101: Move requireRecentAuthentication into a dedicated
recent-authentication module, such as recent-authentication.ts, and update
data-rights-authenticated-application.ts and its tests to import the symbol from
the new module. Preserve the function signature and behavior while removing its
ownership from oauth-http-boundary.
In
`@apps/identity-service/tests/session-authentication-migration.integration.test.ts`:
- Around line 86-99: Update the DROP DATABASE statement in the cleanup block
guarded by lockHeld and adminClient to use the TEMPORARY_DATABASE_NAME constant
instead of duplicating the literal database name. Validate or safely interpolate
the identifier following the existing pattern in the data-rights request ledger
integration test, while preserving the current DROP, unlock, release, and pool
shutdown order.
In `@apps/planning-service/src/planning-runtime.test.ts`:
- Around line 17-34: FakePlanningPool.connect and the existing planning-runtime
tests never exercise FakePlanningConnection’s failCommit option. Add a test that
configures the fake connection to fail on COMMIT, verifies ROLLBACK is issued,
and confirms the connection is not destroyed; alternatively remove failCommit
and its unused branch if this scenario is not required.
In `@apps/planning-service/src/postgres-today-repository-input.test.ts`:
- Around line 29-45: Remove the duplicate lookup-scope tests in the
postgres-today-repository-input test file, since the existing rejects malformed
lookup identifiers before PostgreSQL sees them test already covers both inputs
and validates TodayPersistenceError. If retaining these tests, strengthen both
rejection assertions to require TodayPersistenceError instead of accepting any
thrown error.
In `@apps/planning-service/src/postgres-today-repository.test.ts`:
- Around line 68-80: Update the tests using aggregateRow to include a case whose
local_date is a Date instance created with new Date(2026, 7, 9), rather than
relying only on the string-based DATE fixture. Ensure this case exercises
canonicalTodayDate’s Date handling while preserving the existing string-date
coverage.
In `@apps/web/app/components/today-workspace-sync-panel.tsx`:
- Around line 65-66: Update the currentDraft ref synchronization so it no longer
mutates currentDraft.current during render; move the draft assignment into a
useEffect that runs after commit and depends on draft, preserving the ref’s role
for accessing the latest committed draft.
🪄 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: bd3d326a-b9a6-42d3-bccf-953a6523c41d
📒 Files selected for processing (43)
.github/workflows/ci.ymlCHANGELOG.mdapps/identity-service/migrations/0006_data_rights_request_ledger.sqlapps/identity-service/src/data-rights-authenticated-application.test.tsapps/identity-service/src/data-rights-authenticated-application.tsapps/identity-service/src/data-rights-recent-auth.test.tsapps/identity-service/src/data-rights-request-ledger.integration.test.tsapps/identity-service/src/data-rights-request-ledger.test.tsapps/identity-service/src/data-rights-request-ledger.tsapps/identity-service/src/oauth-http-boundary.tsapps/identity-service/tests/session-authentication-migration.integration.test.tsapps/planning-service/migrations/0003_durable_today_sync.sqlapps/planning-service/src/planning-runtime.test.tsapps/planning-service/src/planning-runtime.tsapps/planning-service/src/postgres-today-repository-input.test.tsapps/planning-service/src/postgres-today-repository.test.tsapps/planning-service/src/postgres-today-repository.tsapps/planning-service/src/today-http.test.tsapps/planning-service/src/today-http.tsapps/planning-service/src/today-invariants.test.tsapps/planning-service/src/today-invariants.tsapps/planning-service/src/today-sync.tsapps/planning-service/tests/postgres-today-lock-order.integration.test.tsapps/web/app/components/today-workspace-sync-panel.tsxapps/web/app/offline/page.tsxapps/web/app/onboarding/onboarding-flow.tsxapps/web/app/styles.cssapps/web/app/today-state.test.tsapps/web/app/today-state.tsapps/web/app/today-sync-client-review-regression.test.tsapps/web/app/today-sync-client.test.tsapps/web/app/today-sync-client.tsapps/web/app/today-workspace-sync.tsapps/web/e2e/mobile-pwa.spec.tsapps/web/e2e/onboarding.spec.tsapps/web/e2e/today-flow.spec.tsapps/web/e2e/today-save-race.spec.tsapps/web/messages/ko.jsonapps/web/package.jsondocs/research/2026-08-09-durable-today-sync-standards.mddocs/superpowers/plans/2026-08-04-data-rights-orchestration-slice.mddocs/superpowers/plans/2026-08-09-durable-today-workspace-sync.mdpackages/commercial-readiness/src/workflow-contract.test.mjs
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web/package.json
- apps/planning-service/src/today-http.test.ts
- apps/web/messages/ko.json
- apps/web/e2e/today-flow.spec.ts
- docs/superpowers/plans/2026-08-09-durable-today-workspace-sync.md
- apps/planning-service/migrations/0003_durable_today_sync.sql
- docs/research/2026-08-09-durable-today-sync-standards.md
- apps/web/app/today-sync-client.ts
| describe('AuthenticatedDataRightsApplication', () => { | ||
| it('derives export ownership only from the authenticated recent session', async () => { | ||
| const AuthenticatedDataRightsApplication = await applicationConstructor(); | ||
| const contexts: unknown[] = []; | ||
| const sessions = { | ||
| async introspectSession(cookieHeader: string | undefined) { | ||
| expect(cookieHeader).toBe('life_os_session=opaque-session'); | ||
| return { statusCode: 200 as const, body: SESSION_BODY }; | ||
| }, | ||
| }; | ||
| const dataRights = { | ||
| async exportWorkspace(context: { | ||
| readonly workspaceId: string; | ||
| readonly actorUserId: string; | ||
| }) { | ||
| contexts.push(context); | ||
| return { schemaVersion: 'life-os.data-export.v1' }; | ||
| }, | ||
| }; | ||
| const application = new AuthenticatedDataRightsApplication( | ||
| sessions, | ||
| dataRights, | ||
| { | ||
| now: () => new Date('2026-08-09T18:00:00.000Z'), | ||
| maximumAgeMs: 10 * 60 * 1000, | ||
| }, | ||
| ); | ||
|
|
||
| await expect( | ||
| application.exportWorkspace('life_os_session=opaque-session'), | ||
| ).resolves.toEqual({ schemaVersion: 'life-os.data-export.v1' }); | ||
| expect(contexts).toEqual([ | ||
| { | ||
| workspaceId: SESSION_BODY.workspaceId, | ||
| actorUserId: SESSION_BODY.userId, | ||
| }, | ||
| ]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
실패 경로 테스트를 추가하십시오.
이 파일은 성공 경로만 검증합니다. session.statusCode !== 200 분기와 오래된 인증 거부 분기는 실행되지 않습니다. 두 분기는 이 경계의 보안 계약입니다. 또한 인증이 실패하면 dataRights.exportWorkspace가 호출되지 않아야 한다는 점도 검증해야 합니다.
As per coding guidelines: "Tests must prove realistic domain accuracy and failure behavior, not only mocked call counts." 및 "Packages that enforce coverage gates must retain 100% statement, branch, function, and line coverage."
💚 추가 테스트 제안
+ it('rejects an unauthenticated session before any data-rights work', async () => {
+ const AuthenticatedDataRightsApplication = await applicationConstructor();
+ const contexts: unknown[] = [];
+ const application = new AuthenticatedDataRightsApplication(
+ {
+ async introspectSession() {
+ return { statusCode: 401 as unknown as 200, body: SESSION_BODY };
+ },
+ },
+ {
+ async exportWorkspace(context: {
+ readonly workspaceId: string;
+ readonly actorUserId: string;
+ }) {
+ contexts.push(context);
+ return {};
+ },
+ },
+ {
+ now: () => new Date('2026-08-09T18:00:00.000Z'),
+ maximumAgeMs: 10 * 60 * 1000,
+ },
+ );
+
+ await expect(
+ application.exportWorkspace('life_os_session=opaque-session'),
+ ).rejects.toThrow('Authentication is required');
+ expect(contexts).toEqual([]);
+ });
+
+ it('rejects a stale authentication instant before any data-rights work', async () => {
+ const AuthenticatedDataRightsApplication = await applicationConstructor();
+ const contexts: unknown[] = [];
+ const application = new AuthenticatedDataRightsApplication(
+ {
+ async introspectSession() {
+ return { statusCode: 200 as const, body: SESSION_BODY };
+ },
+ },
+ {
+ async exportWorkspace(context: {
+ readonly workspaceId: string;
+ readonly actorUserId: string;
+ }) {
+ contexts.push(context);
+ return {};
+ },
+ },
+ {
+ now: () => new Date('2026-08-09T19:00:00.000Z'),
+ maximumAgeMs: 10 * 60 * 1000,
+ },
+ );
+
+ await expect(
+ application.exportWorkspace('life_os_session=opaque-session'),
+ ).rejects.toThrow('Recent authentication is required');
+ expect(contexts).toEqual([]);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('AuthenticatedDataRightsApplication', () => { | |
| it('derives export ownership only from the authenticated recent session', async () => { | |
| const AuthenticatedDataRightsApplication = await applicationConstructor(); | |
| const contexts: unknown[] = []; | |
| const sessions = { | |
| async introspectSession(cookieHeader: string | undefined) { | |
| expect(cookieHeader).toBe('life_os_session=opaque-session'); | |
| return { statusCode: 200 as const, body: SESSION_BODY }; | |
| }, | |
| }; | |
| const dataRights = { | |
| async exportWorkspace(context: { | |
| readonly workspaceId: string; | |
| readonly actorUserId: string; | |
| }) { | |
| contexts.push(context); | |
| return { schemaVersion: 'life-os.data-export.v1' }; | |
| }, | |
| }; | |
| const application = new AuthenticatedDataRightsApplication( | |
| sessions, | |
| dataRights, | |
| { | |
| now: () => new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }, | |
| ); | |
| await expect( | |
| application.exportWorkspace('life_os_session=opaque-session'), | |
| ).resolves.toEqual({ schemaVersion: 'life-os.data-export.v1' }); | |
| expect(contexts).toEqual([ | |
| { | |
| workspaceId: SESSION_BODY.workspaceId, | |
| actorUserId: SESSION_BODY.userId, | |
| }, | |
| ]); | |
| }); | |
| }); | |
| describe('AuthenticatedDataRightsApplication', () => { | |
| it('derives export ownership only from the authenticated recent session', async () => { | |
| const AuthenticatedDataRightsApplication = await applicationConstructor(); | |
| const contexts: unknown[] = []; | |
| const sessions = { | |
| async introspectSession(cookieHeader: string | undefined) { | |
| expect(cookieHeader).toBe('life_os_session=opaque-session'); | |
| return { statusCode: 200 as const, body: SESSION_BODY }; | |
| }, | |
| }; | |
| const dataRights = { | |
| async exportWorkspace(context: { | |
| readonly workspaceId: string; | |
| readonly actorUserId: string; | |
| }) { | |
| contexts.push(context); | |
| return { schemaVersion: 'life-os.data-export.v1' }; | |
| }, | |
| }; | |
| const application = new AuthenticatedDataRightsApplication( | |
| sessions, | |
| dataRights, | |
| { | |
| now: () => new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }, | |
| ); | |
| await expect( | |
| application.exportWorkspace('life_os_session=opaque-session'), | |
| ).resolves.toEqual({ schemaVersion: 'life-os.data-export.v1' }); | |
| expect(contexts).toEqual([ | |
| { | |
| workspaceId: SESSION_BODY.workspaceId, | |
| actorUserId: SESSION_BODY.userId, | |
| }, | |
| ]); | |
| }); | |
| it('rejects an unauthenticated session before any data-rights work', async () => { | |
| const AuthenticatedDataRightsApplication = await applicationConstructor(); | |
| const contexts: unknown[] = []; | |
| const application = new AuthenticatedDataRightsApplication( | |
| { | |
| async introspectSession() { | |
| return { statusCode: 401 as unknown as 200, body: SESSION_BODY }; | |
| }, | |
| }, | |
| { | |
| async exportWorkspace(context: { | |
| readonly workspaceId: string; | |
| readonly actorUserId: string; | |
| }) { | |
| contexts.push(context); | |
| return {}; | |
| }, | |
| }, | |
| { | |
| now: () => new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }, | |
| ); | |
| await expect( | |
| application.exportWorkspace('life_os_session=opaque-session'), | |
| ).rejects.toThrow('Authentication is required'); | |
| expect(contexts).toEqual([]); | |
| }); | |
| it('rejects a stale authentication instant before any data-rights work', async () => { | |
| const AuthenticatedDataRightsApplication = await applicationConstructor(); | |
| const contexts: unknown[] = []; | |
| const application = new AuthenticatedDataRightsApplication( | |
| { | |
| async introspectSession() { | |
| return { statusCode: 200 as const, body: SESSION_BODY }; | |
| }, | |
| }, | |
| { | |
| async exportWorkspace(context: { | |
| readonly workspaceId: string; | |
| readonly actorUserId: string; | |
| }) { | |
| contexts.push(context); | |
| return {}; | |
| }, | |
| }, | |
| { | |
| now: () => new Date('2026-08-09T19:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }, | |
| ); | |
| await expect( | |
| application.exportWorkspace('life_os_session=opaque-session'), | |
| ).rejects.toThrow('Recent authentication is required'); | |
| expect(contexts).toEqual([]); | |
| }); | |
| }); |
🤖 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/identity-service/src/data-rights-authenticated-application.test.ts`
around lines 44 - 82, Extend the AuthenticatedDataRightsApplication tests with
failure cases for introspectSession responses where statusCode is not 200 and
where the authenticated session is expired beyond maximumAgeMs. Assert each
exportWorkspace call rejects with the expected authentication error and verify
dataRights.exportWorkspace is never invoked, while preserving the existing
successful-session assertions.
Source: Coding guidelines
| it('fails closed on future, malformed, or invalid policy timestamps', () => { | ||
| const requireRecentAuthentication = recentAuthenticationGate(); | ||
|
|
||
| expect(() => | ||
| requireRecentAuthentication({ | ||
| authenticatedAt: '2026-08-09T18:00:00.001Z', | ||
| now: new Date('2026-08-09T18:00:00.000Z'), | ||
| maximumAgeMs: 10 * 60 * 1000, | ||
| }), | ||
| ).toThrow('Authentication provenance is invalid'); | ||
| expect(() => | ||
| requireRecentAuthentication({ | ||
| authenticatedAt: 'not-an-instant', | ||
| now: new Date('2026-08-09T18:00:00.000Z'), | ||
| maximumAgeMs: 10 * 60 * 1000, | ||
| }), | ||
| ).toThrow('Authentication provenance is invalid'); | ||
| expect(() => | ||
| requireRecentAuthentication({ | ||
| authenticatedAt: '2026-08-09T17:55:00.000Z', | ||
| now: new Date('invalid'), | ||
| maximumAgeMs: 10 * 60 * 1000, | ||
| }), | ||
| ).toThrow('Recent authentication policy is invalid'); | ||
| expect(() => | ||
| requireRecentAuthentication({ | ||
| authenticatedAt: '2026-08-09T17:55:00.000Z', | ||
| now: new Date('2026-08-09T18:00:00.000Z'), | ||
| maximumAgeMs: 0, | ||
| }), | ||
| ).toThrow('Recent authentication policy is invalid'); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
정규화되지 않은 시각 문자열 사례를 추가하십시오.
requireRecentAuthentication은 canonicalAuthenticatedAt !== input.authenticatedAt 조건으로 비정규 ISO 문자열을 거부합니다. 현재 테스트는 이 조건을 단독으로 실행하지 않습니다. 48행 값은 정규 형식이면서 미래 시각이므로 조건의 다른 절을 통과합니다. 밀리초가 없는 값 하나를 추가하면 이 분기가 검증됩니다.
💚 추가 사례 제안
expect(() =>
requireRecentAuthentication({
authenticatedAt: 'not-an-instant',
now: new Date('2026-08-09T18:00:00.000Z'),
maximumAgeMs: 10 * 60 * 1000,
}),
).toThrow('Authentication provenance is invalid');
+ expect(() =>
+ requireRecentAuthentication({
+ authenticatedAt: '2026-08-09T17:55:00Z',
+ now: new Date('2026-08-09T18:00:00.000Z'),
+ maximumAgeMs: 10 * 60 * 1000,
+ }),
+ ).toThrow('Authentication provenance is invalid');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('fails closed on future, malformed, or invalid policy timestamps', () => { | |
| const requireRecentAuthentication = recentAuthenticationGate(); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T18:00:00.001Z', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Authentication provenance is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: 'not-an-instant', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Authentication provenance is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T17:55:00.000Z', | |
| now: new Date('invalid'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Recent authentication policy is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T17:55:00.000Z', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 0, | |
| }), | |
| ).toThrow('Recent authentication policy is invalid'); | |
| }); | |
| it('fails closed on future, malformed, or invalid policy timestamps', () => { | |
| const requireRecentAuthentication = recentAuthenticationGate(); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T18:00:00.001Z', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Authentication provenance is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: 'not-an-instant', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Authentication provenance is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T17:55:00Z', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Authentication provenance is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T17:55:00.000Z', | |
| now: new Date('invalid'), | |
| maximumAgeMs: 10 * 60 * 1000, | |
| }), | |
| ).toThrow('Recent authentication policy is invalid'); | |
| expect(() => | |
| requireRecentAuthentication({ | |
| authenticatedAt: '2026-08-09T17:55:00.000Z', | |
| now: new Date('2026-08-09T18:00:00.000Z'), | |
| maximumAgeMs: 0, | |
| }), | |
| ).toThrow('Recent authentication policy is invalid'); | |
| }); |
🤖 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/identity-service/src/data-rights-recent-auth.test.ts` around lines 43 -
74, Extend the `fails closed on future, malformed, or invalid policy timestamps`
test to include a valid, non-canonical ISO timestamp without milliseconds, such
as the same historical instant as an existing case. Assert that
`requireRecentAuthentication` throws `Authentication provenance is invalid`,
isolating the `canonicalAuthenticatedAt !== input.authenticatedAt` branch.
Source: Coding guidelines
| const DATABASE_URL = process.env.IDENTITY_DATABASE_URL; | ||
| const describeWithDatabase = DATABASE_URL ? describe : describe.skip; | ||
| const TEST_DATABASE_NAME = 'life_os_data_rights_ledger_test'; | ||
| if (!/^[a-z][a-z0-9_]*$/u.test(TEST_DATABASE_NAME)) { | ||
| throw new Error('TEST_DATABASE_NAME must be a safe PostgreSQL identifier'); | ||
| } | ||
| const DROP_TEST_DATABASE = `DROP DATABASE IF EXISTS "${TEST_DATABASE_NAME}" WITH (FORCE)`; | ||
| const CREATE_TEST_DATABASE = `CREATE DATABASE "${TEST_DATABASE_NAME}"`; | ||
| const MIGRATION_DIRECTORY = resolve(__dirname, '../migrations'); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check Vitest config, module type, and existing advisory-lock patterns.
set -euo pipefail
fd -t f 'package.json' apps/identity-service --exec jq '{type, scripts}' {}
fd -t f 'vitest.config.*' apps/identity-service
fd -t f 'vitest.config.*' apps/identity-service --exec cat {}
rg -n --type=ts -C 3 'pg_advisory_lock|__dirname|import\.meta\.url' apps/identity-serviceRepository: ContextualWisdomLab/life-os
Length of output: 2030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files apps/identity-service/vitest.config.* apps/identity-service package.json apps/identity-service/src/data-rights-request-ledger.integration.test.ts apps/identity-service/tests/session-authentication-migration.integration.test.ts
echo
echo "== tsconfig package/module hints =="
fd -t f 'tsconfig.*' apps/identity-service --exec sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}
echo
echo "== data-rights test relevant lines =="
sed -n '1,140p' apps/identity-service/src/data-rights-request-ledger.integration.test.ts
echo
echo "== session test relevant lines =="
sed -n '50,95p' apps/identity-service/tests/session-authentication-migration.integration.test.ts
sed -n '1,60p' apps/identity-service/tests/session-authentication-migration.integration.test.tsRepository: ContextualWisdomLab/life-os
Length of output: 12260
고정 이름 테스트 데이터베이스를 advisory lock으로 보호하십시오.
beforeAll이 고정 name life_os_data_rights_ledger_test를 DROP DATABASE ... WITH (FORCE)하고 다시 만듭니다. 같은 PostgreSQL 인스턴스를 공유하는 실행에서 다른 테스트의 데이터베이스를 잃거나 잡을 수 있습니다. apps/identity-service/tests/session-authentication-migration.integration.test.ts의 패턴처럼 전용 PoolClient에서 pg_advisory_lock(...)로 잠금 후 관리 데이터베이스 작업과 연결을 시작하십시오. pg_advisory_lock은 세션 범위이므로 Pool.query(...)가 아니라 연결된 client에서 실행해야 합니다.
🤖 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/identity-service/src/data-rights-request-ledger.integration.test.ts`
around lines 13 - 21, Protect the fixed TEST_DATABASE_NAME lifecycle with a
PostgreSQL advisory lock, following the session-authentication migration
integration test pattern. In beforeAll, acquire the lock through a dedicated
PoolClient using pg_advisory_lock before DROP_TEST_DATABASE and
CREATE_TEST_DATABASE, and keep that client/session for the database-management
work and connection startup; do not acquire the lock via Pool.query, and release
the client during teardown.
| it('enforces request kind, digest, completion consistency, receipt digest, and time ordering constraints', async () => { | ||
| const base = [randomUUID(), randomUUID(), randomUUID(), randomUUID()]; | ||
| const invalidRows: ReadonlyArray<readonly unknown[]> = [ | ||
| [...base, 'invalid-kind', 'a'.repeat(64), 'pending', null, '2026-08-09T20:00:00.000Z', null], | ||
| [...base, 'export', 'not-a-digest', 'pending', null, '2026-08-09T20:00:00.000Z', null], | ||
| [...base, 'export', 'a'.repeat(64), 'completed', null, '2026-08-09T20:00:00.000Z', '2026-08-09T20:01:00.000Z'], | ||
| [...base, 'export', 'a'.repeat(64), 'completed', 'not-a-digest', '2026-08-09T20:00:00.000Z', '2026-08-09T20:01:00.000Z'], | ||
| [...base, 'export', 'a'.repeat(64), 'completed', 'b'.repeat(64), '2026-08-09T20:02:00.000Z', '2026-08-09T20:01:00.000Z'], | ||
| ]; | ||
|
|
||
| for (const values of invalidRows) { | ||
| await expect( | ||
| pool.query( | ||
| `INSERT INTO identity.data_rights_requests ( | ||
| request_id, workspace_id, requested_by_user_id, idempotency_key, | ||
| request_kind, request_digest, request_status, receipt_digest, | ||
| requested_at, completed_at | ||
| ) VALUES ( | ||
| $1::uuid, $2::uuid, $3::uuid, $4::uuid, | ||
| $5, $6, $7, $8, $9::timestamptz, $10::timestamptz | ||
| )`, | ||
| [...values], | ||
| ), | ||
| ).rejects.toThrow(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
어떤 제약이 거부했는지 단언하십시오.
다섯 개의 잘못된 행이 같은 base 식별자를 재사용합니다. 어떤 CHECK 제약이 동작하지 않으면 그 행이 삽입됩니다. 이후 행은 기본 키 위반으로 실패하므로, .rejects.toThrow()는 여전히 통과합니다. 결함이 감춰집니다. 각 행마다 새 식별자를 만들고, 기대하는 제약 이름을 단언하십시오.
As per coding guidelines: "Tests must model realistic domain outcomes, not only mocked implementation calls."
💚 제안 변경
- const base = [randomUUID(), randomUUID(), randomUUID(), randomUUID()];
- const invalidRows: ReadonlyArray<readonly unknown[]> = [
- [...base, 'invalid-kind', 'a'.repeat(64), 'pending', null, '2026-08-09T20:00:00.000Z', null],
+ const invalidRows: ReadonlyArray<{
+ readonly constraint: string;
+ readonly values: readonly unknown[];
+ }> = [
+ {
+ constraint: 'data_rights_request_kind_valid',
+ values: ['invalid-kind', 'a'.repeat(64), 'pending', null, '2026-08-09T20:00:00.000Z', null],
+ },
// ... 나머지 행도 같은 형태로 제약 이름을 함께 선언하십시오.
];
- for (const values of invalidRows) {
+ for (const invalidRow of invalidRows) {
+ const identifiers = [randomUUID(), randomUUID(), randomUUID(), randomUUID()];
await expect(
pool.query(
`INSERT INTO identity.data_rights_requests (
request_id, workspace_id, requested_by_user_id, idempotency_key,
request_kind, request_digest, request_status, receipt_digest,
requested_at, completed_at
) VALUES (
$1::uuid, $2::uuid, $3::uuid, $4::uuid,
$5, $6, $7, $8, $9::timestamptz, $10::timestamptz
)`,
- [...values],
+ [...identifiers, ...invalidRow.values],
),
- ).rejects.toThrow();
+ ).rejects.toMatchObject({ constraint: invalidRow.constraint });
}🤖 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/identity-service/src/data-rights-request-ledger.integration.test.ts`
around lines 214 - 239, Update the test “enforces request kind, digest,
completion consistency, receipt digest, and time ordering constraints” so each
invalid row receives fresh UUID identifiers instead of reusing base, preventing
primary-key conflicts from masking CHECK violations. Assert that each insert
fails because of the expected named constraint, rather than only asserting a
generic throw, and map each invalid case to its corresponding constraint.
Source: Coding guidelines
| it('rejects malformed ownership, digest, kind, and time before querying PostgreSQL', async () => { | ||
| for (const invalidInput of [ | ||
| beginInput({ workspaceId: 'not-a-uuid' }), | ||
| beginInput({ requestKind: 'delete' }), | ||
| beginInput({ requestDigest: 'not-a-digest' }), | ||
| beginInput({ requestedAt: 'not-an-instant' }), | ||
| ]) { | ||
| const client = new RecordingSqlClient([]); | ||
| const ledger = new PostgresDataRightsRequestLedger(client); | ||
| await expect(ledger.beginRequest(invalidInput as never)).rejects.toBeInstanceOf( | ||
| DataRightsRequestValidationError, | ||
| ); | ||
| expect(client.calls).toHaveLength(0); | ||
| } | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
저장 데이터 손상 경로 테스트를 추가하십시오.
DataRightsRequestPersistenceError를 발생시키는 분기가 검증되지 않습니다. 예: 상태가 completed인데 receipt_digest가 null인 행, UUID 형식이 아닌 request_id, completed_at이 requested_at보다 이른 행. 이 분기는 fail-closed 계약의 핵심입니다. 커버리지 게이트도 이 분기를 요구합니다.
As per coding guidelines: "Packages that enforce coverage gates must retain 100% statement, branch, function, and line coverage."
💚 추가 테스트 제안
+ it('fails closed when persisted evidence violates ledger invariants', async () => {
+ for (const corrupted of [
+ storedRow({ request_status: 'completed', receipt_digest: null }),
+ storedRow({ request_id: 'not-a-uuid' }),
+ storedRow({
+ request_status: 'completed',
+ receipt_digest: RECEIPT_DIGEST,
+ requested_at: COMPLETED_AT,
+ completed_at: REQUESTED_AT,
+ }),
+ ]) {
+ const client = new RecordingSqlClient([[corrupted]]);
+ const ledger = new PostgresDataRightsRequestLedger(client);
+ await expect(ledger.beginRequest(beginInput())).rejects.toBeInstanceOf(
+ DataRightsRequestPersistenceError,
+ );
+ }
+ });DataRightsRequestPersistenceError를 import 목록에 추가하십시오.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('rejects malformed ownership, digest, kind, and time before querying PostgreSQL', async () => { | |
| for (const invalidInput of [ | |
| beginInput({ workspaceId: 'not-a-uuid' }), | |
| beginInput({ requestKind: 'delete' }), | |
| beginInput({ requestDigest: 'not-a-digest' }), | |
| beginInput({ requestedAt: 'not-an-instant' }), | |
| ]) { | |
| const client = new RecordingSqlClient([]); | |
| const ledger = new PostgresDataRightsRequestLedger(client); | |
| await expect(ledger.beginRequest(invalidInput as never)).rejects.toBeInstanceOf( | |
| DataRightsRequestValidationError, | |
| ); | |
| expect(client.calls).toHaveLength(0); | |
| } | |
| }); | |
| it('rejects malformed ownership, digest, kind, and time before querying PostgreSQL', async () => { | |
| for (const invalidInput of [ | |
| beginInput({ workspaceId: 'not-a-uuid' }), | |
| beginInput({ requestKind: 'delete' }), | |
| beginInput({ requestDigest: 'not-a-digest' }), | |
| beginInput({ requestedAt: 'not-an-instant' }), | |
| ]) { | |
| const client = new RecordingSqlClient([]); | |
| const ledger = new PostgresDataRightsRequestLedger(client); | |
| await expect(ledger.beginRequest(invalidInput as never)).rejects.toBeInstanceOf( | |
| DataRightsRequestValidationError, | |
| ); | |
| expect(client.calls).toHaveLength(0); | |
| } | |
| }); | |
| it('fails closed when persisted evidence violates ledger invariants', async () => { | |
| for (const corrupted of [ | |
| storedRow({ request_status: 'completed', receipt_digest: null }), | |
| storedRow({ request_id: 'not-a-uuid' }), | |
| storedRow({ | |
| request_status: 'completed', | |
| receipt_digest: RECEIPT_DIGEST, | |
| requested_at: COMPLETED_AT, | |
| completed_at: REQUESTED_AT, | |
| }), | |
| ]) { | |
| const client = new RecordingSqlClient([[corrupted]]); | |
| const ledger = new PostgresDataRightsRequestLedger(client); | |
| await expect(ledger.beginRequest(beginInput())).rejects.toBeInstanceOf( | |
| DataRightsRequestPersistenceError, | |
| ); | |
| } | |
| }); |
🤖 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/identity-service/src/data-rights-request-ledger.test.ts` around lines
211 - 225, Extend the data-rights ledger tests around
PostgresDataRightsRequestLedger to cover malformed persisted rows that must
reject with DataRightsRequestPersistenceError, including completed status with
null receipt_digest, an invalid request_id UUID, and completed_at earlier than
requested_at. Assert each case fails closed and add
DataRightsRequestPersistenceError to the test imports while preserving full
coverage.
Source: Coding guidelines
| describe('shared Today invariants', () => { | ||
| it('canonicalizes shared identifier and calendar rules', () => { | ||
| expect( | ||
| canonicalTodayUuidV4('A0EBC2A3-3D39-4B78-88AF-7F952C9049AD', fail), | ||
| ).toBe('a0ebc2a3-3d39-4b78-88af-7f952c9049ad'); | ||
| expect(canonicalTodayDate('2026-08-10', fail)).toBe('2026-08-10'); | ||
| expect(() => canonicalTodayDate('2026-02-30', fail)).toThrow( | ||
| InvariantFailure, | ||
| ); | ||
| }); | ||
|
|
||
| it('validates the complete draft once for domain and persistence callers', () => { | ||
| const draft = canonicalTodayDraft( | ||
| { | ||
| version: TODAY_VERSION, | ||
| date: '2026-08-10', | ||
| actions: [ | ||
| { | ||
| id: 'f4fd9ff3-d182-4516-a30e-b954c8b44ae2', | ||
| title: ' Finish the review ', | ||
| status: 'open', | ||
| priority: 1, | ||
| startMinute: 540, | ||
| durationMinutes: 30, | ||
| createdAt: '2026-08-09T21:00:00Z', | ||
| completedAt: null, | ||
| }, | ||
| ], | ||
| }, | ||
| fail, | ||
| '2026-08-10', | ||
| ); | ||
|
|
||
| expect(draft).toEqual({ | ||
| version: 'life-os.today.v1', | ||
| date: '2026-08-10', | ||
| actions: [ | ||
| { | ||
| id: 'f4fd9ff3-d182-4516-a30e-b954c8b44ae2', | ||
| title: 'Finish the review', | ||
| status: 'open', | ||
| priority: 1, | ||
| startMinute: 540, | ||
| durationMinutes: 30, | ||
| createdAt: '2026-08-09T21:00:00.000Z', | ||
| completedAt: null, | ||
| }, | ||
| ], | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
핵심 실패 불변식이 검증되지 않습니다.
현재 두 테스트는 성공 경로와 날짜 하나의 실패만 다룹니다. today-invariants.ts의 다음 규칙은 테스트가 없습니다.
- 중복 action
id거부 - 중복
priority거부 - 겹치는
open일정 거부 MAXIMUM_ACTIONS(50) 초과 거부startMinute와durationMinutes의 동시 존재 규칙, 그리고 하루 경계 초과 거부status === 'done'과completedAt의 일관성 규칙- 제목의 제어 문자, 코드 포인트 한계, 바이트 한계 거부
- draft 및 action의 추가 키 거부
canonicalTodayDate의Date객체 분기
이 규칙들이 도메인 정확성의 핵심입니다. 실패 케이스를 추가하십시오.
이는 코딩 가이드라인의 "Tests must prove realistic domain accuracy and failure behavior, not only mocked call counts."와 "Packages that enforce coverage gates must retain 100% statement, branch, function, and line coverage." 항목에 따른 지적입니다.
원하시면 실패 케이스 테스트를 생성해 드리겠습니다.
🤖 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/planning-service/src/today-invariants.test.ts` around lines 15 - 65,
Expand the tests around canonicalTodayUuidV4, canonicalTodayDate, and
canonicalTodayDraft to cover each missing invariant failure: duplicate action
IDs and priorities, overlapping open schedules, more than MAXIMUM_ACTIONS
actions, invalid startMinute/durationMinutes combinations and day-boundary
overflow, inconsistent done/completedAt states, title control characters and
code-point/byte limits, and unexpected keys on drafts or actions. Also add
coverage for canonicalTodayDate when given a Date object, asserting both valid
normalization and invalid-date rejection while preserving the existing success
cases.
Source: Coding guidelines
| export function canonicalTodayDate( | ||
| value: unknown, | ||
| fail: TodayInvariantFailure, | ||
| allowDateObject = false, | ||
| ): string { | ||
| if (allowDateObject && value instanceof Date) { | ||
| if (Number.isNaN(value.getTime())) return fail(); | ||
| return value.toISOString().slice(0, 10); | ||
| } | ||
| if (typeof value !== 'string' || !DATE_PATTERN.test(value)) return fail(); | ||
| const parsed = new Date(`${value}T00:00:00.000Z`); | ||
| if ( | ||
| Number.isNaN(parsed.getTime()) || | ||
| parsed.toISOString().slice(0, 10) !== value | ||
| ) { | ||
| return fail(); | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
canonicalTodayDate의 Date 분기가 시간대에 의존하고, 테스트가 이 분기를 다루지 않습니다. pg는 date 컬럼을 로컬 시간대 자정의 Date로 반환합니다. toISOString().slice(0, 10)은 UTC로 변환하므로, 서버 시간대가 UTC보다 앞서면 날짜가 하루 앞당겨집니다. 그 결과 parseAggregateRow의 날짜 비교가 실패하고 모든 읽기가 TodayPersistenceError가 됩니다. 문자열만 사용하는 테스트 fixture 때문에 이 결함이 드러나지 않습니다.
apps/planning-service/src/today-invariants.ts#L48-L66:Date분기에서getFullYear,getMonth,getDate로 로컬 달력 날짜를 만드십시오. 또는 쿼리에서local_date::text로 선택하십시오.apps/planning-service/src/postgres-today-repository.test.ts#L68-L80:aggregateRow에local_date를Date객체로 제공하는 케이스를 추가하고, 비UTCTZ에서도 통과하는지 확인하십시오.
📍 Affects 2 files
apps/planning-service/src/today-invariants.ts#L48-L66(this comment)apps/planning-service/src/postgres-today-repository.test.ts#L68-L80
🤖 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/planning-service/src/today-invariants.ts` around lines 48 - 66, Update
canonicalTodayDate in apps/planning-service/src/today-invariants.ts:48-66 to
derive Date-object values with getFullYear, getMonth, and getDate instead of
converting through toISOString, preserving the local calendar date. Add an
aggregateRow case using a Date local_date in
apps/planning-service/src/postgres-today-repository.test.ts:68-80 and verify it
passes under a non-UTC TZ.
| const cleanups: Array<() => Promise<unknown>> = [ | ||
| async () => await runtime?.close(), | ||
| async () => await migrationPool?.end(), | ||
| async () => | ||
| await adminPool.query('DROP DATABASE IF EXISTS life_os_today_lock_test'), | ||
| async () => await adminPool.end(), | ||
| ]; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
임시 데이터베이스 삭제가 남은 연결 때문에 실패할 수 있습니다.
DROP DATABASE IF EXISTS life_os_today_lock_test는 다른 세션이 남아 있으면 실패합니다. 앞선 정리 단계가 실패한 경우 연결이 남습니다. 그러면 다음 실행이 이전 데이터로 오염됩니다. WITH (FORCE)를 추가하십시오(PostgreSQL 13 이상).
🛡️ 제안 수정
- await adminPool.query('DROP DATABASE IF EXISTS life_os_today_lock_test'),
+ await adminPool.query(
+ 'DROP DATABASE IF EXISTS life_os_today_lock_test WITH (FORCE)',
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cleanups: Array<() => Promise<unknown>> = [ | |
| async () => await runtime?.close(), | |
| async () => await migrationPool?.end(), | |
| async () => | |
| await adminPool.query('DROP DATABASE IF EXISTS life_os_today_lock_test'), | |
| async () => await adminPool.end(), | |
| ]; | |
| const cleanups: Array<() => Promise<unknown>> = [ | |
| async () => await runtime?.close(), | |
| async () => await migrationPool?.end(), | |
| async () => | |
| await adminPool.query( | |
| 'DROP DATABASE IF EXISTS life_os_today_lock_test WITH (FORCE)', | |
| ), | |
| async () => await adminPool.end(), | |
| ]; |
🤖 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/planning-service/tests/postgres-today-lock-order.integration.test.ts`
around lines 105 - 111, Update the cleanup query in the cleanups array to use
PostgreSQL’s forced database drop option, adding WITH (FORCE) to DROP DATABASE
IF EXISTS for life_os_today_lock_test. Keep the existing cleanup order and
connection-closing callbacks unchanged.
| if (cleanupFailures.length > 0) { | ||
| const cleanupError = new AggregateError( | ||
| cleanupFailures, | ||
| 'Today lock test cleanup failed', | ||
| ); | ||
| if (primaryFailure instanceof Error) { | ||
| if (primaryFailure.cause === undefined) { | ||
| Object.defineProperty(primaryFailure, 'cause', { | ||
| configurable: true, | ||
| value: cleanupError, | ||
| }); | ||
| } | ||
| } else if (primaryFailure === undefined) { | ||
| throw cleanupError; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
finally 블록 안의 throw가 Biome 린트를 위반합니다.
Biome 규칙 lint/correctness/noUnsafeFinally가 132행을 오류로 보고합니다. CI에서 린트가 실패합니다. finally에서는 정리 결과만 기록하고, 실제 오류 발생은 블록 밖으로 옮기십시오.
🛡️ 제안 수정
+ let pendingCleanupError: AggregateError | undefined;
} finally {
...
if (cleanupFailures.length > 0) {
const cleanupError = new AggregateError(
cleanupFailures,
'Today lock test cleanup failed',
);
if (primaryFailure instanceof Error) {
if (primaryFailure.cause === undefined) {
Object.defineProperty(primaryFailure, 'cause', {
configurable: true,
value: cleanupError,
});
}
} else if (primaryFailure === undefined) {
- throw cleanupError;
+ pendingCleanupError = cleanupError;
}
}
}
+ if (pendingCleanupError) throw pendingCleanupError;🧰 Tools
🪛 Biome (2.5.6)
[error] 132-132: Unsafe usage of 'throw'.
(lint/correctness/noUnsafeFinally)
🤖 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/planning-service/tests/postgres-today-lock-order.integration.test.ts`
around lines 119 - 134, Update the cleanup handling in the finally block of the
Today lock integration test so it only records cleanup failures and never throws
there; move propagation of cleanupError outside the finally block while
preserving the existing primaryFailure cause attachment and behavior when no
primary failure exists.
Source: Linters/SAST tools
| 1. Wire the authenticated data-rights application and orchestration flow to the durable ledger and expose bounded status resources without accepting ownership fields in JSON. | ||
| 2. Register concrete identity, planning, habit, AI audit, calendar, review, notification, privacy, and integration contributors. | ||
| 3. Add immutable operational/audit events, bounded retention/expiry, rate limits, and legal-hold decisions around the request lifecycle. | ||
| 4. Stream encrypted exports to object storage with short-lived download authorization, explicit download audit, and retention deletion. | ||
| 5. Define backup-expiry behavior so erased source data cannot be silently reintroduced through unsupported restores. | ||
| 6. Add cross-service recovery drills proving a failed destructive execution can be safely replayed to completion. | ||
|
|
||
| Refs #21 and #58. | ||
| Refs #21, #55, and #58. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
규제 관련 항목에 APA 7 참고문헌을 추가하십시오.
42-44행은 보존/만료, 법적 보류, 백업 만료 정책을 후속 작업으로 선언합니다. 이 항목은 데이터 권리 표준에서 유래합니다. 현재 문서에는 참고문헌이 없습니다. 근거 규정과 출처를 APA 7 형식으로 추가하고, 공식 발행본과 초안·프리프린트를 구분하십시오.
As per coding guidelines: "Standards and research claims must include APA 7 references and distinguish publications from drafts or preprints."
🤖 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 `@docs/superpowers/plans/2026-08-04-data-rights-orchestration-slice.md` around
lines 40 - 47, Add an APA 7 references section to the plan document supporting
the retention/expiry, legal-hold, and backup-expiry policy claims in items 3 and
5. Cite authoritative regulatory or standards publications, clearly label any
drafts or preprints as such, and distinguish them from official final
publications.
Source: Coding guidelines
Buyer-visible outcome
A signed-in user can explicitly move a browser-local Today plan into durable workspace storage, reopen workspace Today on another device, and reconcile stale edits without silently overwriting newer work. Local Today remains local until the user chooses a workspace action.
Implemented on the current branch
life-os.today.v1planning aggregate keyed by authenticated workspace and local dateIf-None-Match: *create and exact strongIf-Matchupdate preconditionsbrowser-acceptancejob so Playwright buyer journeys execute rather than remain unrun fixturesExact-head evidence
For head
8260c6b097f589144f5ce601870979d4f6891621, CI, AppGuardrail, SAST Semgrep, Security Scan, Commercial Readiness, and CodeRabbit status are successful. Current inline review threads are resolved. The PR is Ready for review; predecessor or stale-head evidence is not reused.Remaining before merge
mainimmediately before any mergeImplemented on active PRuntil protected integrationSecurity and privacy invariants
Refs #121 and #21.
Summary by CodeRabbit
새로운 기능
버그 수정
문서
테스트