---
phase: 68
plan: 02
type: execute
wave: 1
depends_on: []
files_modified:
  - tests/registration_error_surfaces.rs
  - test/sql/p651_ok.yaml
autonomous: true
scope_items: [C1, C2, C3]
threat_refs: [T-68-04, T-68-05, T-68-06]
must_haves:
  truths:
    - "`tests/registration_error_surfaces.rs:136` no longer slices `body` via `&body[..body.len().min(400)]` — uses `body.get(..400).unwrap_or(body)` instead (C1, panic-free on non-UTF-8 boundary)."
    - "`tests/registration_error_surfaces.rs:164` transmute needle `[\"std::\", \"mem::\", \"transmute\"]` (no trailing `(`) catches both bare `std::mem::transmute(...)` and turbofish `std::mem::transmute::<T, U>(...)` forms (C2)."
    - "`test/sql/p651_ok.yaml` no longer exists in the repo (C3 — runtime `COPY (...) TO __TEST_DIR__/p651_ok.yaml` remains the actual contract under test for `phase651_yaml_filesystem_access_gating.test`)."
  artifacts:
    - path: "tests/registration_error_surfaces.rs"
      provides: "Panic-free body-slice error formatter (C1); turbofish-catching transmute needle (C2); one-line comment explaining the looser needle"
      contains: "body.get(..400).unwrap_or(body)"
    - path: "test/sql/p651_ok.yaml"
      provides: "(DELETED — must not exist post-plan)"
  key_links:
    - from: "tests/registration_error_surfaces.rs::test_registration_error_surfaces"
      to: "cpp/src/shim.cpp (sv_register_table_function body)"
      via: "fs::read_to_string + literal-substring assertion"
      pattern: "init_cb is mandatory"
---

<objective>
Land the three PR #35 Copilot review comments as a single coherent plan. All three are tests-only edits: two in `tests/registration_error_surfaces.rs` (C1 + C2) and one fixture deletion (C3). No production code touched.

Per the orchestrator's plan_breakdown: 68-02 runs in Wave 1 in parallel with 68-01 because there is zero file-modification overlap with `src/body_parser.rs` or the body-parser fixture.

Purpose: Pre-tag hygiene before v0.10.0 milestone close. PR #35 reviewer (Copilot) raised three inline comments; this plan clears them.

Output: One commit covering C1 + C2 (both edits to the same file), one commit for C3 (`git rm` of the unused fixture). Total: 2 commits, 1 file edited, 1 file deleted.
</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/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-CONTEXT.md
@.planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/SCOPE.md
@.planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-RESEARCH.md
@CLAUDE.md

<interfaces>
<!-- Current shape of the affected sections in tests/registration_error_surfaces.rs. Extracted verbatim. -->

From `tests/registration_error_surfaces.rs:127-136` (C1 site):

```rust
let body_close = body_close_rel.expect("matching `}` for sv_register_table_function body");
let body = &after_header[body_open..=body_close];

assert!(
    body.contains("init_cb is mandatory"),
    "Plan 02a D-05 refusal text missing — `sv_register_table_function` body must contain \
     the literal `init_cb is mandatory` so a null `init_cb` is rejected at registration time \
     and the message reaches the caller via `error_buf`. Body excerpt (first 400 chars): {}",
    &body[..body.len().min(400)],          // ← C1: byte-slice; panics on non-UTF-8 boundary.
);
```

From `tests/registration_error_surfaces.rs:160-166` (C2 site):

```rust
let transmute_needle = {
    // Constructed at runtime so we can still grep this file for the
    // literal token to confirm coverage, while the assertion below
    // looks only at non-comment lines.
    let parts = ["std::", "mem::", "transmute("];  // ← C2: trailing '(' misses turbofish form.
    parts.concat()
};
```

From `test/sql/phase651_yaml_filesystem_access_gating.test:40-70` (verifies C3 is safe):

```
# the test writes its own p651_ok.yaml at runtime via COPY (...) TO ...
statement ok
COPY (SELECT 'base_table: t1
...' AS content) TO '__TEST_DIR__/p651_ok.yaml';
```
</interfaces>
</context>

<tasks>

<task type="auto">
  <name>Task 1: C1 + C2 — UTF-8-safe body slice + turbofish-catching transmute needle (single commit)</name>
  <files>tests/registration_error_surfaces.rs</files>
  <read_first>
    - tests/registration_error_surfaces.rs (full file, ~178 lines — both edit sites are within)
    - .planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-CONTEXT.md (D-11 and D-12)
    - .planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-RESEARCH.md (Item-by-Item §C1 and §C2 — including the subtlety that the looser needle still concatenates to `std::mem::transmute`, preserving the plan-checker's substring-match invariant)
  </read_first>
  <action>
    Step A — C1 (per D-11): At `tests/registration_error_surfaces.rs:135` (the body-excerpt argument inside the `assert!(body.contains("init_cb is mandatory"), ..., &body[..body.len().min(400)]);` block), replace:

    ```rust
    &body[..body.len().min(400)],
    ```

    with:

    ```rust
    body.get(..400).unwrap_or(body),
    ```

    `body.get(..400)` returns `Some(&str)` only when 400 is a valid UTF-8 boundary AND ≤ `body.len()`. On a non-UTF-8 boundary it returns `None`, and the `.unwrap_or(body)` fallback substitutes the full body. The body is ASCII in practice (it's `cpp/src/shim.cpp` content), so the substituted slice is unchanged in the happy path. The panic-on-error formatter path is now panic-free.

    Step B — C2 (per D-12): At `tests/registration_error_surfaces.rs:164` (the `transmute_needle` parts array), change:

    ```rust
    let parts = ["std::", "mem::", "transmute("];
    ```

    to:

    ```rust
    let parts = ["std::", "mem::", "transmute"];
    ```

    (Drop the trailing `(` from the third token.) Add a one-line comment immediately above the `let parts = ...` line:

    ```rust
    // C2 (Phase 68): no trailing '(' — needle now catches both bare std::mem::transmute(...)
    // and turbofish std::mem::transmute::<T, U>(...) forms. Turbofish is the real footgun.
    ```

    Substring-match invariant preserved per RESEARCH §C2: the concatenated needle is still `"std::mem::transmute"`, which the plan-checker's `grep -q "std" + "::mem::transmute"` invariant continues to satisfy (the plan-checker greps for the literal adjacent token sequence in the existing docstring, which is unchanged).

    Step C — Verify both edits in one shot:

    ```
    cargo test --test registration_error_surfaces > /tmp/claude/c1c2.log 2>&1
    RC=$?
    tail -60 /tmp/claude/c1c2.log
    [ $RC -eq 0 ] || exit 1
    ```

    Step D — Stage and commit. Single commit message: `fix(68-02): C1+C2 — UTF-8-safe body slice + turbofish-catching transmute needle in registration_error_surfaces`. NO `--no-verify`.
  </action>
  <verify>
    <automated>cargo test --test registration_error_surfaces</automated>
  </verify>
  <acceptance_criteria>
    - `tests/registration_error_surfaces.rs` no longer contains the byte sequence `&body[..body.len().min(400)]` — verifiable via `grep -c 'body\.len()\.min(400)' tests/registration_error_surfaces.rs` returning `0`.
    - `tests/registration_error_surfaces.rs` contains `body.get(..400).unwrap_or(body)` — verifiable via `grep -c 'body\.get(\.\.400)\.unwrap_or(body)' tests/registration_error_surfaces.rs` returning `1`.
    - `tests/registration_error_surfaces.rs` no longer contains the literal token `"transmute("` inside the `parts` array — verifiable via `grep -n 'parts = \[' tests/registration_error_surfaces.rs` showing the third element as `"transmute"` (no trailing `(`).
    - A C2-explanation comment containing the substring `turbofish` appears immediately above the `parts = [...]` line — verifiable via `grep -B1 'parts = \[' tests/registration_error_surfaces.rs | grep -ci turbofish` returning ≥ 1.
    - `cargo test --test registration_error_surfaces` exits 0 (the test continues to pass — the FFI-safety invariant is upheld because the file legitimately contains no `transmute(` invocation).
    - Pre-commit hook (`cargo fmt --check` + clippy) passes without `--no-verify`.
    - Single commit landing both C1 + C2 (verifiable via `git log -1 --name-only | grep registration_error_surfaces`).
  </acceptance_criteria>
  <done>C1 and C2 land bundled in one commit. The body-slice formatter is panic-free; the transmute needle catches both bare and turbofish forms. Existing assertions still pass.</done>
</task>

<task type="auto">
  <name>Task 2: C3 — delete unused test/sql/p651_ok.yaml fixture</name>
  <files>test/sql/p651_ok.yaml</files>
  <read_first>
    - test/sql/p651_ok.yaml (15-line static fixture being deleted)
    - test/sql/phase651_yaml_filesystem_access_gating.test (verify no static-file reference; all 3 mentions point at runtime `__TEST_DIR__/p651_ok.yaml`)
    - .planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-CONTEXT.md (D-13)
    - .planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-RESEARCH.md (Item-by-Item §C3 — verification of unused status)
  </read_first>
  <action>
    Step A — Safety check before deletion. Verify no other test references the static checked-in fixture (RESEARCH already verified this; re-run as a guard):

    ```
    grep -rn "p651_ok\.yaml" test/ src/ examples/ docs/ > /tmp/claude/c3_refs.txt 2>&1
    cat /tmp/claude/c3_refs.txt
    ```

    Expected output: only references in `test/sql/phase651_yaml_filesystem_access_gating.test`, and every match should be of the form `__TEST_DIR__/p651_ok.yaml` (runtime-generated path), NOT a bare `p651_ok.yaml` load. If any other file references `p651_ok.yaml` as a static file path, HALT and report — do not delete.

    Step B — Delete the file:

    ```
    git rm test/sql/p651_ok.yaml
    ```

    Step C — Verify the gating test still passes:

    ```
    just build > /tmp/claude/c3_build.log 2>&1
    RC=$?
    tail -30 /tmp/claude/c3_build.log
    [ $RC -eq 0 ] || exit 1

    just test-sql phase651_yaml_filesystem_access_gating > /tmp/claude/c3_test.log 2>&1
    RC=$?
    tail -40 /tmp/claude/c3_test.log
    [ $RC -eq 0 ] || exit 1
    ```

    Step D — Stage and commit. Single commit message: `chore(68-02): C3 — delete unused test/sql/p651_ok.yaml (runtime COPY TO is the gating contract)`. NO `--no-verify`.

    Note: the `git rm` already stages the deletion; just `git commit` should be sufficient. No `--no-verify`.
  </action>
  <verify>
    <automated>just test-sql phase651_yaml_filesystem_access_gating</automated>
  </verify>
  <acceptance_criteria>
    - `test/sql/p651_ok.yaml` does not exist — verifiable via `test ! -f test/sql/p651_ok.yaml` exiting 0.
    - No remaining references to `p651_ok.yaml` as a static file path exist in the repo — `grep -rn "p651_ok\.yaml" test/ src/ examples/ docs/ | grep -v '__TEST_DIR__'` returns no matches (the only allowed matches are the runtime `__TEST_DIR__/p651_ok.yaml` references in `phase651_yaml_filesystem_access_gating.test`).
    - `just test-sql phase651_yaml_filesystem_access_gating` exits 0 — the gating test still exercises the runtime `COPY (...) TO __TEST_DIR__/...` path (the actual contract under test, per D-13).
    - Single commit landing the deletion (verifiable via `git log -1 --name-status | grep '^D.*p651_ok\.yaml'`).
  </acceptance_criteria>
  <done>The dead fixture is gone. The gating test continues to pass because its contract is the runtime `COPY TO` path, not the checked-in file.</done>
</task>

</tasks>

<threat_model>
## Trust Boundaries

| Boundary | Description |
|----------|-------------|
| `tests/registration_error_surfaces.rs` ← `cpp/src/shim.cpp` body | The test reads `cpp/src/shim.cpp` as a string and asserts structural invariants. The body is ASCII in practice but the assertion's error-formatting path must be panic-free on hypothetical non-UTF-8 inputs to keep CI diagnostics useful. |
| FFI registration shim invariant | The test asserts the registration shim never invokes `std::mem::transmute` on FFI function pointers — guards against a contributor reintroducing the layout-assumption pattern Phase 65 retired. |

## STRIDE Threat Register

| Threat ID | Category | Component | Disposition | Mitigation Plan |
|-----------|----------|-----------|-------------|-----------------|
| T-68-04 | Denial of Service | `tests/registration_error_surfaces.rs:135` byte-slice panic on non-UTF-8 boundary in the assertion's error-formatting path | mitigate | C1 swaps `&body[..body.len().min(400)]` to `body.get(..400).unwrap_or(body)`, which is panic-free regardless of input contents. |
| T-68-05 | Tampering | FFI safety invariant evasion via turbofish `std::mem::transmute::<T, U>(...)` slipping past the substring needle | mitigate | C2 drops the trailing `(` so the needle catches both bare and turbofish forms. |
| T-68-06 | Information disclosure | Dead `test/sql/p651_ok.yaml` fixture in repo gives a misleading impression that it's the test contract; a contributor might modify it expecting tests to consume the change | accept | C3 deletes the fixture. Severity LOW — the dead-fixture surface area is tiny and the test correctly exercises the runtime path. |
</threat_model>

<verification>
After both tasks complete:

```bash
cargo test --test registration_error_surfaces > /tmp/claude/ut.log 2>&1
RC=$?
tail -40 /tmp/claude/ut.log
[ $RC -eq 0 ] || exit 1

just test-sql phase651_yaml_filesystem_access_gating > /tmp/claude/sqllog.log 2>&1
RC=$?
tail -40 /tmp/claude/sqllog.log
[ $RC -eq 0 ] || exit 1

# Confirm no static p651_ok.yaml lingers
test ! -f test/sql/p651_ok.yaml || exit 1
```

All three checks MUST pass.
</verification>

<success_criteria>
- 3 SCOPE.md items (C1, C2, C3) closed.
- C1: `body.get(..400).unwrap_or(body)` replaces the byte-slice; no byte-boundary panic possible.
- C2: transmute needle drops the trailing `(` and a comment explains the turbofish coverage; substring-match invariant preserved.
- C3: `test/sql/p651_ok.yaml` deleted; gating test still passes.
- 2 commits total (one for C1+C2, one for C3).
- No `--no-verify` used.
</success_criteria>

<output>
Create `.planning/phases/68-pre-tag-cleanup-phase-67-review-pr-35-code-review-follow-ups/68-02-SUMMARY.md` when done.
</output>