Skip to content

feat(tenant): isolate durable lifecycle state - #53

Closed
seonghobae wants to merge 103 commits into
mainfrom
agent/tenant-scoped-lifecycle
Closed

feat(tenant): isolate durable lifecycle state#53
seonghobae wants to merge 103 commits into
mainfrom
agent/tenant-scoped-lifecycle

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Product gap

Protected main keys durable provider lifecycle state by endpoint alias + remote batch id and lacks package-owned trusted tenant identity/RLS for shared-table MSA deployments.

Bounded implementation

This branch adds strict host-selected tenant_scope, TenantDurableBatchAPIClient, tenant-qualified lifecycle persistence/read helpers, tenant-qualified uniqueness/indexes, parameterized transaction-local tenant context, forced default-deny PostgreSQL RLS, atomic legacy backfill/constraint/RLS migration, packaged/container schema identity, and standalone compatibility. Production application identities remain NOSUPERUSER NOBYPASSRLS; provider metadata/IDs/payloads/headers never select tenant scope.

Test-first / review evidence

Tenant validation, schema, persistence, client compatibility, migration, and isolation contracts were introduced test-first. The valid migration-order/RLS findings were repaired; the only returned inline review thread is resolved/outdated. No predecessor review/check evidence transfers after a source change.

A later CodeRabbit review included maintainability-only suggestions such as consolidating duplicated test doubles/exception-handler structure. Those do not establish a correctness/security regression on the unchanged current source and are deliberately not used to churn this stack root: changing #53 for a low-value refactor would invalidate every downstream exact-base relation. Re-evaluate such refactors from protected main after the stack lands.

Current exact state

  • Source head: f6f328a8b9f7db57c5286be00c854fafc0b5bbf8.
  • Independently resolved protected pg-llm-batch/main: bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.
  • GitHub reports Draft and mergeable.
  • CI 31272388550, Security Scan 31272388560, and SAST Semgrep 31272388548: completed / success on this source relation.
  • Exact-current-head OpenCode review 31274235004 is still formal CHANGES_REQUESTED; its stated reason was failed coverage-evidence infrastructure, not a pg-llm-batch semantic/source finding. It remains an unsuperseded review state until a fresh current-head review legitimately replaces it.

The protected-main repository CI still predates #88 exact-source checkout governance, so successful PR-triggered runs are staged integration evidence rather than final contributor-source-head acceptance.

Current prerequisite classification

Repository-local exact-source governance #88 remains the immediate prerequisite. Current protected organization rules require the central workflow set and review-thread resolution, but specify required_approving_review_count: 0, code-owner review disabled, and no last-push approval requirement. Therefore lack of an independent formal approval is not itself a current leaf merge prerequisite unless live policy changes.

The protected central OpenCode required-workflow entrypoint on .github/main is a stable-context wrapper; its required jobs are separate from authenticated review dispatch. ContextualWisdomLab/.github#790 remains read-only and must not be copied, weakened, triggered, dismissed, resolved, or merged by this writer, but its historical review state is not promoted into an extra leaf approval requirement beyond live policy.

After #88 reaches protected main, move this PR from Draft only after refetching the unchanged head/base and current writer state. Acquire fresh required central/repository workflow evidence under exact-source governance and a fresh review outcome sufficient to supersede the existing infrastructure-only CHANGES_REQUESTED state. Do not dismiss that current-head review merely to manufacture mergeability; supersede it with legitimate new evidence first.

Downstream stack boundary

The current product chain remains:

#53 -> #57 -> #58 -> #59 -> #60 -> #92 -> #94 -> #95 -> #96 -> #97.

Each downstream PR must continue to descend from its exact predecessor. Former #55/#56 and #78/#79/#80/#83/#84 are superseded/closed implementation lines and their evidence does not transfer. Avoid low-value source churn on this root because changing #53 invalidates every downstream exact-base relation.

Follow-up lifecycle boundaries

These are independent follow-ups and do not justify expanding or rewriting this root PR while it owns the tenant-lifecycle stack.

Merge / release boundary

Keep Draft until #88 reaches protected main. Then obtain fresh exact-current-head required workflow/review evidence, re-evaluate current protected main/rulesets, and regenerate every applicable exact-source quality/security/dependency/package/provenance/live-PostgreSQL/release-acceptance gate on the unchanged final source. Merge only with zero valid unresolved findings, the existing infrastructure-only CHANGES_REQUESTED legitimately superseded, review threads resolved, and every review/approval requirement actually imposed by live policy satisfied.

The feature remains Unreleased; release evaluation begins only after protected integration and integrated-head release acceptance.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dad122c7-5205-401c-aae4-e6b6b007194d

📥 Commits

Reviewing files that changed from the base of the PR and between 7fa2a19 and f3409bd.

📒 Files selected for processing (6)
  • AGENTS.md
  • CHANGELOG.md
  • README.md
  • docs/superpowers/plans/2026-08-05-tenant-scoped-lifecycle.md
  • tests/test_remote_batch_lifecycle.py
  • tests/test_tenant_operator_documentation.py
📝 Walkthrough

Walkthrough

테넌트 범위를 durable lifecycle 식별자에 추가했습니다. PostgreSQL 복합 키와 강제 RLS를 적용했습니다. tenant-aware 저장·조회 API와 TenantDurableBatchAPIClient를 추가했습니다. standalone 호환성, 마이그레이션, 오류 복구, 문서 및 테스트를 갱신했습니다.

Changes

테넌트 범위 lifecycle 격리

Layer / File(s) Summary
스키마와 RLS 계약
pg_llm_batch/schema.sql, docker/postgres/init/02_schema.sql, tests/test_tenant_lifecycle_schema.py, tests/test_integration_pg.py
tenant_scope 열, 테넌트 복합 키, tenant-scoped 인덱스와 forced RLS 정책을 추가했습니다. 기존 행은 standalone으로 보정합니다.
영속성 API와 상태 검증
pg_llm_batch/db.py, tests/test_tenant_lifecycle_persistence.py, tests/test_remote_batch_*
tenant scope 검증, transaction-local 설정, tenant-qualified upsert와 조회 API를 추가했습니다. bounded response 테스트 더블과 상태 계약을 갱신했습니다.
테넌트 durable client와 공개 API
pg_llm_batch/durable_client.py, pg_llm_batch/__init__.py, tests/test_tenant_durable_client.py, tests/test_lifecycle_seam_validation.py
TenantDurableBatchAPIClient와 tenant recorder를 추가했습니다. 기존 standalone recorder 계약을 유지하고 생성 시 callable seam과 tenant scope를 검증합니다.
운영 문서와 검증 계약
README.md, ARCHITECTURE.md, docs/adr/*, docs/doctoring/*, docs/remote-batch-lifecycle.md, tests/test_tenant_operator_documentation.py
tenant identity 출처, RLS 경계, 마이그레이션, rollback 조건, 운영 사용법과 문서 검증 범위를 기록했습니다.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Host
  participant TenantDurableBatchAPIClient
  participant Provider
  participant PostgreSQL
  Host->>TenantDurableBatchAPIClient: 인증된 tenant_scope 전달
  TenantDurableBatchAPIClient->>TenantDurableBatchAPIClient: tenant_scope 검증
  TenantDurableBatchAPIClient->>Provider: 배치 생성 또는 상태 요청
  Provider-->>TenantDurableBatchAPIClient: provider 상태 반환
  TenantDurableBatchAPIClient->>PostgreSQL: transaction-local scope 설정
  TenantDurableBatchAPIClient->>PostgreSQL: tenant-qualified lifecycle 저장 또는 조회
  PostgreSQL-->>TenantDurableBatchAPIClient: RLS 적용 결과 반환
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 79.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 테넌트별 durable lifecycle 상태 격리라는 PR의 주요 변경 사항을 간결하고 정확하게 설명합니다.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch agent/tenant-scoped-lifecycle
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/tenant-scoped-lifecycle

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current source head f6f328a8b9f7db57c5286be00c854fafc0b5bbf8 against protected main bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10. Focus on tenant-scope trust, forced RLS/default-deny migration ordering, tenant-qualified uniqueness/read/write predicates, pre-I/O tenant validation, legacy standalone migration compatibility, and current package/container schema identity. The earlier migration-plan ordering finding is resolved. Treat central OpenCode coverage-evidence failure as infrastructure/merge-readiness evidence, not a leaf source finding.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10'
head='f6f328a8b9f7db57c5286be00c854fafc0b5bbf8'
repo='ContextualWisdomLab/pg-llm-batch'

printf '%s\n' '== Exact commit identity =='
git rev-parse "${base}^{commit}"
git rev-parse "${head}^{commit}"
git merge-base --is-ancestor "$base" "$head" && echo 'BASE_IS_ANCESTOR=true' || echo 'BASE_IS_ANCESTOR=false'
git show -s --format='base: %H%n%an%n%s' "$base"
git show -s --format='head: %H%n%an%n%s' "$head"

printf '%s\n' '== Exact changed paths =='
git diff --name-status "$base" "$head"
printf '%s\n' '== Exact diff stat =='
git diff --stat "$base" "$head"

printf '%s\n' '== Relevant implementation outlines at requested head =='
git show "${head}:pg_llm_batch/db.py" > /tmp/pg_llm_batch_db.py
git show "${head}:pg_llm_batch/durable_client.py" > /tmp/pg_llm_batch_durable_client.py
ast-grep outline /tmp/pg_llm_batch_db.py --items all --view expanded
ast-grep outline /tmp/pg_llm_batch_durable_client.py --items all --view expanded

printf '%s\n' '== Exact implementation diff =='
git diff --unified=70 "$base" "$head" -- \
  pg_llm_batch/db.py pg_llm_batch/durable_client.py pg_llm_batch/__init__.py

printf '%s\n' '== Canonical schema and image schema at requested head =='
git show "${head}:pg_llm_batch/schema.sql" > /tmp/canonical_schema.sql
git show "${head}:docker/postgres/init/02_schema.sql" > /tmp/image_schema.sql
cmp -s /tmp/canonical_schema.sql /tmp/image_schema.sql && echo 'SCHEMAS_IDENTICAL=true' || echo 'SCHEMAS_IDENTICAL=false'
sha256sum /tmp/canonical_schema.sql /tmp/image_schema.sql
cat -n /tmp/canonical_schema.sql

printf '%s\n' '== Targeted contract-test outlines at requested head =='
for path in \
  tests/test_tenant_scope_validation.py \
  tests/test_tenant_lifecycle_persistence.py \
  tests/test_tenant_durable_client.py \
  tests/test_standalone_lifecycle_compatibility.py \
  tests/test_tenant_lifecycle_schema.py \
  tests/test_integration_pg.py \
  tests/test_schema_integrity.py
do
  if git cat-file -e "${head}:${path}" 2>/dev/null; then
    printf '\n-- %s --\n' "$path"
    git show "${head}:${path}" > "/tmp/$(basename "$path")"
    ast-grep outline "/tmp/$(basename "$path")" --items all --view expanded
  fi
done

printf '%s\n' '== Exact-head PR and check evidence =='
gh pr view 53 --repo "$repo" --json headRefOid,baseRefOid,mergeable,reviewDecision,statusCheckRollup \
  2>&1 || true
gh api "repos/${repo}/commits/${head}/check-runs?per_page=100" \
  --jq '.check_runs[] | [.name,.status,.conclusion,.html_url] | `@tsv`' 2>&1 || true

Length of output: 94677


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10'
head='f6f328a8b9f7db57c5286be00c854fafc0b5bbf8'
repo='ContextualWisdomLab/pg-llm-batch'

show_range() {
  local path="$1"
  local start="$2"
  local end="$3"
  printf '\n== %s:%s-%s at requested head ==\n' "$path" "$start" "$end"
  git show "${head}:${path}" | sed -n "${start},${end}p" | cat -n
}

show_range pg_llm_batch/db.py 150 645
show_range pg_llm_batch/durable_client.py 20 390
show_range tests/test_tenant_scope_validation.py 1 130
show_range tests/test_tenant_lifecycle_persistence.py 75 330
show_range tests/test_tenant_durable_client.py 1 330
show_range tests/test_standalone_lifecycle_compatibility.py 1 150
show_range tests/test_tenant_lifecycle_schema.py 1 150
show_range tests/test_integration_pg.py 100 285
show_range tests/test_tenant_operator_documentation.py 1 150
show_range docs/superpowers/plans/2026-08-05-tenant-scoped-lifecycle.md 1 220

printf '%s\n' '== Tenant-qualified lifecycle SQL in the requested diff =='
git diff --unified=12 "$base" "$head" -- pg_llm_batch/db.py pg_llm_batch/schema.sql \
  | rg -n -C4 '(tenant_scope|ON CONFLICT|WHERE|CREATE INDEX|UNIQUE|ROW LEVEL|CREATE POLICY|set_config)' || true

printf '%s\n' '== RLS migration and policy definition in the base =='
git show "${base}:pg_llm_batch/schema.sql" \
  | rg -n -C5 '(llm_remote_batch_jobs|ROW LEVEL|CREATE POLICY|tenant_scope|NO FORCE|ENABLE|FORCE)' || true

printf '%s\n' '== Failed Strix check metadata and log =='
gh api "repos/${repo}/actions/jobs/93140451999" \
  --jq '{name,status,conclusion,started_at,completed_at,html_url,steps}' 2>&1 || true
gh run view 31272387553 --repo "$repo" --log-failed 2>&1 | tail -n 350 || true

printf '%s\n' '== Review states and unresolved review threads =='
gh api graphql -F owner='ContextualWisdomLab' -F name='pg-llm-batch' -F number=53 -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewDecision
      reviews(first: 100) {
        nodes { author { login } state commit { oid } submittedAt }
      }
      reviewThreads(first: 100) {
        nodes { isResolved comments(first: 1) { nodes { author { login } body } } }
      }
    }
  }
}' --jq '.data.repository.pullRequest' 2>&1 || true

Length of output: 120649


@seonghobae I reviewed only head f6f328a8b9f7db57c5286be00c854fafc0b5bbf8 against base bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.

I found one source-level blocker.

  • pg_llm_batch/schema.sql: The anonymous migration block ends after FORCE ROW LEVEL SECURITY at Line 183. The policy is dropped and recreated in separate statements at Lines 193-203. Therefore, policy recreation is not atomic with owner-enforcement relaxation, legacy backfill, constraint migration, and the RLS enable/force transition.

    The policy order itself is safe: RLS is enabled and forced before policy recreation, and the interval without a policy is default-deny. However, this does not meet the documented contract in docs/superpowers/plans/2026-08-05-tenant-scoped-lifecycle.md, which requires policy recreation and forced-RLS restoration in the same PostgreSQL statement. Move the idempotent policy recreation into the same atomic migration unit, or revise the documented atomicity contract if separate policy DDL is intentional and safe.

I verified these exact-head source contracts:

  • tenant_scope validation is strict, ASCII-only, and non-coercive.
  • Tenant validation occurs before tenant persistence database access and before tenant client base construction.
  • The standalone persistence return shape excludes tenant_scope.
  • The legacy DurableBatchAPIClient recorder seam remains four arguments.
  • Lifecycle uniqueness, upsert conflict handling, reads, and the operational index include tenant_scope.
  • set_config('pg_llm_batch.tenant_scope', %s, true) is parameterized and transaction-local.
  • The migration backfills legacy rows to standalone without row deletion.
  • ENABLE ROW LEVEL SECURITY occurs before FORCE ROW LEVEL SECURITY in the migration block.
  • The RLS policy uses matching USING and WITH CHECK predicates. Missing tenant context is default-deny.
  • Packaged and image schemas are identical. Both have SHA-256 1d286474dd8e243797f9d67bb81694641dc405797547905d98d7e80e9886464f.
  • Documentation preserves the trusted-application-boundary limitation and documents NOSUPERUSER NOBYPASSRLS.

The exact-head Python, coverage, package, container, CodeQL, Semgrep, OSV, Trivy, and dependency-review checks succeeded. The strix check failed during Run Strix (quick). The available workflow data does not identify a leaf source defect. Treat it as merge-readiness evidence.

I did not approve the PR because the atomic policy-recreation contract remains unmet.

You are interacting with an AI system.

@seonghobae
seonghobae marked this pull request as ready for review August 11, 2026 21:09
@seonghobae
seonghobae marked this pull request as draft August 11, 2026 21:17

Copy link
Copy Markdown
Contributor Author

@opencode-agent address

Continue the existing PR branch. Refetch the exact current head/base, all reviews/threads, and every GitHub Check; fix current defects directly and keep working while central prerequisites run.

Once central .github#790 bounded trusted-uv retry handling and .github#812 current-attempt artifact binding are protected, issue a full fresh exact-head OpenCode/Noema dispatch. Do not use failed-jobs-only reruns or reuse expired, predecessor, synthetic, skipped, cancelled, or status-only source evidence. Preserve hash-pinned dependency closure, least privilege, two-word-or-longer snake_case database object names, realistic batch/transaction/concurrency tests, Python 3.14, 100% production statement/branch coverage and public docstrings, security/SBOM/provenance checks, APA 7 doctoring, and CHANGELOG discipline. Use NVIDIA_NIM_API_KEY, never COPILOT_GITHUB_TOKEN.

Do not open a competing PR, synthesize approval, or bypass protection. Merge only after all exact-head gates succeed, actionable threads are zero, and a qualifying non-author formal approval exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant