Skip to content

feat(db): spec-2.3a db query tool#98

Open
sqlrush wants to merge 2 commits into
mainfrom
spec-2.3a-db-query-tool
Open

feat(db): spec-2.3a db query tool#98
sqlrush wants to merge 2 commits into
mainfrom
spec-2.3a-db-query-tool

Conversation

@sqlrush

@sqlrush sqlrush commented Jun 8, 2026

Copy link
Copy Markdown
Owner

What

spec-2.3a-db-query-tool implementation (design: opendbrb PR #81, APPROVED @ R2 2026-06-08, 三路 pre-impl + path 4/4). The tool substrate bundled DB skills (2.3b) will drive.

Deliverables

D What Where
D-1 db.QueryConn capability (Conn + Query; spec-1.18 R-8 兑现,db.Conn 零改) + QueryOptions/QueryResult internal/domain/db/query.go
D-2 postgres: pgxPool +BeginTx + 窄 queryTx/queryRows + READ ONLY tx + 恒 ROLLBACK + MaxRows+1 sentinel + formatCell 类型表; classify 仅 25006→READONLY_VIOLATION domain/db/postgres/query.go
D-3 db_query ToolExecutor: ctx 双轨先于 classify + lazy open 可重试 + capability-close + aligned 表 + 16KiB UTF-8 cap internal/app/tools/dbquery/
D-4 bootstrap DBQueryExecutors(选得出才注册,不 Open) + ambiguous/none 区分 log + warnIfInsecureSSL file-only bootstrap/{connection,tui_launcher}.go
D-5 2 errcode(中文,Q11) + manifest + blank-import db/errors.go + dbquery/errors.go
D-6 集成(fake QueryConn 全链路 + ctx 双轨 + dedup + 2.3 ToolFilter) + pg_integration parked tests/integration/dbquery/ + postgres/integration_test.go
D-7 import-rules + driver.go errata + gate

codex/claude 三路 finding 落地(高价值)

  • codex CRIT-1: ctx cancel/deadline 在 open/query 错误后先判 → fatal Go error(classify 保 ctx root under Unwrap)→ Loop 走 spec-1.21 FinishCancelled/TOOL_TIMEOUT,不被包成 recoverable DB.TIMEOUT。集成测试端到端验证。
  • codex HIGH-1: MaxRows+1 sentinel(99/100/101 表测)
  • codex HIGH-2: capability 断言失败 conn.Close()(Close-once 测)
  • cr/codex HIGH-2: formatCell 类型表 via driver.Valuer(pgtype.Numeric→十进制 / []byte→\x hex;非 fmt.Sprint struct 垃圾)
  • cr HIGH-1: 仅 25006→READONLY_VIOLATION,0A000→QUERY_FAILED 回归测
  • cr/codex HIGH-3: 窄 queryTx/queryRows,fake embed pgx 接口只实现用到的方法
  • codex MED-1: 16KiB UTF-8 安全截断(CJK-boundary 测)
  • arch CRIT: 只读非零副作用——residual(advisory lock/nextval/temp/远端)文档化(用户 path 4/4 不主动 cleanup,真边界 least-privilege)

用户 path 4/4

Q12 residual 文档化不 cleanup / Q6 cap 64KiB→16KiB / Q8 dedup cacheable + 时变表 caveat / Q11 新码中文。

规则 21 全调用方(seam/SSL 改动)

  • pgxPool +BeginTx:调用方 = pgConn(实现)+ 1.18 fakePool(补 BeginTx,0 行为改);production *pgxpool.Pool 自动满足
  • warnIfInsecureSSL → file-only:唯一调用方 OpenConnection(1.19 零生产调用方,本 spec 第一个);既有 TestWarnIfInsecureSSLNoPanic 不破

Quality

Coverage: dbquery 95.4% / db 100% / postgres 97.3% / bootstrap 89.2%(全 ≥85%)。make gate PASSED;go mod tidy 0-diff(0 新依赖);errcode manifest +2;pg_integration 编译进 gate(go build -tags pg_integration ✓)。

Next

  • CI 8 job 绿
  • 三路 post-impl review
  • FROZEN: registry ordinal 54 + 双仓 tag v0.54.0-stage2.54

Spec: spec-2.3a-db-query-tool.md (opendbrb PR #81)

sqlrush added 2 commits June 8, 2026 11:41
Why: the tool substrate bundled DB skills (spec-2.3b) drive — a
read-only SQL entry the LLM can call against the active PostgreSQL
connection. Realizes the spec-1.18 R-8 forward design (QueryConn as a
composed capability, db.Conn frozen untouched).

- db.QueryConn capability (QueryConn = Conn + Query) + QueryOptions /
  QueryResult (driver-side text-rendered rows; § 3.7 minimal common set)
- postgres: pgxPool seam +BeginTx; narrow queryTx/queryRows so fakes
  implement 2/5 methods not pgx's 11/9 (cr/codex HIGH-3); READ ONLY tx
  + always-ROLLBACK + extended-protocol single statement = server-side
  read-only (NOT zero side effects — residual advisory-lock/nextval/
  temp/remote documented, R-5); MaxRows+1 sentinel distinguishes
  exactly-100 from 101+ (codex HIGH-1); formatCell pinned type table
  via driver.Valuer (pgtype.Numeric→decimal, []byte→\x hex, time→
  RFC3339 — not fmt.Sprint garbage, cr/codex HIGH-2); classify +25006
  →READONLY_VIOLATION only (0A000 stays QUERY_FAILED, cr HIGH-1)
- db_query ToolExecutor (lowercase native name): two-track failures —
  ctx cancel/deadline checked FIRST → fatal Go error so the Loop honors
  spec-1.21 (codex CRIT-1: not a recoverable DB.TIMEOUT); lazy open
  with retryable first failure (zero startup DB I/O); capability-assert
  failure closes the Conn (no pool leak, codex HIGH-2); aligned text
  table + 16KiB UTF-8-safe cap (64KiB→16KiB removes the spec-2.3 R-7
  conflict; rune-safe cut, codex MED-1)
- bootstrap: DBQueryExecutors registers only when a connection is
  selectable (no open); ambiguous vs none differentiated log (cr MED-2);
  warnIfInsecureSSL → file-only WarnForceFile (no TUI tear, codex MED-3)
- 2 new errcodes (Chinese, same DB package convention — user Q11);
  driver.go capability note errata spec-2.1→2.3a
- integration: fake QueryConn full chain + ctx-fatal two-track +
  dedup replay + spec-2.3 ToolFilter interplay; pg_integration parked
  (real 25006 / multi-statement reject / pg_stat / numeric)

Coverage: dbquery 95.4% / db 100% / postgres 97.3% / bootstrap 89.2%.
Zero new deps; existing 1.18/1.19 tests unmodified (fakes add methods).

Spec: spec-2.3a-db-query-tool.md
Why: claude post-impl 双路 review (code-reviewer + go-reviewer 均
APPROVE-WITH-FIXES, 0 CRIT/0 HIGH) findings absorb:

- go MED-1: Rollback uses context.WithoutCancel — a cancelled query ctx
  no longer makes pgx hard-close (die) the pooled conn on ROLLBACK;
  the conn returns cleanly to the pool (connection churn under cancel)
- go MED-2: renderValue explicit int16/32/64/uint32/float32/64 cases via
  strconv — float8 columns (pg_stat checkpoint_write_time) now render
  '3600000' not '3.6e+06'; + golden case float64(3.6e6)/int32
- cr MED-1: driver.Valuer branch adds explicit string assertion
  (pgtype.Numeric → decimal) before the fmt.Sprint fallback
- cr HIGH-1: TestWarnIfInsecureSSL_FileOnly — asserts the SSL warning
  reaches the debug file but NEVER stderr (TUI no-tear; DoD gap)
- cr HIGH-2: TestNewPoolUsesExtendedProtocol + newPool INVARIANT comment
  guarding against QueryExecModeSimpleProtocol (multi-statement gate)
- cr MED-2: TestDBQuery_SkillScopeDenied — db_query outside a skill's
  allowed-tools returns SCOPE_TOOL_DENIED (ToolFilter regression)
- go LOW-1: fakeTx/fakeRows comment listing the methods Query touches

make gate PASSED (the TestDiagnoseLoop_NoToolRegression flake is the
pre-existing issue #97, unrelated — this spec does not touch that path).

Spec: spec-2.3a-db-query-tool.md
@sqlrush

sqlrush commented Jun 8, 2026

Copy link
Copy Markdown
Owner Author

Post-impl review trace — Route 1 (claude code-reviewer)

Reviewer: claude-code-reviewer (independent run)
Scope: full diff main...spec-2.3a @ 1fa72b9 + pgx v5.7.6 源 (pgconn ctx.Done/ROLLBACK, pgxpool Release)
Findings:
  CRIT: 0  HIGH: 2  MED: 2  LOW: 3
  HIGH-1 SSL no-tear 测试缺(DoD gap) / HIGH-2 QueryExecModeSimpleProtocol invariant 注释+测试缺
  MED-1 renderValue Valuer 加 string 断言 / MED-2 db_query scope-denied 集成测试缺
  LOW (确认非问题): Rollback cancelled-ctx 经 pgx die 安全 / timestamptz 走 time.Time case / sentinel+Close 正确
Verdict: APPROVE-WITH-FIXES (核心 two-track/readonly/sentinel/classify-25006/capability-close/lazy-retry/UTF8-cap 正确)

Post-impl review trace — Route 2 (claude go-reviewer)

Reviewer: claude-go-reviewer (independent run; 不同入口)
Scope: 同 diff; go vet/staticcheck/golangci 0 issues; race clean
Findings:
  CRIT: 0  HIGH: 0  MED: 2  LOW: 1  NOTED: 4
  MED-1 Rollback(ctx) 用 cancelled ctx → pgx die 弃连接 → 应 context.WithoutCancel(连接 churn)
  MED-2 float64 走 default fmt.Sprint → 科学计数 '3.6e+06' → 显式 case + strconv FormatFloat 'f'
  LOW-1 fake embed nil interface 注释 / NOTED: mutex-over-IO 已文档化 / errDriverNoQuery 内部 sentinel OK / defer 序正确
Verdict: APPROVE-WITH-FIXES

R-fix (cef12a2) — 全部 absorb

Finding Fix
go MED-1 Rollback cancelled-ctx context.WithoutCancel(ctx) — 连接干净回池
go MED-2 float 科学计数 renderValue 显式 int/float case + strconv;golden +float64(3.6e6)→'3600000'
cr MED-1 Valuer string 断言 Numeric→decimal 显式分支先于 fmt.Sprint
cr HIGH-1 SSL no-tear TestWarnIfInsecureSSL_FileOnly(file 有/stderr 无)
cr HIGH-2 SimpleProtocol invariant TestNewPoolUsesExtendedProtocol + newPool INVARIANT 注释
cr MED-2 scope-denied TestDBQuery_SkillScopeDenied(SCOPE_TOOL_DENIED 回归)
go LOW-1 fake 注释 fakeTx/fakeRows 方法清单注释

make gate PASSED;覆盖率 dbquery 95.4% / postgres 92.4% / bootstrap 89.2%。

TestDiagnoseLoop_NoToolRegression 偶发 flake = pre-existing issue #97(本 spec 不碰该路径,隔离重跑 20× 绿)。

待 Route 3 (codex) post-impl + 用户 review → FROZEN(registry ordinal 54 + 双仓 tag v0.54.0-stage2.54)。

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