Skip to content

feat: add trusted local PostgreSQL snapshot CLI - #724

Open
seonghobae wants to merge 22 commits into
mainfrom
feat/trusted-local-snapshot-cli
Open

feat: add trusted local PostgreSQL snapshot CLI#724
seonghobae wants to merge 22 commits into
mainfrom
feat/trusted-local-snapshot-cli

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Buyer-visible capability

Adds a Unix-domain-socket-only pg-erd-snapshot CLI for trusted local PostgreSQL schema snapshots without weakening the web API SSRF boundary or exposing password-bearing DSNs. The canonical PostgreSQL collector is shared by the web and CLI paths so both surfaces produce the same snapshot contract.

Safety and compatibility

  • requires an explicit existing absolute Unix-domain socket directory;
  • has no implicit /tmp fallback and accepts no TCP host or password-bearing DSN;
  • explicitly passes an empty authentication value to asyncpg.connect, preventing ambient PGPASSWORD or passfile fallback while preserving peer/trust authentication over the local socket;
  • includes a test-first regression that sets PGPASSWORD and proves the connection call receives the explicit empty value instead of inheriting the environment;
  • preserves the remote web API allowlist and SSRF boundary;
  • keeps asyncpg catalog reads sequential because one connection cannot safely execute overlapping operations;
  • handles PostgreSQL installations with Citus absent, present, or missing its distributed-table catalog;
  • redacts connection failure details from stderr;
  • documents every public production and test definition required by the repository contract;
  • retains compact and pretty deterministic JSON output.

The review request to add a new KV/credential registry was rejected after verification: no such established CLI registry exists in this repository, and introducing one solely for this command would create a competing configuration contract. PGHOST remains constrained by the existing Unix-socket directory validator.

Test-first correction

  • 0c17794aa97a7ec0c795f28f2842bf2a27f766c9: added the failing regression that sets PGPASSWORD and requires an explicit empty connection value;
  • b3d4ce7e3e03fb1b44ff5ffbb70217e05281a93e: implemented the explicit empty authentication policy in production code.

Exact-head validation

Current head: b3d4ce7e3e03fb1b44ff5ffbb70217e05281a93e.

Exact-head CI, security gates, automated review, unresolved threads, and independent non-author approval must be revalidated after the correction. The PR must not merge until repository policy and every required gate pass on this exact head.

Release status

CHANGELOG.md records the operator-facing capability. No standalone release is proposed until the repository's broader release acceptance gates are satisfied.

Summary by CodeRabbit

  • 새 기능

    • 신뢰된 Unix-domain socket을 통해 로컬 PostgreSQL 스냅샷을 생성하는 pg-erd-snapshot CLI를 추가했습니다.
    • 결과를 JSON 또는 보기 쉬운 형식으로 표준 출력에 제공하며, 비밀번호와 민감한 연결 정보가 인자나 로그에 노출되지 않습니다.
    • 스키마·포트·소켓 경로를 검증하고 오류 발생 시 명확한 오류와 종료 상태를 제공합니다.
    • PostgreSQL 스키마 및 Citus 분산 테이블 정보를 포함한 스냅샷 수집을 지원합니다.
  • 문서

    • CLI 사용법과 로컬 연결 제한 사항을 개발 문서에 추가했습니다.
  • 품질 개선

    • 관련 테스트와 100% 코드 커버리지 검증을 CI에 적용했습니다.

@coderabbitai

coderabbitai Bot commented Aug 3, 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: 28 minutes

Limit details: You’ve used the included review currently available.

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?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d8efdbe1-5806-4476-93f5-513ece0df5ce

📥 Commits

Reviewing files that changed from the base of the PR and between 1d714a1 and dbc3158.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • backend/app/local_snapshot_cli.py
  • backend/tests/test_local_snapshot_cli.py
📝 Walkthrough

Walkthrough

PostgreSQL 스냅샷 수집 로직을 별도 함수로 분리했습니다. Unix 소켓 전용 pg-erd-snapshot CLI를 추가했습니다. 입력 검증, 연결 종료, 오류 처리, JSON 출력을 구현하고 테스트했습니다.

Changes

로컬 스냅샷 수집과 CLI

Layer / File(s) Summary
PostgreSQL 스냅샷 수집 분리
backend/app/pg_introspect/introspect.py, backend/app/pg_introspect/snapshot_collect.py, backend/tests/test_pg_snapshot_collect.py
카탈로그 조회, Citus 데이터 처리, 컬럼 예시 추가, 저장용 정제 로직을 collect_postgres_snapshot으로 이동했습니다. introspect_postgres는 새 함수를 호출합니다. Citus 확장 상태와 카탈로그 부재 동작을 테스트했습니다.
Unix 소켓 CLI 구현과 검증
backend/app/local_snapshot_cli.py, backend/tests/test_local_snapshot_cli.py
Unix 소켓 디렉터리, 포트, 스키마 식별자를 검증합니다. DSN과 비밀번호 없이 연결하고 스냅샷을 수집한 뒤 연결을 종료합니다. 오류는 stderr와 종료 코드로 처리합니다.
CLI 배포와 CI 검증
backend/pyproject.toml, .github/workflows/ci.yml, backend/tests/test_docstrings.py, README.md, CHANGELOG.md, backend/app/pg_introspect/introspect.py
pg-erd-snapshot 프로젝트 명령을 등록했습니다. Unix 소켓 전용 실행 방법과 JSON stdout 출력을 문서화했습니다. 관련 모듈의 문서 문자열 검증과 100% 커버리지 검증을 CI에 추가했습니다. 관련 변경 사항을 changelog에 기록하고 PostgreSQL 연결 보조 함수에 docstring을 추가했습니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to 1d714

The new snapshot CLI may mishandle PostgreSQL connection timeouts instead of reporting them through its intended failure path. Merge should wait for timeout handling and regression coverage to be added.

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant pg-erd-snapshot
  participant asyncpg
  participant PostgreSQL
  Operator->>pg-erd-snapshot: 로컬 스냅샷 명령 실행
  pg-erd-snapshot->>asyncpg: 검증된 Unix 소켓 연결 인자 전달
  asyncpg->>PostgreSQL: 연결 및 카탈로그 조회
  PostgreSQL-->>asyncpg: 스냅샷 메타데이터 반환
  asyncpg-->>pg-erd-snapshot: 연결 결과 반환
  pg-erd-snapshot-->>Operator: 정제된 JSON을 stdout에 출력
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.16% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 신뢰된 로컬 PostgreSQL 스냅샷 CLI 추가라는 주요 변경 사항을 정확하고 간결하게 설명합니다.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/trusted-local-snapshot-cli

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
backend/app/pg_introspect/snapshot_collect.py (1)

23-50: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

독립적인 카탈로그 조회를 병렬로 실행하는 것을 고려하십시오.

schemas, relations, columns, constraints, indexes, pk_columns, fk_edges, has_citus 조회는 서로 의존성이 없습니다. 현재 구현은 이들을 순차적으로 await합니다. 이 함수는 이제 CLI와 웹 API 양쪽에서 호출되는 공용 경로이므로, 순차 실행은 원격 데이터베이스 대상에서 왕복 지연시간을 누적시킵니다. asyncio.gather()로 병렬 실행하면 전체 수집 시간을 줄일 수 있습니다.

⚡ 병렬 실행 제안
-    schemas = await conn.fetch(queries.SCHEMAS_SQL, schema_name, include_system)
-    relations = await conn.fetch(queries.RELATIONS_SQL, schema_name, include_system)
-    columns = await conn.fetch(queries.COLUMNS_SQL, schema_name, include_system)
-    constraints = await conn.fetch(
-        queries.CONSTRAINTS_SQL, schema_name, include_system
-    )
-    indexes = await conn.fetch(queries.INDEXES_SQL, schema_name, include_system)
-    pk_columns = await conn.fetch(
-        queries.PK_COLUMNS_SQL, schema_name, include_system
-    )
-    fk_edges = await conn.fetch(queries.FK_EDGES_SQL, schema_name, include_system)
-    citus_distributed_tables = []
-    has_citus = await conn.fetchval(
-        "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'citus')"
-    )
+    (
+        schemas,
+        relations,
+        columns,
+        constraints,
+        indexes,
+        pk_columns,
+        fk_edges,
+        has_citus,
+    ) = await asyncio.gather(
+        conn.fetch(queries.SCHEMAS_SQL, schema_name, include_system),
+        conn.fetch(queries.RELATIONS_SQL, schema_name, include_system),
+        conn.fetch(queries.COLUMNS_SQL, schema_name, include_system),
+        conn.fetch(queries.CONSTRAINTS_SQL, schema_name, include_system),
+        conn.fetch(queries.INDEXES_SQL, schema_name, include_system),
+        conn.fetch(queries.PK_COLUMNS_SQL, schema_name, include_system),
+        conn.fetch(queries.FK_EDGES_SQL, schema_name, include_system),
+        conn.fetchval(
+            "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'citus')"
+        ),
+    )
+    citus_distributed_tables = []

asyncio 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 `@backend/app/pg_introspect/snapshot_collect.py` around lines 23 - 50, Update
the snapshot collection flow around the independent catalog fetches to import
asyncio and execute the schemas, relations, columns, constraints, indexes,
pk_columns, fk_edges, and has_citus queries concurrently with asyncio.gather().
Preserve the existing result assignments and Citus-specific fallback handling
after the parallel fetches complete.
backend/tests/test_local_snapshot_cli.py (1)

106-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

main()의 예외 처리 분기에 대한 테스트를 추가하는 것을 고려하십시오.

main()OSErrorasyncpg.PostgresError 발생 시 종료 코드 1을 반환합니다. 이 경로에 대한 테스트가 없습니다. asyncio.run을 몽키패치하여 예외를 발생시키고 종료 코드와 stderr 메시지를 검증하는 테스트를 추가하면 회귀를 방지할 수 있습니다.

🤖 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 `@backend/tests/test_local_snapshot_cli.py` around lines 106 - 115,
backend/tests/test_local_snapshot_cli.py에 main()의 예외 처리 경로를 검증하는 테스트를 추가하십시오.
asyncio.run을 몽키패치해 OSError와 asyncpg.PostgresError를 각각 발생시키고, main()이 종료 코드 1을
반환하는지와 예상 stderr 메시지를 검증하십시오.
🤖 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 `@backend/app/local_snapshot_cli.py`:
- Around line 54-129: Add docstrings to the public functions build_parser,
capture_local_snapshot, and main, describing each function’s purpose, inputs,
and return value as appropriate. Keep the existing behavior unchanged and ensure
the module satisfies interrogate’s 100% documentation threshold.
- Around line 68-73: Update the --host argument in the local snapshot CLI parser
so it no longer defaults to the unsafe hardcoded "/tmp" path when PGHOST is
unset. Make the host explicit by requiring the argument or otherwise rejecting
an unset PGHOST with a clear user-facing validation error, while preserving
_socket_directory validation for provided values.

---

Nitpick comments:
In `@backend/app/pg_introspect/snapshot_collect.py`:
- Around line 23-50: Update the snapshot collection flow around the independent
catalog fetches to import asyncio and execute the schemas, relations, columns,
constraints, indexes, pk_columns, fk_edges, and has_citus queries concurrently
with asyncio.gather(). Preserve the existing result assignments and
Citus-specific fallback handling after the parallel fetches complete.

In `@backend/tests/test_local_snapshot_cli.py`:
- Around line 106-115: backend/tests/test_local_snapshot_cli.py에 main()의 예외 처리
경로를 검증하는 테스트를 추가하십시오. asyncio.run을 몽키패치해 OSError와 asyncpg.PostgresError를 각각
발생시키고, main()이 종료 코드 1을 반환하는지와 예상 stderr 메시지를 검증하십시오.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e44f98a-56d9-4101-b42a-b2b4a623a055

📥 Commits

Reviewing files that changed from the base of the PR and between 6f27589 and 73fef74.

📒 Files selected for processing (6)
  • README.md
  • backend/app/local_snapshot_cli.py
  • backend/app/pg_introspect/introspect.py
  • backend/app/pg_introspect/snapshot_collect.py
  • backend/pyproject.toml
  • backend/tests/test_local_snapshot_cli.py

Comment thread backend/app/local_snapshot_cli.py
Comment thread backend/app/local_snapshot_cli.py
@seonghobae

Copy link
Copy Markdown
Collaborator Author

Reviewed the two summary-only nitpicks on current head a40ebd1:

  • Added regression coverage for both OSError and asyncpg.PostgresError redaction paths in main().
  • Did not use asyncio.gather() on the catalog queries: collect_postgres_snapshot receives one asyncpg.Connection, and asyncpg rejects overlapping operations on one connection. The reads remain sequential, with an explanatory comment; true parallelism would require a pool plus an explicit multi-connection snapshot-consistency policy.

@seonghobae
seonghobae enabled auto-merge (squash) August 3, 2026 10:25

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head a40ebd19807588262c370541322909de3416c5ab.

  • Head SHA: a40ebd19807588262c370541322909de3416c5ab

  • Workflow run: 30808360272

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file: README.md"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file: README.md"]
  R1 --> V1["required checks"]
  Evidence --> S2["Backend (5 files)"]
  S2 --> I2["API and service runtime"]
  I2 --> R2["Review risk: Backend (5 files)"]
  R2 --> V2["backend tests"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: dbc315867013fbf0481853039ec53d75e6101743
  • Workflow run: 32637689186
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head dbc315867013fbf0481853039ec53d75e6101743.

  • Head SHA: dbc315867013fbf0481853039ec53d75e6101743

  • Workflow run: 32637689186

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 1

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: ci.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: ci.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file (2 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (2 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Backend (7 files)"]
  S3 --> I3["API and service runtime"]
  I3 --> R3["Review risk: Backend (7 files)"]
  R3 --> V3["backend tests"]
Loading

@seonghobae
seonghobae dismissed opencode-agent[bot]’s stale review August 3, 2026 11:39

Dismissed as superseded by current head a785e7e. Current-head CI run 30810197074 now enforces pytest-cov with --cov-fail-under=100, reports 490/490 selected backend statements covered, 394 tests passed, and explicitly checks docstrings for the new public modules. Current-head Security Scan and Semgrep also pass. Independent current-head approval remains required.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head 97f15008a1daec56f5e3494fdbd6d0cf238b854d. Verify the local snapshot CLI accepts only explicit existing absolute Unix-socket directories, never TCP/password DSNs, shares the canonical collector, redacts failures, and retains complete backend/docstring coverage. Exact-head CI, Security Scan, Semgrep, and CodeRabbit are green; submit APPROVE only if no blocker remains.

Copy link
Copy Markdown
Collaborator Author

Review follow-up on exact head 97f15008a1daec56f5e3494fdbd6d0cf238b854d: the suggested CLI error-path tests are present for both OSError and asyncpg.PostgresError, asserting exit code 1 and redacted stderr. The asyncio.gather() catalog-fetch suggestion is intentionally not applied because all queries share one asyncpg connection; overlapping operations on a single connection are not a valid execution contract. Sequential catalog reads preserve correctness while the shared collector remains reusable by both web and CLI paths.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Please re-review exact head 97f15008a1daec56f5e3494fdbd6d0cf238b854d after central coverage evidence is repaired by .github#731. Repository CI, Security Scan, and Semgrep are green; review threads are resolved. Verify the Unix-socket-only trust boundary, absence of implicit /tmp and password-bearing DSNs, redacted failures, full docstrings, and real error-path tests. Submit APPROVE only if no blocker remains.

Copy link
Copy Markdown
Collaborator Author

@cwl-noema-review Please independently review exact head 97f15008a1daec56f5e3494fdbd6d0cf238b854d after central coverage repair. Confirm local Unix-socket validation, web SSRF-boundary preservation, sequential single-connection collection, and redacted failure behavior; submit APPROVE only if no blocker remains.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent Please independently review exact current head 97f15008a1daec56f5e3494fdbd6d0cf238b854d. The prior coverage finding was on stale head a40ebd19807588262c370541322909de3416c5ab; current exact-head backend CI, Security Scan, SAST Semgrep, and CodeRabbit status all succeed, every review thread is resolved, the CLI has 100% public docstrings, and the unsafe /tmp fallback is removed. Verify the Unix-socket-only boundary, bounded/redacted failure behavior, shared sequential asyncpg collector, Citus fallback, package entrypoint, tests, and CHANGELOG. Submit APPROVE only if no blocker remains.

@cwl-noema-review Please perform the second independent exact-head review after OpenCode and submit APPROVE only if no actionable finding remains.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head 97f15008a1daec56f5e3494fdbd6d0cf238b854d. Current-head CI, Security Scan, and SAST Semgrep are successful; every inline thread is resolved. Verify the Unix-domain-socket-only trust boundary, absence of /tmp or TCP/password DSN fallback, SSRF-boundary preservation, sequential single-connection asyncpg semantics, Citus fallbacks, redacted failures, 100% selected coverage/docstrings, and operator documentation. Submit APPROVE only for this exact head if no blocking finding remains.

Copy link
Copy Markdown
Collaborator Author

@opencode-agent @cwl-noema-review Please independently review exact current head 97f15008a1daec56f5e3494fdbd6d0cf238b854d. All repository-local CI, Semgrep, and Security Scan workflows succeed and all review threads are resolved. Verify explicit existing absolute Unix-socket-directory requirements with no /tmp or TCP/DSN fallback, passwordless local connection arguments, canonical collector reuse without weakening the web SSRF allowlist, sequential asyncpg catalog reads on one connection, Citus absent/missing-catalog handling, redacted failure output, public docstrings and 100% selected coverage, complete CLI exception-path tests, and no new network/password/database-object authority. Submit APPROVE only if no blocker remains.

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 4, 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

printf '%s\n' '== checked-out commit =='
git rev-parse HEAD
printf '%s\n' '== changed files against first parent =='
git diff --name-status HEAD^ HEAD

printf '%s\n' '== changed-file outlines =='
for f in \
  backend/app/local_snapshot_cli.py \
  backend/app/pg_introspect/introspect.py \
  backend/app/pg_introspect/snapshot_collect.py \
  backend/tests/test_local_snapshot_cli.py
do
  if [ -f "$f" ]; then
    echo "--- $f"
    ast-grep outline "$f" --items all || true
  fi
done

printf '%s\n' '== PostgreSQL connection and allowlist references =='
rg -n -i -C 3 \
  '(allowlist|allowed.host|ssrf|introspect_postgres|collect_postgres_snapshot|asyncpg\.connect|create_pool|postgres.*host|host.*postgres)' \
  backend/app backend/tests \
  -g '*.py' || true

printf '%s\n' '== direct diff =='
git diff --no-ext-diff --unified=80 HEAD^ HEAD -- \
  backend/app/local_snapshot_cli.py \
  backend/app/pg_introspect/introspect.py \
  backend/app/pg_introspect/snapshot_collect.py \
  backend/tests/test_local_snapshot_cli.py \
  backend/pyproject.toml \
  README.md

Length of output: 47343


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '== pull request metadata =='
gh pr view 724 --repo ContextualWisdomLab/pg-erd-cloud \
  --json baseRefName,baseRefOid,headRefName,headRefOid,commits \
  --jq '{baseRefName,baseRefOid,headRefName,headRefOid, commits: [.commits[] | {oid,messageHeadline}]}'

printf '%s\n' '== candidate merge bases and changed files =='
base_ref="$(gh pr view 724 --repo ContextualWisdomLab/pg-erd-cloud --json baseRefOid --jq .baseRefOid)"
merge_base="$(git merge-base HEAD "$base_ref")"
printf 'base_ref=%s\nmerge_base=%s\n' "$base_ref" "$merge_base"
git diff --name-status "$merge_base" HEAD

printf '%s\n' '== local CLI implementation =='
sed -n '1,220p' backend/app/local_snapshot_cli.py

printf '%s\n' '== canonical collector implementation =='
sed -n '1,280p' backend/app/pg_introspect/snapshot_collect.py

printf '%s\n' '== web PostgreSQL path =='
sed -n '1,180p' backend/app/pg_introspect/introspect.py

printf '%s\n' '== local CLI tests =='
sed -n '1,260p' backend/tests/test_local_snapshot_cli.py

printf '%s\n' '== collector tests =='
sed -n '1,220p' backend/tests/test_pg_snapshot_collect.py

printf '%s\n' '== package entry point and operator documentation =='
sed -n '1,180p' backend/pyproject.toml
rg -n -C 12 'pg-erd-snapshot|Unix-domain|Unix socket|snapshot CLI' README.md || true

Length of output: 25350


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="37873d7b5421117bf3e08d488ce98b565a109034"

printf '%s\n' '== full PR diff, excluding lockfiles =='
git diff --no-ext-diff --unified=25 "$base" HEAD -- \
  .github/workflows/ci.yml \
  CHANGELOG.md \
  README.md \
  backend/app/local_snapshot_cli.py \
  backend/app/pg_introspect/introspect.py \
  backend/app/pg_introspect/snapshot_collect.py \
  backend/pyproject.toml \
  backend/tests/test_docstrings.py \
  backend/tests/test_local_snapshot_cli.py \
  backend/tests/test_pg_snapshot_collect.py

printf '%s\n' '== changed-path references to connection authority and defaults =='
git diff --no-ext-diff -U0 "$base" HEAD -- \
  backend/app/local_snapshot_cli.py \
  backend/app/pg_introspect/introspect.py \
  backend/app/pg_introspect/snapshot_collect.py \
  README.md \
| rg -n -i -C 2 \
  '(asyncpg\.connect|dsn|password|passfile|PGHOST|/tmp|localhost|host=|allowlist|validate_postgres_dsn_target|asyncio\.gather|UndefinedTableError)' \
|| true

printf '%s\n' '== docstring coverage rules and changelog entry =='
sed -n '1,260p' backend/tests/test_docstrings.py
rg -n -C 8 'snapshot|Unix|socket|CLI' CHANGELOG.md .github/workflows/ci.yml || true

printf '%s\n' '== all collector call sites =='
rg -n -C 3 '\bcollect_postgres_snapshot\s*\(' backend -g '*.py'

printf '%s\n' '== local CLI environment and connect argument references =='
rg -n -C 2 '(PGHOST|PGDATABASE|PGPORT|PGUSER|asyncpg\.connect|password|dsn)' \
  backend/app/local_snapshot_cli.py backend/tests/test_local_snapshot_cli.py

Length of output: 42967


@seonghobae Reviewed exact head 97f15008a1daec56f5e3494fdbd6d0cf238b854d.

I found no blocking issue.

  • The CLI requires an existing absolute Unix-socket directory. It has no implicit /tmp fallback.
  • The CLI passes no DSN or password to asyncpg.connect.
  • The web PostgreSQL path retains validate_postgres_dsn_target. The CLI does not weaken the web SSRF boundary.
  • Both paths use collect_postgres_snapshot.
  • The collector performs sequential queries on one asyncpg.Connection.
  • The Citus absent and missing-catalog paths are handled and tested.
  • CLI failure output redacts error details for OSError and asyncpg.PostgresError.
  • The new public functions have docstrings. The added coverage and exception-path tests are present.

The README /tmp value is an explicit Unix-socket directory example. It is not a fallback.

[approve]

You are interacting with an AI system.

@seonghobae
seonghobae marked this pull request as draft August 4, 2026 12:46
auto-merge was automatically disabled August 4, 2026 12:46

Pull request was converted to draft

@seonghobae
seonghobae marked this pull request as ready for review August 4, 2026 12:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

40-44: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Coverage 범위를 목표 모듈과 일치시키세요.

backend/pyproject.toml--cov 범위를 고정하지만 10개 파일을 포함합니다. app/snowflake_introspect/introspect.py도 포함하지 않습니다. 세 모듈만 100% 검사하려면 include를 해당 모듈로 제한하거나 CI 명령에 각 --cov=<module> 옵션을 추가하세요.

🤖 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 @.github/workflows/ci.yml around lines 40 - 44, Update the “Tests and
coverage (pytest)” step so coverage explicitly targets only the three intended
modules, matching the configured scope in backend/pyproject.toml and excluding
app/snowflake_introspect/introspect.py; use an appropriate coverage include
setting or explicit --cov=<module> options while preserving the 100% threshold.
🤖 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 `@backend/tests/test_local_snapshot_cli.py`:
- Around line 43-54: Update local_snapshot_cli.build_parser and its related
runtime configuration flow to stop reading PGDATABASE or PGHOST directly from
os.environ; obtain these values through the established KV or credential
registry, using environment variables only during registry bootstrap. Revise
test_parser_requires_explicit_host_without_pghost to mock registry lookups
instead of setting or deleting runtime environment variables, while preserving
the explicit-host validation and SystemExit behavior.

In `@backend/tests/test_pg_snapshot_collect.py`:
- Around line 47-52: Add docstrings to the following four public test functions
to document the behavior they validate:
test_collect_postgres_snapshot_handles_each_citus_state in
backend/tests/test_pg_snapshot_collect.py (lines 47-52) should document that it
validates snapshot collection behavior for each Citus mode state; the test in
backend/tests/test_local_snapshot_cli.py (lines 43-46) should document required
host validation when PGHOST environment variable is absent; the test in
backend/tests/test_local_snapshot_cli.py (lines 116-121) should document compact
and pretty JSON output formatting behavior; and the test in
backend/tests/test_local_snapshot_cli.py (lines 161-167) should document
connection error message sanitization behavior. Each docstring should be brief
and placed immediately after the function definition.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 40-44: Update the “Tests and coverage (pytest)” step so coverage
explicitly targets only the three intended modules, matching the configured
scope in backend/pyproject.toml and excluding
app/snowflake_introspect/introspect.py; use an appropriate coverage include
setting or explicit --cov=<module> options while preserving the 100% threshold.
🪄 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: 389a4334-c76b-4260-a792-24a7d392d671

📥 Commits

Reviewing files that changed from the base of the PR and between 73fef74 and 1c8dfce.

📒 Files selected for processing (8)
  • .github/workflows/ci.yml
  • CHANGELOG.md
  • backend/app/local_snapshot_cli.py
  • backend/app/pg_introspect/snapshot_collect.py
  • backend/pyproject.toml
  • backend/tests/test_docstrings.py
  • backend/tests/test_local_snapshot_cli.py
  • backend/tests/test_pg_snapshot_collect.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • backend/app/pg_introspect/snapshot_collect.py
  • backend/pyproject.toml
  • backend/app/local_snapshot_cli.py

Comment thread backend/tests/test_local_snapshot_cli.py
Comment thread backend/tests/test_pg_snapshot_collect.py
@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@opencode-agent

Copy link
Copy Markdown
Contributor

Already queued @opencode-agent on this exact request for PR #724 at head dbc315867013fbf0481853039ec53d75e6101743. Central exact-name Actions artifacts are the durable dispatch ledger; existing review workflows remain authoritative for the final verdict and failure evidence.

@seonghobae seonghobae added the enhancement New feature or request label Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: data Database, schema, migration, ETL, or lineage enhancement New feature or request priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: feature New or expanded product capability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant