fix(bootstrap): preserve explicit source precedence - #89
seonghobae wants to merge 25 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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
changed the title
|
@coderabbitai review Review exact current source head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== checked-out and requested commits ==='
git rev-parse HEAD
git cat-file -t 277cad148bdc00e7478c99c26741d8edef0f03d5
git cat-file -t bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10
git merge-base --is-ancestor bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10 277cad148bdc00e7478c99c26741d8edef0f03d5 && echo 'base is ancestor of head'
echo '=== exact source diff summary ==='
git diff --stat bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10 277cad148bdc00e7478c99c26741d8edef0f03d5
git diff --name-status bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10 277cad148bdc00e7478c99c26741d8edef0f03d5
echo '=== resolver definitions and focused call sites ==='
rg -n -C 5 --glob '*.py' '^\s*def resolve_(dsn|secret_key)\s*\(|\bresolve_(dsn|secret_key)\s*\(' .
echo '=== relevant Python outlines ==='
fd -e py . | xargs -r ast-grep outline --items all --type function --match 'resolve_dsn|resolve_secret_key|SecretStore' --view expanded
echo '=== precedence and regression-test references ==='
rg -n -C 4 --glob '*.py' 'explicit_empty|resolve_dsn|resolve_secret_key|PG_LLM_BATCH_DSN|PG_LLM_BATCH_SECRET_KEY|SecretStore' .
echo '=== relevant documentation ==='
rg -n -C 4 -i 'explicit.*empty|empty.*explicit|source precedence|ambient|Fernet|decryption.authority|rollback|PG_LLM_BATCH_(DSN|SECRET_KEY)' \
README.md CHANGELOG.md docs 2>/dev/null || true
echo '=== GitHub PR state and exact-head checks ==='
gh pr view 89 --repo ContextualWisdomLab/pg-llm-batch \
--json number,isDraft,headRefOid,baseRefOid,mergeable,reviewDecision,reviews,statusCheckRollup \
--jq '{number,isDraft,headRefOid,baseRefOid,mergeable,reviewDecision,reviews:[.reviews[]|{author:.author.login,state}],checks:[.statusCheckRollup[]|{name:.name,status:.status,conclusion:.conclusion,workflowName:.workflowName,headSha:.headSha}]}'
echo '=== PR `#88` state ==='
gh pr view 88 --repo ContextualWisdomLab/pg-llm-batch \
--json number,state,isDraft,mergedAt,headRefOid,baseRefOid,mergeable \
--jq '{number,state,isDraft,mergedAt,headRefOid,baseRefOid,mergeable}'Length of output: 50656 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== exact patch ==='
git diff --find-renames --find-copies --unified=80 \
bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10 \
277cad148bdc00e7478c99c26741d8edef0f03d5 \
-- pg_llm_batch/bootstrap.py tests/test_bootstrap_cli.py \
tests/test_bootstrap_explicit_dsn_precedence.py \
docs/doctoring/bootstrap-dsn-precedence.md CHANGELOG.md
echo '=== bootstrap implementation ==='
sed -n '1,110p' pg_llm_batch/bootstrap.py
echo '=== CLI parse, dispatch, and error boundary ==='
sed -n '35,180p' pg_llm_batch/cli.py
echo '=== bootstrap regression tests ==='
sed -n '1,125p' tests/test_bootstrap_cli.py
sed -n '1,180p' tests/test_bootstrap_explicit_dsn_precedence.py
echo '=== SecretStore key initialization and encrypted-read behavior ==='
sed -n '264,390p' pg_llm_batch/config.py
echo '=== environment-variable reads outside bootstrap ==='
rg -n --glob '*.py' -P '\b(?:os\.(?:environ|getenv)|environ\[)' pg_llm_batch tests || true
echo '=== doctoring document ==='
sed -n '1,140p' docs/doctoring/bootstrap-dsn-precedence.mdLength of output: 45539
The regression coverage verifies explicit-empty behavior, existing non-empty precedence, environment fallback, missing required DSN failure, and missing optional-key behavior. The doctoring and changelog state the precedence, decryption-authority, verification, and rollback contracts. PR
|
|
@coderabbitai review |
|
@coderabbitai review Please review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="ContextualWisdomLab/pg-llm-batch"
head="d0c784239e948b94d405b5cb0be3615fe2456d76"
base="bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10"
echo "=== Source identity ==="
git rev-parse HEAD
git cat-file -t "$head"
git cat-file -t "$base"
git merge-base --is-ancestor "$base" "$head" && echo "protected base is an ancestor of requested head"
git diff --name-status "$base" "$head"
git diff --unified=60 "$base" "$head" -- \
pg_llm_batch/bootstrap.py \
tests/test_bootstrap_explicit_dsn_precedence.py \
docs/doctoring/bootstrap-dsn-precedence.md \
CHANGELOG.md
echo "=== Bootstrap resolver implementation ==="
ast-grep outline pg_llm_batch/bootstrap.py --items all --type function --match 'resolve_dsn|resolve_secret_key' --view expanded
sed -n '1,180p' pg_llm_batch/bootstrap.py
echo "=== Resolver callers and I/O ordering ==="
rg -n -C 5 --glob '*.py' \
'^\s*def resolve_(dsn|secret_key)\s*\(|\bresolve_(dsn|secret_key)\s*\(|(?:connect|SecretStore|create_engine|psycopg)' \
pg_llm_batch tests
echo "=== Regression coverage ==="
ast-grep outline tests/test_bootstrap_explicit_dsn_precedence.py --items all --type function --view expanded
sed -n '1,260p' tests/test_bootstrap_explicit_dsn_precedence.py
echo "=== Doctoring and release contract ==="
sed -n '1,240p' docs/doctoring/bootstrap-dsn-precedence.md
rg -n -C 3 'bootstrap|DSN|secret|whitespace|precedence' CHANGELOG.md
echo "=== Live pull request and dependency state ==="
gh pr view 89 --repo "$repo" \
--json number,isDraft,headRefOid,baseRefOid,mergeable,reviewDecision,statusCheckRollup \
--jq '{number,isDraft,headRefOid,baseRefOid,mergeable,reviewDecision,checks:[.statusCheckRollup[]|{name,status,conclusion,headSha,workflowName}]}'
gh pr view 88 --repo "$repo" \
--json number,state,isDraft,mergedAt,headRefOid,baseRefOid,mergeable \
--jq '{number,state,isDraft,mergedAt,headRefOid,baseRefOid,mergeable}'Length of output: 137650
The checked-out source is exactly
PR
|
|
Superseded by protected-main replacement PR #139. The replacement was rebuilt from the then-current protected main, preserved the intended omission-vs-explicit authority boundary, strengthened the string check to exact |
1 participant
RCA
Bootstrap source selection originally used boolean truthiness, conflating omission (
None) with explicit false-valued input and allowing ambiguous database/decryption authority. The branch now separates source absence, source type, and source value.resolve_dsn()consultsPG_LLM_BATCH_DSNonly when the explicit argument is omitted. An explicit Postgres DSN must be an exact string; non-string, empty, or whitespace-only explicit values fail with boundedConfigErrorbefore environment fallback or libpq. Omitted input also rejects empty/whitespace-only environment DSNs. Valid nonblank DSNs retain byte identity.resolve_secret_key()consultsPG_LLM_BATCH_SECRET_KEYonly when the explicit argument is omitted. An explicit Fernet bootstrap key must be an exact string; non-string values fail before environment fallback/store construction, while the valid explicit empty string remains empty and cannot silently inherit ambient decryption authority.Test-first evidence
Earlier RED→GREEN cycles established empty/whitespace precedence and ambient blank-DSN rejection. The latest type-authority cycle is independently reviewable:
c140ee069eb498a0b98902a8956e046b7eb35039addedtests/test_bootstrap_type_boundary.py. CI31354062621failed seven intended cases: booleans/floats/bytes/containers either crossed the boundary or produced unrelated Python errors instead of bounded bootstrap validation.6319d21648bd9176909b026ca17e69624cd4fbc9rejects non-string explicit DSN/Fernet values before fallback; CI31354421145, Security Scan31354421128, and SAST Semgrep31354421133succeeded.8b283e294f503bed38395ba4b9e60497a0ed41derequirednon-string,explicit Postgres DSN,explicit Fernet, andbefore environment fallbackin doctoring/CHANGELOG. CI31354545493failed only that new documentation contract.bootstrap secret-key precedence) caught and prevented semantic drift in the changelog. Current head2df9aaf27e47bf54743d22ce610f6b8188429049preserves both contracts.Predecessor RED and GREEN results are development provenance only.
Current exact state
2df9aaf27e47bf54743d22ce610f6b8188429049.maintip:bf2cc2e140dc3ff4a56c3203f80f41bb9fed5d10.31354988532: completed / success.31354988512: completed / success.31354988530: completed / success.These PR-triggered workflows still precede protected-main exact-source governance PR #88, so successful branch runs remain staged integration evidence until that prerequisite integrates and the final source is revalidated under the protected exact-source contract.
Merge boundary
Keep Draft until #88 reaches protected
main; then reconcile only when integration correctness requires it and reacquire every applicable exact-source CI, security, dependency, packaging, provenance, semantic-review, and branch-policy gate. Do not churn a clean source head solely to retrigger an unavailable external reviewer.Merge only with zero valid unresolved findings, live repository policy/protection satisfied, every required exact-current-head gate terminal-success, current semantic review where required, and qualifying independent non-author formal
APPROVEDreview where required. The protected code-owner gate is separately disabled/on hold for the current solo-maintainer state and must not be inferred as universally mandatory. Failed RED, predecessor-head, stale-base, generated-merge-only, queued, pending, cancelled, skipped-required, absent, neutral-required, rate-limited, status-only, author-only, infrastructure-only, or synthetic-only evidence is not final acceptance.