feat(notification): add service-owned data-rights contributor - #198
Open
seonghobae wants to merge 151 commits into
Open
feat(notification): add service-owned data-rights contributor#198seonghobae wants to merge 151 commits into
seonghobae wants to merge 151 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNotification 서비스에 데이터 권리 contributor와 workspace 데이터 삭제 함수를 추가했습니다. 삭제 영수증, 멱등성, 권한 검증, mutation guard를 구현했습니다. 런타임 연결, 배포 권한 설정, 단위·계약·통합 테스트도 추가했습니다. ChangesNotification 데이터 권리 처리
Estimated code review effort: 5 (Critical) | ~100 minutes Mergeability Score: 🔵 Low · up to The PR’s lint command can modify files outside src while still succeeding, which may create unnoticed repository changes. The PR is mergeable with explicit owner awareness and should follow up by making lint read-only or verifying all modified paths. Sequence Diagram(s)sequenceDiagram
participant Client
participant NotificationRuntime
participant NotificationDataRightsContributor
participant PostgreSQL
Client->>NotificationRuntime: 데이터 권리 요청 전달
NotificationRuntime->>NotificationDataRightsContributor: handle(request) 호출
NotificationDataRightsContributor->>PostgreSQL: export 또는 삭제 요청 실행
PostgreSQL-->>NotificationDataRightsContributor: 데이터 또는 삭제 증거 반환
NotificationDataRightsContributor-->>Client: 데이터 권리 응답 반환
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 |
seonghobae
marked this pull request as ready for review
August 12, 2026 08:43
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Author
|
@coderabbitai review Please review the unchanged exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested_head='b7bec140695a12eb39d537dacb3644581603daf1'
actual_head="$(git rev-parse HEAD)"
printf 'HEAD=%s\nREQUESTED=%s\n' "$actual_head" "$requested_head"
git diff --check "${requested_head}^" "$requested_head"
git diff --name-status "${requested_head}^" "$requested_head"
printf '\n--- notification package lint ---\n'
jq -r '.scripts.lint // empty' apps/notification-service/package.json
printf '\n--- outlines ---\n'
ast-grep outline apps/notification-service/src/notification-data-rights.ts --items all
ast-grep outline apps/notification-service/src/notification-runtime.ts --items all
printf '\n--- migration sections ---\n'
cat -n apps/notification-service/migrations/0002_data_rights_erasure.sql
printf '\n--- contributor sections ---\n'
cat -n apps/notification-service/src/notification-data-rights.ts
printf '\n--- runtime sections ---\n'
cat -n apps/notification-service/src/notification-runtime.ts
printf '\n--- migration runner relevant sections ---\n'
rg -n -C 8 'apply_service_migrations|NOTIFICATION|runtime_role|GRANT|REVOKE' infra/kubernetes/run-migrations.sh
printf '\n--- test assertion map ---\n'
for f in \
apps/notification-service/src/notification-data-rights.behavior.test.ts \
apps/notification-service/src/notification-data-rights.integration.test.ts \
apps/notification-service/src/notification-data-rights-migration.test.ts \
apps/notification-service/src/notification-data-rights.test.ts \
infra/tests/notification-migration-role.spec.ts
do
echo "--- $f ---"
ast-grep outline "$f" --items all
doneLength of output: 80049 🏁 Script executed: #!/bin/bash
set -euo pipefail
head='b7bec140695a12eb39d537dacb3644581603daf1'
base="$(git merge-base "$head" origin/main 2>/dev/null || git merge-base "$head" main)"
printf 'MERGE_BASE=%s\n' "$base"
printf '%s\n' '--- complete PR files ---'
git diff --name-status "$base" "$head"
printf '%s\n' '--- complete PR diff check ---'
git diff --check "$base" "$head"
printf '%s\n' '--- data-rights authority code map ---'
fd -t f 'notification-data-rights' apps/notification-service/src
ast-grep outline apps/notification-service/src/notification-data-rights-authority-replay.ts --items all
cat -n apps/notification-service/src/notification-data-rights-authority-replay.ts
printf '%s\n' '--- existing notification migrations and schema references ---'
fd -t f -e sql . apps/notification-service/migrations | sort
rg -n -C 5 'CREATE TABLE.*(reminder_occurrences|reminder_outcomes|inbox_messages|data_rights_authority_replay)|CREATE (OR REPLACE )?FUNCTION|CREATE TRIGGER|ALTER.*OWNER|GRANT|REVOKE' apps/notification-service/migrations
printf '%s\n' '--- complete changed tests ---'
for f in \
apps/notification-service/src/notification-data-rights.behavior.test.ts \
apps/notification-service/src/notification-data-rights.integration.test.ts \
apps/notification-service/src/notification-data-rights-migration.test.ts \
apps/notification-service/src/notification-data-rights.test.ts \
infra/tests/notification-migration-role.spec.ts
do
echo "--- $f ---"
cat -n "$f"
done
printf '%s\n' '--- deployment configuration and environment changes ---'
git diff --unified=80 "$base" "$head" -- .env.example .github/workflows/deploy.yml infra/kubernetes/run-migrations.sh apps/notification-service/package.json
printf '%s\n' '--- runtime references to contributor and replay guard ---'
rg -n -C 4 '\b(dataRightsContributor|dataRightsAuthorityReplayGuard|NotificationDataRightsContributor|PostgresNotificationDataRightsAuthorityReplayGuard)\b' --glob '*.ts' .Length of output: 177345 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- HTTP and controller source maps ---'
for f in \
apps/notification-service/src/notification-data-rights-controller.ts \
apps/notification-service/src/notification-data-rights-http-boundary.ts \
apps/notification-service/src/notification-http.ts \
apps/notification-service/src/server.ts \
packages/contracts/src/data-rights.ts
do
echo "--- $f ---"
ast-grep outline "$f" --items all
cat -n "$f"
done
printf '%s\n' '--- focused authority and HTTP tests ---'
for f in \
apps/notification-service/src/notification-data-rights-controller.test.ts \
apps/notification-service/src/notification-data-rights-http-boundary.test.ts \
apps/notification-service/src/notification-http.test.ts \
apps/notification-service/src/server.test.ts \
apps/notification-service/src/notification-data-rights-authority-replay.test.ts \
apps/notification-service/src/notification-data-rights-authority-replay.integration.test.ts \
apps/notification-service/src/notification-data-rights-pagination.test.ts \
apps/notification-service/src/notification-data-rights-preflight-regression.test.ts \
apps/notification-service/src/notification-data-rights-preflight-verification.test.ts
do
echo "--- $f ---"
cat -n "$f"
done
printf '%s\n' '--- data-rights endpoint and authority helper definitions ---'
rg -n -C 5 'verifyDataRights|authorityClaimDigest|verify.*Authority|data-rights/contributor|NOTIFICATION_DATA_RIGHTS_CONTEXT_SECRET' \
apps packages --glob '*.{ts,js,mjs}'
printf '%s\n' '--- migration 0003 and runtime provisioning ---'
cat -n apps/notification-service/migrations/0003_data_rights_authority_replay.sql
cat -n infra/postgres/provision/notification-runtime.psql
printf '%s\n' '--- CI changes and package workspace commands ---'
git diff --unified=50 f8559bf31dc098bdd58473747805a229bf860cc7 b7bec140695a12eb39d537dacb3644581603daf1 -- .github/workflows/ci.yml packages/contracts/src/data-rights.ts packages/contracts/src/data-rights-contract.typecheck.tsLength of output: 167208
I found no current-head findings. I verified that I reviewed the full PR diff from merge base
You are interacting with an AI system. |
seonghobae
added
enhancement
ChatGPT Codex Connector
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| /** Creates the same least-privilege replay-table grant required from deployment. */ | ||
| async function grantRuntimeReplayAuthority(pool: Pool): Promise<void> { | ||
| await pool.query(` |
| application_name: 'life-os-notification-replay-admin', | ||
| max: 2, | ||
| }); | ||
| await administrativePool.query(`DO $$ |
| .query('DROP SCHEMA IF EXISTS notification_service CASCADE') | ||
| .catch(() => undefined); | ||
| await administrativePool | ||
| .query(`DROP OWNED BY ${RUNTIME_ROLE}`) |
| .query(`DROP OWNED BY ${RUNTIME_ROLE}`) | ||
| .catch(() => undefined); | ||
| await administrativePool | ||
| .query(`DROP ROLE IF EXISTS ${RUNTIME_ROLE}`) |
| ); | ||
| } | ||
|
|
||
| /** Requires one ordinary JSON object so prototypes cannot add hidden authority fields. */ |
| await expect( | ||
| bootstrapNotificationService( | ||
| { | ||
| NOTIFICATION_DATABASE_URL: 'postgresql://runtime.invalid/life_os', |
| await expect( | ||
| bootstrapNotificationService( | ||
| { | ||
| NOTIFICATION_DATABASE_URL: 'postgresql://runtime.invalid/life_os', |
| await expect( | ||
| bootstrapNotificationService( | ||
| { | ||
| NOTIFICATION_DATABASE_URL: 'postgresql://runtime.invalid/life_os', |
| \gexec | ||
|
|
||
| ALTER ROLE :"runtime_role" | ||
| WITH LOGIN PASSWORD :'runtime_password' |
| '${NOTIFICATION_RUNTIME_DATABASE_PASSWORD:?Set NOTIFICATION_RUNTIME_DATABASE_PASSWORD}', | ||
| ); | ||
| expect(legacyUpgrade).toContain("POSTGRES_PASSWORD must not remain 'lifeos'"); | ||
| expect(legacyUpgrade).toContain("ALTER ROLE lifeos PASSWORD :'next_admin_password';"); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
life-os.data-rights-contributor.v1participant for tenant export, erasure preflight, erasure, and verificationTest-first evidence
The branch preserves the initial RED contract commit
d68a0c557bb42143690d07a6071887818f25df6a, which required production runtime composition before the implementation commit.Branch lineage
This clean branch starts from protected main
a56ccbf898d43208fc2f7fc6b591588109e60726and supersedes the stale, incomplete, no-PR branchfeat/notification-data-rights-contributor-v1, which only carried a migration based on an older main.Advances #55 and the data-portability portion of #21. It does not close either issue because other service-owned contributors remain.
Summary by CodeRabbit
새 기능
보안
테스트