---
phase: 65.1-phase-65-code-review-remediation
plan: 03b
type: execute
wave: 2
depends_on:
  - 65.1-03a
files_modified:
  - src/query/explain.rs
  - src/query/table_function.rs
  - tests/no_long_lived_conn.rs
  - Cargo.toml
autonomous: true
requirements:
  - REM-WR-05

must_haves:
  truths:
    - "Both `src/query/*.rs` dispatchers wrap the raw FFI `conn` parameter into a `BorrowedConnection` via `unsafe { BorrowedConnection::new(conn) }` immediately on entry"
    - "Helper functions `collect_explain_lines`, `try_infer_schema` accept `&BorrowedConnection` rather than raw `ffi::duckdb_connection`"
    - "`tests/no_long_lived_conn.rs` extended with `DisconnectFinder` AST walker that walks all of `src/**/*.rs` for any call expression whose terminal path segment is `duckdb_disconnect`; passes (finds zero call sites)"
    - "`walkdir` is in `[dev-dependencies]` of Cargo.toml (added if not present)"
  artifacts:
    - path: src/query/table_function.rs
      provides: "sv_semantic_view_bind_rust + try_infer_schema migrated to BorrowedConnection"
      contains: "BorrowedConnection::new"
    - path: src/query/explain.rs
      provides: "sv_explain_semantic_view_bind_rust + collect_explain_lines migrated to BorrowedConnection"
      contains: "BorrowedConnection::new"
    - path: tests/no_long_lived_conn.rs
      provides: "DisconnectFinder AST guard walking all of src/"
      contains: "duckdb_disconnect"
  key_links:
    - from: tests/no_long_lived_conn.rs::DisconnectFinder
      to: src/**/*.rs
      via: "AST walk for terminal-segment ident match `duckdb_disconnect`"
      pattern: "duckdb_disconnect"
---

<objective>
Complete the WR-05 BorrowedConnection migration from Plan 03a by:

1. Migrating the remaining 2 dispatchers in `src/query/*.rs` (`sv_explain_semantic_view_bind_rust` at `explain.rs:98`, `sv_semantic_view_bind_rust` at `table_function.rs:101`) to the BorrowedConnection wrap-on-entry pattern.
2. Migrating the helper functions `collect_explain_lines` and `try_infer_schema` to accept `&BorrowedConnection`.
3. Extending `tests/no_long_lived_conn.rs` with a second AST walker (`DisconnectFinder`) per CONTEXT D-11 — defence in depth alongside the type-level enforcement.

Note: Plan 11 (WR-08) further changes `try_infer_schema`'s return type to `Result<..., String>`. This plan only changes the signature to accept `&BorrowedConnection`; Plan 11 stacks the Result change on top.

Wave 2 because it depends on Plan 03a (the `BorrowedConnection` newtype must exist in `src/ddl/read_ffi.rs` first).

This plan is entirely Rust-side and does NOT modify any C++ shim.

Implements D-10 (WR-05 query/ side) + D-11 (WR-05 AST defence in depth).
</objective>

<execution_context>
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
@$HOME/.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/STATE.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-CONTEXT.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md
@.planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md
@src/ddl/read_ffi.rs
@src/query/table_function.rs
@src/query/explain.rs
@tests/no_long_lived_conn.rs
</context>

<interfaces>
The 2 query/ dispatcher migration sites (completes the 17-site WR-05 migration started in Plan 03a):

| # | File | Line | Symbol |
|---|------|------|--------|
| 16 | src/query/explain.rs | 98 | sv_explain_semantic_view_bind_rust |
| 17 | src/query/table_function.rs | 101 | sv_semantic_view_bind_rust |

Secondary helpers to migrate (must accept `&BorrowedConnection`):
- src/query/explain.rs:~340 collect_explain_lines
- src/query/table_function.rs:~438, ~705 try_infer_schema (and any sibling helper)

DisconnectFinder AST walker (PATTERNS.md lines 551-633):

```
struct DisconnectFinder {
    findings: Vec<(String, String)>,
    current_file: String,
}

impl<'ast> syn::visit::Visit<'ast> for DisconnectFinder {
    fn visit_expr_call(&mut self, node: &'ast syn::ExprCall) {
        if let syn::Expr::Path(p) = &*node.func {
            if let Some(last_seg) = p.path.segments.last() {
                if last_seg.ident == "duckdb_disconnect" {
                    let path = quote::quote!(#p).to_string();
                    self.findings.push((self.current_file.clone(), path));
                }
            }
        }
        syn::visit::visit_expr_call(self, node);
    }
}

#[test]
fn no_duckdb_disconnect_anywhere_in_src() {
    let mut finder = DisconnectFinder { findings: vec![], current_file: String::new() };
    for entry in walkdir::WalkDir::new("src/") {
        let entry = entry.unwrap();
        if entry.path().extension().and_then(|e| e.to_str()) != Some("rs") { continue; }
        let content = std::fs::read_to_string(entry.path()).unwrap_or_default();
        let Ok(file) = syn::parse_str::<syn::File>(&content) else { continue };
        finder.current_file = entry.path().display().to_string();
        finder.visit_file(&file);
    }
    assert!(
        finder.findings.is_empty(),
        "duckdb_disconnect calls found in src/ — these break the BORROW contract. \
         Use BorrowedConnection (Phase 65.1 D-10 + D-11). Findings: {:#?}",
        finder.findings,
    );
}
```
</interfaces>

<tasks>

<task type="auto">
  <name>Task 1: Migrate the 2 query/ dispatchers + helper functions</name>
  <read_first>
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 408-461 (canonical wrap-on-entry pattern)
    - src/query/explain.rs (full file — find sv_explain_semantic_view_bind_rust + collect_explain_lines)
    - src/query/table_function.rs (full file — find sv_semantic_view_bind_rust + try_infer_schema; note Plan 11 will further change try_infer_schema's return type)
    - src/ddl/read_ffi.rs (Plan 03a established BorrowedConnection here — confirm pub visibility)
    - src/ddl/list.rs (post-Plan-03a — example wrap-on-entry pattern for the 2 query/ dispatchers to mirror)
  </read_first>
  <files>src/query/explain.rs, src/query/table_function.rs</files>
  <action>
For EACH of the 2 query/ dispatcher symbols enumerated in `<interfaces>`:

1. Keep the `extern "C"` FFI signature unchanged (the C++ side passes a raw `libduckdb_sys::duckdb_connection`).
2. Inside the function body, IMMEDIATELY after the `let result = std::panic::catch_unwind(AssertUnwindSafe(|| { ... }))` opener, add:
   ```
   use crate::ddl::read_ffi::BorrowedConnection;
   let borrowed = unsafe { BorrowedConnection::new(conn) };
   ```
3. Replace the existing `if conn.is_null() { write_err(...); return 1_u8; }` early-out with `if borrowed.is_null() { write_err(...); return 1_u8; }`.
4. EVERY subsequent reference to `conn` inside the body must be replaced with EITHER `&borrowed` (when calling a helper that accepts `&BorrowedConnection`) or `borrowed.as_raw()` (when calling raw FFI like `ffi::duckdb_query`).
5. Do NOT leave any bare `conn` reference inside the body after the `let borrowed = ...` line.

For helper functions, change signatures so they accept `&BorrowedConnection`:

- `src/query/explain.rs::collect_explain_lines(conn, sql)` then `collect_explain_lines(borrowed: &BorrowedConnection, sql: &str)`. Body uses `borrowed.as_raw()` wherever the previous body used `conn`.
- `src/query/table_function.rs::try_infer_schema(conn, sql)` then `try_infer_schema(borrowed: &BorrowedConnection, sql: &str)` (signature change only — return type stays `Option<...>` in this plan; Plan 11 changes it to `Result<_, String>`).

If any callsite passes the raw `conn` to a helper that's NOT yet migrated, migrate it. Re-grep to confirm no bare `conn` references remain in dispatcher bodies after the wrap.

Verify incrementally with `cargo build` between files; CLAUDE.md Rule 1.
  </action>
  <verify>
    <automated>cargo build &gt; /tmp/claude/p03b_t1_build.log 2>&amp;1; RC=$?; tail -60 /tmp/claude/p03b_t1_build.log; test $RC -eq 0 &amp;&amp; cargo test &gt; /tmp/claude/p03b_t1_test.log 2>&amp;1; RC2=$?; tail -50 /tmp/claude/p03b_t1_test.log; test $RC2 -eq 0</automated>
  </verify>
  <acceptance_criteria>
    - `grep -rE "BorrowedConnection::new\\(conn\\)" src/query/ | wc -l` returns at least 2 (one per query/ dispatcher)
    - `grep -rE "\\bconn\\.is_null\\(\\)" src/query/` returns zero matches inside dispatcher bodies
    - `collect_explain_lines` and `try_infer_schema` signatures accept `&BorrowedConnection`
    - `cargo build` + `cargo test` both exit 0 — no regressions
  </acceptance_criteria>
  <done>Both query/ dispatchers + helpers consume `&BorrowedConnection`; 17-site WR-05 migration complete across ddl/ + query/.</done>
</task>

<task type="auto">
  <name>Task 2: Extend tests/no_long_lived_conn.rs with DisconnectFinder walking all of src/</name>
  <read_first>
    - tests/no_long_lived_conn.rs (existing Finder struct + Visit impl at lines 60-99 — the template)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 551-633 (target DisconnectFinder shape)
    - Cargo.toml (confirm `walkdir` is in dev-dependencies; if not, add it)
  </read_first>
  <files>tests/no_long_lived_conn.rs, Cargo.toml</files>
  <action>
1. Inspect `Cargo.toml`'s `[dev-dependencies]`. If `walkdir` is not listed, add it:
   ```
   walkdir = "2"
   ```

2. In `tests/no_long_lived_conn.rs`, add a new `struct DisconnectFinder` and `impl<'ast> Visit<'ast> for DisconnectFinder` that follows the existing `Finder` template (lines 60-99) but:
   - Is NOT scoped to `init_extension` — it walks every function
   - Looks for any `syn::ExprCall` whose terminal path segment ident equals `duckdb_disconnect`
   - Records the source file path + the full call path

3. Add a new `#[test] fn no_duckdb_disconnect_anywhere_in_src()` that:
   - Walks `src/` recursively via `walkdir::WalkDir::new("src/")`
   - For each file ending in `.rs`, parses with `syn::parse_str::<syn::File>(...)` (skip on parse error)
   - Runs the `DisconnectFinder` Visit
   - Collects all findings into a Vec
   - Asserts the Vec is empty with a message naming `BorrowedConnection` and D-10 + D-11

The full target body is in PATTERNS.md lines 581-633 — adapt verbatim.

DO NOT modify the existing `Finder` / `no_long_lived_connection_in_init_extension` test. The existing test stays as the `duckdb_connect`-in-`init_extension` guard. The new test is the `duckdb_disconnect`-anywhere-in-src guard.

DO NOT add `#[ignore]` — this test MUST run by default and MUST pass once Plan 03a + Plan 03b Task 1 land (no `duckdb_disconnect` calls anywhere in src/, the newtype prevents it).
  </action>
  <verify>
    <automated>cargo test --test no_long_lived_conn &gt; /tmp/claude/p03b_t2_ast.log 2>&amp;1; RC=$?; tail -30 /tmp/claude/p03b_t2_ast.log; test $RC -eq 0 &amp;&amp; grep -q "no_duckdb_disconnect_anywhere_in_src" tests/no_long_lived_conn.rs</automated>
  </verify>
  <acceptance_criteria>
    - `Cargo.toml` `[dev-dependencies]` lists `walkdir`
    - `tests/no_long_lived_conn.rs` defines `struct DisconnectFinder` with `Visit` impl
    - One new `#[test] fn no_duckdb_disconnect_anywhere_in_src()` exists
    - Test body uses `walkdir::WalkDir::new("src/")` and `syn::parse_str::<syn::File>(...)`
    - Existing `Finder` and `no_long_lived_connection_in_init_extension` test still present and unchanged in behaviour
    - `cargo test --test no_long_lived_conn` passes both tests (output shows 2 passed)
    - The failure-message string in the new test references `BorrowedConnection` (so the diagnostic on regression is clear)
  </acceptance_criteria>
  <done>AST-walk defence in depth: `duckdb_disconnect` cannot appear in src/ without the new test failing.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| C++ ↔ Rust FFI (query/ bind dispatchers) | C++ passes a borrowed `duckdb_connection`; Rust must not delete it |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-65.1-03b-01 | Tampering | Contributor introduces `ffi::duckdb_disconnect` on a raw `conn` parameter before wrapping | mitigate | AST walker in `no_long_lived_conn.rs` catches any `duckdb_disconnect` call anywhere in `src/` (D-11) |
| T-65.1-03b-02 | Tampering | AST walker mis-identifies a `use … as duckdb_disconnect;` alias | accept | Documented limitation per Phase 65 D-22 — bounded scope, defence in depth only; type-level guard via BorrowedConnection is the primary mechanism |
</threat_model>

<verification>
  <automated>just test-all &gt; /tmp/claude/p03b_all.log 2>&amp;1; RC=$?; tail -100 /tmp/claude/p03b_all.log; test $RC -eq 0</automated>
</verification>

<success_criteria>
- Both query/ dispatcher sites consume `BorrowedConnection` via wrap-on-entry
- Helper functions (`collect_explain_lines`, `try_infer_schema`) accept `&BorrowedConnection`
- `tests/no_long_lived_conn.rs` extended with `DisconnectFinder` test that walks all of `src/` for `duckdb_disconnect` and passes
- `just test-all` green
</success_criteria>

<output>
Create `.planning/phases/65.1-phase-65-code-review-remediation/65.1-03b-SUMMARY.md` when done.
</output>
