---
phase: 65.1-phase-65-code-review-remediation
plan: 03a
type: execute
wave: 1
depends_on: []
files_modified:
  - src/ddl/read_ffi.rs
  - src/ddl/list.rs
  - src/ddl/describe.rs
  - src/ddl/show_columns.rs
  - src/ddl/show_dims.rs
  - src/ddl/show_dims_for_metric.rs
  - src/ddl/show_metrics.rs
  - src/ddl/show_facts.rs
  - src/ddl/show_materializations.rs
  - src/ddl/get_ddl.rs
  - src/ddl/read_yaml.rs
autonomous: true
requirements:
  - REM-WR-05
  - REM-IN-06

must_haves:
  truths:
    - "A `BorrowedConnection` newtype exists in `src/ddl/read_ffi.rs` with `#[repr(transparent)]`, no `Drop`, no `Clone`, and an `unsafe fn new(ffi::duckdb_connection) -> Self` constructor"
    - "All 13 ddl/ dispatcher functions wrap the raw FFI `conn` parameter into a `BorrowedConnection` via `unsafe { BorrowedConnection::new(conn) }` immediately on entry"
    - "Helper functions `probe_catalog_table_present`, `CatalogReader::new` accept `&BorrowedConnection` rather than raw `ffi::duckdb_connection`"
    - "A `compile_fail` doctest on `BorrowedConnection` proves that `ffi::duckdb_disconnect(&mut borrowed)` does NOT type-check (W-02 fix: target the exact `*mut duckdb_connection` shape duckdb_disconnect expects)"
    - "IN-06 (D-26): the duplicate `probe_catalog_table_present` in `src/ddl/list.rs` and the duplicate `write_err` are DELETED in this plan (absorbed from former Plan 09 scope per plan-checker B-04). Single caller in `sv_list_semantic_views_bind_rust` imports the canonical helpers from `read_ffi`."
  artifacts:
    - path: src/ddl/read_ffi.rs
      provides: "BorrowedConnection newtype + W-02-fixed compile_fail doctest + as_raw()/is_null() accessors. probe_catalog_table_present + write_err migrated to &BorrowedConnection."
      contains: "BorrowedConnection"
    - path: src/ddl/list.rs
      provides: "All dispatchers consume BorrowedConnection; duplicate probe_catalog_table_present + write_err deleted (IN-06); imports from read_ffi"
      contains: "use crate::ddl::read_ffi::"
  key_links:
    - from: src/ddl/list.rs
      to: src/ddl/read_ffi.rs::BorrowedConnection
      via: "wrap-on-entry pattern at the top of each dispatcher body"
      pattern: "BorrowedConnection::new"
    - from: src/ddl/list.rs::sv_list_semantic_views_bind_rust
      to: src/ddl/read_ffi.rs::probe_catalog_table_present
      via: "use crate::ddl::read_ffi::probe_catalog_table_present (IN-06 D-26 dedup)"
      pattern: "read_ffi::probe_catalog_table_present"
---

<objective>
Introduce the `BorrowedConnection` newtype (CONTEXT D-10) and migrate all 13 `src/ddl/*.rs` dispatchers to consume it. The FFI signature stays `conn: libduckdb_sys::duckdb_connection` (the C++ side passes a raw handle); the newtype is constructed inside the function body via `let borrowed = unsafe { BorrowedConnection::new(conn) };`. Anywhere downstream that needs the raw handle reaches into `.as_raw()`. The defining property: `ffi::duckdb_disconnect` takes `*mut ffi::duckdb_connection` not `*mut BorrowedConnection`, so the call simply does not type-check.

**B-04 absorption**: Per plan-checker, IN-06 (D-26) dedup work for `src/ddl/list.rs` moves into this plan (formerly Plan 09 Task 2). This avoids the brittle byte-diff coupling between Plan 03 and Plan 09 — the BorrowedConnection migration is the natural moment to converge the two `probe_catalog_table_present` copies onto the canonical `src/ddl/read_ffi.rs` version. Migrate ONLY the canonical version; update `src/ddl/list.rs` callers (`sv_list_semantic_views_bind_rust:99` and `sv_list_terse_semantic_views_bind_rust:265`) to import the canonical from `read_ffi`; delete the duplicate `probe_catalog_table_present` and `write_err` from `list.rs`.

This plan is the **ddl/ half** of the WR-05 migration. Plan 03b handles `src/query/*.rs` dispatchers + the AST guard extension in `tests/no_long_lived_conn.rs`.

W-02 fix (plan-checker feedback): the compile_fail doctest uses the *exact* `*mut ffi::duckdb_connection` shape that `duckdb_disconnect` accepts, demonstrating that `&mut BorrowedConnection` cannot be coerced to it.

This plan is entirely Rust-side and does NOT modify any C++ shim. It can run in parallel with all other Wave 1 plans (01, 02a, 05).

Purpose: Eliminate the "code review only" enforcement of the BORROW contract. The per-call `Connection probe(*context.db)` constructed in C++ bind callbacks is a stack object — `duckdb_disconnect` on its bridged handle would `delete` a stack pointer (UB). Today only ~20 `reinterpret_cast<duckdb_connection>(&probe)` sites and code comments protect this; a contributor could regress trivially.

Output: ~13 dispatcher signatures updated (parameter wrap pattern); 1 new newtype + helper-function signature changes for `probe_catalog_table_present` (canonical only — duplicate deleted) + `CatalogReader::new`; 1 compile_fail doctest using the duckdb_disconnect-exact-shape; IN-06 dedup absorbed.

Implements D-10 (WR-05 ddl/ side), D-26 (IN-06).
</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/ddl/list.rs
</context>

<interfaces>
<!-- Target BorrowedConnection shape — PATTERNS.md lines 354-404 -->

```
#[repr(transparent)]
pub struct BorrowedConnection(ffi::duckdb_connection);

impl BorrowedConnection {
    #[must_use]
    pub unsafe fn new(conn: ffi::duckdb_connection) -> Self { Self(conn) }
    #[must_use]
    pub fn as_raw(&self) -> ffi::duckdb_connection { self.0 }
    #[must_use]
    pub fn is_null(&self) -> bool { self.0.is_null() }
}
// Deliberately NO Drop, NO Clone, NO impls that could lead to disconnect.
```

The 13 ddl/ dispatcher migration sites in this plan:

| # | File | Line | Symbol |
|---|------|------|--------|
| 1 | src/ddl/list.rs | 81 | sv_list_semantic_views_bind_rust |
| 2 | src/ddl/list.rs | 265 | sv_list_terse_semantic_views_bind_rust |
| 3 | src/ddl/describe.rs | 18 | sv_describe_semantic_view_bind_rust |
| 4 | src/ddl/show_columns.rs | 21 | sv_show_columns_in_semantic_view_bind_rust |
| 5 | src/ddl/show_dims.rs | 25 | sv_show_semantic_dimensions_all_bind_rust |
| 6 | src/ddl/show_dims.rs | 148 | sv_show_semantic_dimensions_bind_rust |
| 7 | src/ddl/show_dims_for_metric.rs | 21 | sv_show_semantic_dimensions_for_metric_bind_rust |
| 8 | src/ddl/show_metrics.rs | 16 | sv_show_semantic_metrics_all_bind_rust |
| 9 | src/ddl/show_metrics.rs | 139 | sv_show_semantic_metrics_bind_rust |
| 10 | src/ddl/show_facts.rs | 16 | sv_show_semantic_facts_all_bind_rust |
| 11 | src/ddl/show_facts.rs | 139 | sv_show_semantic_facts_bind_rust |
| 12 | src/ddl/show_materializations.rs | 18 | sv_show_semantic_materializations_all_bind_rust |
| 13 | src/ddl/show_materializations.rs | 130 | sv_show_semantic_materializations_bind_rust |
| 14 | src/ddl/get_ddl.rs | 37 | sv_get_ddl_exec_rust |
| 15 | src/ddl/read_yaml.rs | 42 | sv_read_yaml_from_semantic_view_exec_rust |

(15 sites total across ddl/. Plan 03b handles the remaining 2 in `src/query/*.rs` for a total of 17 across the full WR-05 migration.)

Secondary helpers to migrate in THIS plan (must accept `&BorrowedConnection`):
- src/ddl/read_ffi.rs:57 probe_catalog_table_present (canonical)
- CatalogReader::new (used at list.rs:99 and possibly elsewhere)

IN-06 dedup absorption:
- src/ddl/list.rs:~191-210 duplicate `probe_catalog_table_present` — DELETE
- src/ddl/list.rs:~216-... duplicate `write_err` — DELETE
- Update `sv_list_semantic_views_bind_rust:99` and `sv_list_terse_semantic_views_bind_rust:265` to import the canonical versions via `use crate::ddl::read_ffi::{probe_catalog_table_present, write_err};`

W-02 compile_fail doctest target:
The doctest must show that `ffi::duckdb_disconnect`'s required argument type (`*mut ffi::duckdb_connection`) cannot be constructed from `BorrowedConnection`. The plan-checker's recommended shape:

```rust
/// ```compile_fail
/// use duckdb_semantic_views::ddl::read_ffi::BorrowedConnection;
/// let mut b: BorrowedConnection = unsafe { std::mem::zeroed() };
/// // duckdb_disconnect takes *mut duckdb_connection. &mut BorrowedConnection
/// // is NOT *mut duckdb_connection, even though BorrowedConnection is
/// // #[repr(transparent)] wrapping duckdb_connection.
/// unsafe { libduckdb_sys::duckdb_disconnect(&mut b) };
/// ```
```

Verify the exact shape against the actual `libduckdb_sys` binding signature before committing — the canonical signature is `pub unsafe extern "C" fn duckdb_disconnect(connection: *mut duckdb_connection)`. Crate path: replace `duckdb_semantic_views` with the actual crate name from Cargo.toml (`grep "^name" Cargo.toml`).
</interfaces>

<tasks>

<task type="auto">
  <name>Task 1: Add BorrowedConnection newtype + W-02-fixed compile_fail doctest in src/ddl/read_ffi.rs</name>
  <read_first>
    - src/ddl/read_ffi.rs (full file — observe existing helper doc-comment style at lines 47-56)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 337-404 (full target shape)
    - Cargo.toml (confirm crate name for the doctest `use` path; e.g. `name = "duckdb-semantic-views"` → use `duckdb_semantic_views::...`)
    - libduckdb_sys docs for `duckdb_disconnect` — exact signature is `pub unsafe extern "C" fn duckdb_disconnect(connection: *mut duckdb_connection)` (verify by `grep -A1 "pub unsafe extern" target/doc/libduckdb_sys/fn.duckdb_disconnect.html` or by reading the binding file directly)
  </read_first>
  <files>src/ddl/read_ffi.rs</files>
  <action>
Append the `BorrowedConnection` newtype after the existing helpers (after `write_err` at the bottom of the file). The struct, its impl, and the doc comments must follow the shape in `<interfaces>` above and PATTERNS.md lines 354-404 exactly.

Mandatory doc-comment fields:
1. Top-level struct doc explaining the BORROW contract: handle is owned by C++ stack `Connection probe(*context.db)`; Rust must not `duckdb_disconnect`.
2. A `# Negative compile coverage` section containing a `/// ```compile_fail` fenced doctest that:
   - imports the type from the crate path (`use {crate_name}::ddl::read_ffi::BorrowedConnection;` — replace `{crate_name}` with actual from Cargo.toml)
   - constructs `let mut b: BorrowedConnection = unsafe { std::mem::zeroed() };` (use `let mut` because the duckdb_disconnect call needs `&mut`)
   - attempts `unsafe { libduckdb_sys::duckdb_disconnect(&mut b) };`
   - The doctest comment explains: `duckdb_disconnect` takes `*mut duckdb_connection`. `&mut BorrowedConnection` is NOT coercible to that even though `BorrowedConnection` is `#[repr(transparent)]` — Rust newtype distinctness is what blocks the coercion.
3. `# Safety` section on `new()` matching the existing `probe_catalog_table_present` doc style (lines 47-56).

**W-02 fix**: the doctest MUST use `let mut b` (mutable binding) so the `&mut b` doesn't fail to compile for the WRONG reason (immutable-binding error). The intended failure is the type mismatch between `&mut BorrowedConnection` and `*mut duckdb_connection`. Before committing, run the doctest manually (`cargo test --doc -p {crate-name} 2>&1 | grep -A 10 BorrowedConnection`) and confirm the failure message references the type mismatch, NOT a mutability error.

The struct MUST be `pub` (consumed by other modules) and MUST have:
- `#[repr(transparent)]`
- NO `#[derive(...)]` (especially no Clone/Copy)
- NO `Drop` impl
- ONLY `new()`, `as_raw()`, `is_null()` methods

Add nothing else — no helper methods like `query()` or `prepare()`. Those would defeat the type-level guard by encouraging passing `BorrowedConnection` around without unwrapping intent.

DO NOT modify `probe_catalog_table_present` yet — Task 2 does that as part of the helper migration.
  </action>
  <verify>
    <automated>cargo build &gt; /tmp/claude/p03a_t1_build.log 2>&amp;1; RC=$?; tail -40 /tmp/claude/p03a_t1_build.log; test $RC -eq 0 &amp;&amp; grep -q "compile_fail" src/ddl/read_ffi.rs &amp;&amp; grep -q "repr(transparent)" src/ddl/read_ffi.rs &amp;&amp; grep -q "pub struct BorrowedConnection" src/ddl/read_ffi.rs &amp;&amp; cargo test --doc 2&gt;&amp;1 | head -50</automated>
  </verify>
  <acceptance_criteria>
    - `src/ddl/read_ffi.rs` contains `#[repr(transparent)] pub struct BorrowedConnection(ffi::duckdb_connection);`
    - `impl BorrowedConnection` has exactly three methods: `unsafe fn new`, `fn as_raw`, `fn is_null`
    - No `Drop`, `Clone`, `Copy`, or other trait impls on `BorrowedConnection` (`grep -E "impl\\s+\\w+\\s+for\\s+BorrowedConnection" src/ddl/read_ffi.rs` returns zero matches)
    - A `compile_fail` fenced doctest block appears in the struct doc comment
    - The doctest uses `let mut b: BorrowedConnection = unsafe { std::mem::zeroed() };` (mutable binding)
    - The doctest attempts `duckdb_disconnect(&mut b)` and is annotated explaining the type mismatch (not mutability)
    - `cargo build` exits 0
    - When `cargo test --doc` runs the compile_fail, the failure (verified manually) is a type-mismatch error referencing `*mut duckdb_connection`, NOT an immutable-binding error
  </acceptance_criteria>
  <done>BorrowedConnection newtype exists with W-02-fixed negative-compile doctest targeting the exact duckdb_disconnect signature; build green.</done>
</task>

<task type="auto">
  <name>Task 2: Migrate all 15 ddl/ dispatcher sites + helper functions; absorb IN-06 dedup</name>
  <read_first>
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 408-461 (canonical wrap-on-entry pattern from src/ddl/list.rs)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 451-481 (full migration site table + helper enumeration)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-RESEARCH.md lines 765-767 (IN-06 D-26 detail)
    - src/ddl/list.rs (Wave 0 canonical spike — adapt this shape to the other dispatcher sites; also locate the two duplicate helpers ~191-210 and ~216-...)
    - src/ddl/read_ffi.rs lines 57-95 (the canonical `probe_catalog_table_present` and `write_err`)
    - All 11 dispatcher files in files_modified
  </read_first>
  <files>
    src/ddl/list.rs, src/ddl/describe.rs, src/ddl/show_columns.rs, src/ddl/show_dims.rs,
    src/ddl/show_dims_for_metric.rs, src/ddl/show_metrics.rs, src/ddl/show_facts.rs,
    src/ddl/show_materializations.rs, src/ddl/get_ddl.rs, src/ddl/read_yaml.rs,
    src/ddl/read_ffi.rs
  </files>
  <action>
**Part A — BorrowedConnection migration (15 ddl/ dispatcher sites):**

For EACH of the 15 ddl/ 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.

**Part B — Helper function migration:**

- `src/ddl/read_ffi.rs::probe_catalog_table_present(conn: ffi::duckdb_connection) -> bool` becomes `probe_catalog_table_present(borrowed: &BorrowedConnection) -> bool`. Body uses `borrowed.as_raw()` wherever it currently uses `conn` for `ffi::duckdb_query`/`ffi::duckdb_prepare` calls.
- `CatalogReader::new(conn, table_present)` — same signature change. Search for all `CatalogReader::new(` callers and update them to pass `&borrowed`.

If any callsite passes the raw `conn` to a helper that's NOT yet migrated, migrate the helper too (no half-migrated state — the type system will catch this).

**Part C — IN-06 dedup absorption (D-26):**

1. **Verify byte-identical duplication** by diffing the two helper bodies (after Part B migration shape):
   - `src/ddl/list.rs:~191-210` `probe_catalog_table_present` vs the now-migrated `src/ddl/read_ffi.rs:~57-74` canonical
   - `src/ddl/list.rs:~216-...` `write_err` vs `src/ddl/read_ffi.rs:~84-95` canonical
   
   If they have diverged in non-trivial ways (e.g. different error messages, different param order), halt and report — do NOT merge silently.

2. **Update list.rs callers** (`sv_list_semantic_views_bind_rust:~99`, `sv_list_terse_semantic_views_bind_rust:~265`) to import the canonical versions:
   ```rust
   use crate::ddl::read_ffi::{probe_catalog_table_present, write_err};
   ```
   (Or convert to fully-qualified `crate::ddl::read_ffi::probe_catalog_table_present` at the call site — whichever matches the existing import style of the file.)

3. **Delete the duplicate definitions** in `src/ddl/list.rs` — the entire `fn probe_catalog_table_present(...)` block (~191-210) and the entire `fn write_err(...)` block (~216-... — find the end via brace matching).

4. Verify all other usages in list.rs work with the imported canonical versions.

Note: `src/query/table_function.rs::try_infer_schema` and `src/query/explain.rs::collect_explain_lines` are migrated in Plan 03b (this plan stays scoped to ddl/).

Verify incrementally with `cargo build` between files; CLAUDE.md Rule 1 — redirect to file then tail.
  </action>
  <verify>
    <automated>cargo build &gt; /tmp/claude/p03a_t2_build.log 2>&amp;1; RC=$?; tail -80 /tmp/claude/p03a_t2_build.log; test $RC -eq 0 &amp;&amp; cargo test &gt; /tmp/claude/p03a_t2_test.log 2>&amp;1; RC2=$?; tail -50 /tmp/claude/p03a_t2_test.log; test $RC2 -eq 0 &amp;&amp; ! grep -q "^fn probe_catalog_table_present" src/ddl/list.rs &amp;&amp; ! grep -q "^fn write_err" src/ddl/list.rs</automated>
  </verify>
  <acceptance_criteria>
    - `grep -rE "BorrowedConnection::new\\(conn\\)" src/ddl/ | wc -l` returns at least 15 (one per ddl/ dispatcher)
    - `grep -rE "\\bconn\\.is_null\\(\\)" src/ddl/` returns zero matches inside dispatcher bodies (replaced with `borrowed.is_null()`)
    - `probe_catalog_table_present` signature in `src/ddl/read_ffi.rs` takes `&BorrowedConnection`
    - `CatalogReader::new` accepts `&BorrowedConnection`
    - `src/ddl/list.rs` contains ZERO `fn probe_catalog_table_present` definitions (only imports/usages)
    - `src/ddl/list.rs` contains ZERO `fn write_err` definitions (only imports/usages)
    - Both `sv_list_semantic_views_bind_rust` and `sv_list_terse_semantic_views_bind_rust` call the canonical versions from `read_ffi`
    - `cargo build` exits 0
    - `cargo test` exits 0 — no behavioural regressions in unit/proptest/AST-guard tests
  </acceptance_criteria>
  <done>All 15 ddl/ dispatchers + helpers consume `&BorrowedConnection`; duplicates in list.rs deleted (IN-06 absorbed); no bare `conn` references remain inside dispatcher bodies; Rust test suite passes.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| C++ ↔ Rust FFI (ddl/ bind/exec dispatchers) | C++ passes a borrowed `duckdb_connection` (bridged from a stack `Connection probe`); Rust must not delete it |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-65.1-03a-01 | Tampering | Stack-Connection lifetime UB via `duckdb_disconnect` on borrowed handle | mitigate | `BorrowedConnection` newtype: `ffi::duckdb_disconnect` accepts `*mut ffi::duckdb_connection` not the newtype — call simply does not type-check (D-10). AST-walk extension lives in Plan 03b |
| T-65.1-03a-02 | Tampering | Contributor reintroduces duplicate `probe_catalog_table_present` after dedup | accept | Single canonical home + import-based call sites make divergence visible at code review |
</threat_model>

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

<success_criteria>
- `BorrowedConnection` newtype defined in `src/ddl/read_ffi.rs` with `repr(transparent)`, no `Drop`/`Clone`
- W-02-fixed compile_fail doctest proves `duckdb_disconnect(&mut b)` cannot compile due to type mismatch (not mutability)
- All 15 ddl/ dispatcher sites consume `BorrowedConnection` via wrap-on-entry
- Helper functions (`probe_catalog_table_present`, `CatalogReader::new`) accept `&BorrowedConnection`
- IN-06 absorbed: duplicates in list.rs deleted, canonical imports in place
- `just test-all` green
</success_criteria>

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