---
phase: 65.1-phase-65-code-review-remediation
plan: 02a
type: execute
wave: 1
depends_on: []
files_modified:
  - cpp/src/shim.cpp
  - cpp/src/shim.hpp
autonomous: true
requirements:
  - REM-WR-02
  - REM-CR-02

must_haves:
  truths:
    - "Both `sv_register_table_function` and `sv_register_scalar_function` accept `(char *error_buf, size_t error_buf_len)` trailing parameters"
    - "Registration failures (including null required args, exceptions in CreateTableFunction) write the underlying `e.what()` text into `error_buf` and return `false`"
    - "`sv_register_table_function` refuses null `init_cb` at registration time and writes a clear `null required argument (init_cb is mandatory)` message into `error_buf`"
    - "The `(void)emitted; // single-shot semantics — even without local state we emit once per bind` misleading comment at cpp/src/shim.cpp:1148 is deleted (D-06)"
    - "All ~17 internal `sv_register_*` wrappers in shim.cpp carry the trailing error_buf pair and forward it to the inner helper"
  artifacts:
    - path: cpp/src/shim.hpp
      provides: "Updated extern declarations for sv_register_table_function + sv_register_scalar_function with error_buf trailing pair"
      contains: "char *error_buf, size_t error_buf_len"
    - path: cpp/src/shim.cpp
      provides: "Implementations with snprintf into error_buf + null-init refusal (D-05) in sv_register_table_function + D-06 comment deletion"
      contains: "init_cb is mandatory"
  key_links:
    - from: cpp/src/shim.cpp::sv_register_table_function
      to: cpp/src/shim.cpp::sv_register_*_wrappers
      via: "wrappers forward caller's error_buf into inner helper"
      pattern: "error_buf, error_buf_len"
---

<objective>
Rework the C ABI of `sv_register_table_function` and `sv_register_scalar_function` in `cpp/src/shim.hpp` + `cpp/src/shim.cpp` so registration failures surface their underlying DuckDB exception text via a `(char *error_buf, size_t error_buf_len)` trailing-pair parameter — the same convention already used by `sv_parser_override_rust` (shim.cpp:57-61). Tighten `sv_register_table_function` to refuse `init_cb == nullptr` at registration time (CR-02 D-05). Delete the misleading `(void)emitted` comment at cpp/src/shim.cpp:1148 (D-06).

This is the **C++-only half** of the WR-02 work. Plan 02b consumes this new ABI from the Rust side (`src/lib.rs` call sites + integration test population). Splitting the ABI rework from the call-site cascade keeps each plan within context budget and means the 17-wrapper edit + test population live in their own scoped plan.

Purpose: Today registration failures `fprintf(stderr, ...)` and return `false`; the caller in `init_extension` then returns a generic `"Failed to register X"` message that drops the underlying cause. ADBC/JDBC/Python clients with redirected stderr lose the diagnostic entirely. Per CONTEXT D-08/D-09: error_buf is the only ABI-stable channel.

Output: Updated C ABI surface; ~17 wrappers updated to forward the buffer pair; null-init_cb refusal active; (void)emitted comment deleted. No Rust changes; intermediate Rust build will fail compile until Plan 02b lands — that's expected (this plan is the C-side of a paired change).

Implements D-08, D-09 (WR-02) + D-05 (CR-02 registration-time enforcement) + D-06 (delete `(void)emitted` misleading comment at cpp/src/shim.cpp:1148).
</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
@.planning/phases/65-overridecontext-connection-teardown/65-REVIEW.md
@cpp/src/shim.hpp
@CLAUDE.md
</context>

<interfaces>
<!-- Anchor signatures the executor must reproduce. PATTERNS.md lines 142-205 have the full target shape. -->

Existing pattern (template) — `cpp/src/shim.cpp:57-61`:
```
uint8_t sv_parser_override_rust(
    const void *ctx_ptr,
    const char *query_ptr, size_t query_len,
    char **sql_out_ptr, size_t *sql_out_len,
    char *error_out, size_t error_out_len);
```

Existing `sv_register_table_function` signature — `cpp/src/shim.cpp:562-570` + `cpp/src/shim.hpp` declaration:
```
bool sv_register_table_function(
    duckdb_database db_handle,
    const char *name,
    const LogicalType *arg_types,
    size_t arg_count,
    table_function_bind_t bind_cb,
    table_function_t exec_cb,
    table_function_init_local_t init_cb);
```

Target signature (add trailing pair):
```
bool sv_register_table_function(
    duckdb_database db_handle,
    const char *name,
    const LogicalType *arg_types,
    size_t arg_count,
    table_function_bind_t bind_cb,
    table_function_t exec_cb,
    table_function_init_local_t init_cb,
    char *error_buf, size_t error_buf_len);
```
</interfaces>

<tasks>

<task type="auto">
  <name>Task 1: Rework C ABI in shim.hpp + shim.cpp (sv_register_table_function + sv_register_scalar_function)</name>
  <read_first>
    - cpp/src/shim.hpp (extern declarations — the WR-02 target lives at lines 68-75 + 95-101 per PATTERNS.md line 20)
    - cpp/src/shim.cpp lines 57-61 (template — `sv_parser_override_rust` extern declaration with `(char *error_out, size_t error_out_len)` trailing pair)
    - cpp/src/shim.cpp lines 562-689 (existing `sv_register_table_function` and `sv_register_scalar_function` bodies)
    - cpp/src/shim.cpp around lines 411-420, 783-784, 973-974 (existing caller-side `char error_buf[1024]; std::memset(error_buf, 0, sizeof(error_buf));` allocation pattern)
    - cpp/src/shim.cpp line 1148 (the `(void)emitted; // single-shot semantics — even without local state we emit once per bind` comment to delete per D-06)
    - .planning/phases/65.1-phase-65-code-review-remediation/65.1-PATTERNS.md lines 142-205 (full target shape for WR-02)
  </read_first>
  <files>cpp/src/shim.hpp, cpp/src/shim.cpp</files>
  <action>
A. **shim.hpp** — locate the extern declarations for `sv_register_table_function` and `sv_register_scalar_function` (research notes them at lines 68-75 and 95-101). Append `, char *error_buf, size_t error_buf_len` to both. Add a doc-comment block above each citing CONTEXT D-08/D-09 and noting that the buffer MUST be a writable region of `error_buf_len` bytes (caller allocates).

B. **shim.cpp `sv_register_table_function` body (lines ~562-626)**:

1. Update the function definition signature to match the new shape with trailing `char *error_buf, size_t error_buf_len`.
2. Replace the existing early-arg null check (lines ~572-577) with the D-05 enforced shape:
   - Refuse when any of: `db_handle == nullptr`, `name == nullptr`, `bind_cb == nullptr`, `exec_cb == nullptr`, `init_cb == nullptr`. NOTE: previously `init_cb` may have been allowed; this plan tightens it (CR-02 D-05).
   - On refusal, call `snprintf(error_buf, error_buf_len, "sv_register_table_function('%s'): null required argument (init_cb is mandatory)", name ? name : "(null)");` then `return false;`.
   - Guard against `error_buf == nullptr || error_buf_len == 0` BEFORE calling snprintf — in that case skip the buffer write but still return false.
3. Replace the existing `catch (const std::exception &e)` block (lines ~615-625) so it calls `snprintf(error_buf, error_buf_len, "sv_register_table_function('%s') failed: %s", name ? name : "(null)", e.what());` instead of `fprintf(stderr, ...)`. Keep `return false;`. Per D-09, do NOT also write to stderr — `error_buf` is the only channel.
4. Add a `catch (...)` block immediately after that writes `"sv_register_table_function('<name>') failed: unknown C++ exception"` into the buffer and returns false (defensive — matches the safety posture of the surrounding code).

C. **shim.cpp `sv_register_scalar_function` body (lines ~639-689)**:

1. Update the signature the same way.
2. Update the null-arg check to refuse `db_handle == nullptr || name == nullptr || scalar_cb == nullptr` (scalar functions have NO `init_local` concept — do NOT add an init_cb check here per researcher Open Question 3). Write the error message into the buffer using the same snprintf pattern.
3. Update the catch block the same way as `sv_register_table_function`.
4. Add a `catch (...)` block.

D. **shim.cpp line 1148** — delete the comment `(void)emitted; // single-shot semantics — even without local state we emit once per bind` per D-06. The CR-02 fix (Plan 08) will re-shape the surrounding callback, but the misleading comment goes now to prevent it from leaking into Plan 08's diff noise.

DO NOT change any other shim.cpp content in this plan. CR-01 (FileSystem read) is Plan 07; CR-02 single-shot exec callback rewrites are Plan 08; WR-06 layout probe is Plan 10; WR-07/08 hard errors are Plan 11; WR-09 idempotence is Plan 12.

E. **Internal helper wrappers**: per RESEARCH.md Option 2 recommendation (lines 359-364), every existing `sv_register_*` wrapper in shim.cpp (~17 sites such as `sv_register_list_semantic_views`, `sv_register_describe_semantic_view`, ...) calls `sv_register_table_function` internally. For each wrapper:
   - Add `char *error_buf, size_t error_buf_len` to ITS signature too (mechanical edit ~17 wrappers).
   - Forward those params into the inner `sv_register_table_function` call.
   - DO NOT allocate a local error_buf inside the wrapper — pass the caller's buffer straight through.

This keeps the error-text accuracy intact: when wrapper `sv_register_list_semantic_views` calls `sv_register_table_function`, the failure message names `sv_register_table_function('list_semantic_views')` which is exactly what the user needs.

Verify the C++ side compiles via `just build` after each cluster of edits. Note: the build WILL fail at the Rust link step until Plan 02b lands — that's expected for this plan's scope. The success criterion for this plan is "C++ compiles cleanly even though Rust link fails on extern signature mismatch."

CLAUDE.md Rule 1: redirect to file then tail.
  </action>
  <verify>
    <automated>just build &gt; /tmp/claude/p02a_build.log 2>&amp;1; RC=$?; tail -80 /tmp/claude/p02a_build.log; if [ $RC -ne 0 ]; then echo "EXPECTED: build may fail at Rust link step; verify C++ compile-only step passed"; grep -E "shim.cpp.*error:|shim.hpp.*error:" /tmp/claude/p02a_build.log | head -10; if grep -E "shim.cpp.*error:|shim.hpp.*error:" /tmp/claude/p02a_build.log; then false; else true; fi; else true; fi</automated>
  </verify>
  <acceptance_criteria>
    - `cpp/src/shim.hpp` shows trailing `char *error_buf, size_t error_buf_len` on both `sv_register_table_function` and `sv_register_scalar_function` extern declarations
    - `cpp/src/shim.cpp` definition signatures match
    - `grep -n "init_cb is mandatory" cpp/src/shim.cpp` returns exactly one match (in `sv_register_table_function`)
    - `grep -n "(void)emitted" cpp/src/shim.cpp` returns zero matches
    - `grep -cE "fprintf\\(stderr" cpp/src/shim.cpp` returns the same count as before MINUS 2 (the two register-helper stderr writes are gone) — verify by recording the pre-edit count first
    - C++ compile succeeds (the Rust link step may fail on extern mismatch — that is expected and resolved by Plan 02b)
    - All ~17 internal `sv_register_*` wrappers carry `error_buf, error_buf_len` and pass them through
  </acceptance_criteria>
  <done>C ABI updated; all wrappers thread error_buf through; null-init_cb refusal active; (void)emitted comment deleted. Rust link can fail; C++ compiles cleanly.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| Rust ↔ C++ FFI | `sv_register_*` calls cross the FFI boundary; failures must surface via ABI-stable channel (error_buf), not stderr |
| Caller of `init_extension` | DuckDB host calls our extension's init hook; surfaces our error message verbatim in `LOAD semantic_views` failure |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-65.1-02a-01 | Information disclosure | `sv_register_*` stderr writes | mitigate | Replace `fprintf(stderr, ...)` with `snprintf(error_buf, ...)` — error stays in the caller's diagnostic channel, no leak to global stderr that may be redirected/logged elsewhere (D-09) |
| T-65.1-02a-02 | Denial of service | Unbounded row stream from null init_cb | mitigate | Refuse registration at boot when `init_cb == nullptr` (D-05). Failing at LOAD is the right blast radius |
| T-65.1-02a-03 | Tampering | Buffer overflow in snprintf | accept | `snprintf(buf, buf_len, ...)` truncates and NUL-terminates within buffer cap — safe by construction |
</threat_model>

<verification>
  <automated>cd cpp && (cmake --build build || just build) &gt; /tmp/claude/p02a_cpp_only.log 2>&amp;1; cd ..; grep -E "shim.cpp.*error:|shim.hpp.*error:" /tmp/claude/p02a_cpp_only.log && false || true</automated>
</verification>

<success_criteria>
- `sv_register_table_function` and `sv_register_scalar_function` carry `(char *error_buf, size_t error_buf_len)` trailing pair (declarations + definitions)
- All ~17 wrappers in shim.cpp forward the buffer pair
- Null-init_cb refusal active for table functions (CR-02 D-05); not added for scalar functions (no init_local concept)
- `(void)emitted` misleading comment deleted from shim.cpp:1148 (D-06)
- C++ side compiles cleanly (Rust link can fail; full test suite resumes after Plan 02b)
</success_criteria>

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