diff --git a/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md b/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md index bf729fbf6e3..e9458bd6f74 100644 --- a/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md +++ b/docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md @@ -126,7 +126,7 @@ The LLM is one egress; a real turn crosses several. The contract is **every netw | Shell / process | `RuntimeProcessPort` | **inert `RecordingProcessPort`** (scripted output, no real process) — **NEW, see below** | real shell only via explicit `.with_live_shell()` | | Secrets / OAuth | `SecretStore` | `StaticSecretStore` (fixture handles); refresh worker not spawned | `.with_secret(handle, value)` to seed | | Approval gates | approval store | in-memory auto-approve | `.deny_capability` / explicit gate resolution | -| Embeddings | `EmbeddingProvider` | none wired; `InMemoryBackend` linear-scan | **caveat:** semantic-ordering assertions are unreliable — assert membership, not vector rank | +| Embeddings | `EmbeddingProvider` | none wired, **and unreachable from the harness path — no fake built (Slice 9 verdict, see below)** | memory_search is pure FTS; **assert membership, not vector rank** | | Trace Commons (telemetry) | `ContributionHttpSink` (`ironclaw_reborn_traces`) | **already captured** — the agent path (`HostEgressContributionSink`) routes through `RuntimeHttpEgress`, so `RecordingRuntimeHttpEgress` records it; no new sink needed | assert by filtering `recorded_egress.requests()` by the contribution URL | | MCP servers | MCP client | not wired by default | opt-in: `.with_mock_mcp(server)` wiring the existing `MockMcpServer` — **P1 ergonomics** | @@ -134,6 +134,16 @@ The LLM is one egress; a real turn crosses several. The contract is **every netw Injection seam: `RebornBuildInput` → `build_local_runtime` does not expose a process-port override today (`apply_runtime_process_binding` handles only `TenantSandbox`/`None`). Wiring the recording port needs a small `#[cfg(any(test, feature = "test-support"))]` accessor in `ironclaw_reborn_composition::factory` accepting an injectable `Arc` — the same visibility-promotion pattern Step 4 uses for `mount_local_dev_database_roots`. Scoped into Step 5b. +**Embeddings — no fake (Slice 9 verdict, seam unreachable).** Investigation for the planned embeddings fake found **no seam to intercept in the Reborn harness path**, so the fake is descoped. Evidence (base `6a3b10fa5`): + +- The Reborn first-party memory tools (`builtin.memory_search`/`memory_write`/`memory_read`/`memory_tree`) dispatch through `NativeMemoryService` only. `MemoryCapabilityState::service_for` (`crates/ironclaw_host_runtime/src/first_party_tools/memory.rs:161`) *always* builds `NativeMemoryService::from_filesystem(filesystem, …)`; the sole override (`memory_service_for_test`) is `#[cfg(test)]`, internal to that crate's own unit tests and unreachable from the cross-crate `tests/support/reborn/` harness. +- `NativeMemoryService::search` (`crates/ironclaw_memory_native/src/service.rs:104`) hardcodes `.with_vector(false)` on every request — there is no parameter or opt-in — so vector/embedding search is unconditionally off and `EmbeddingProvider::embed` is never called on a search. +- `build_native_backend` (`service.rs:415`) wires `ChunkingMemoryDocumentIndexer::new(repository)` with `embedding_provider: None` and never calls `.with_embedding_provider`; on write, `build_chunk_writes(…, None)` persists text-only chunks and the backend even swallows indexer errors — so `embed` is never called on a write either. +- The harness wires no `EmbeddingProvider`/`MemoryService` at all (grep of `tests/support/reborn/` is empty), and memory_search returns correct **membership** results via FTS with zero embeddings wiring. +- Trait mismatch compounds it: the Reborn path uses `ironclaw_memory_native::EmbeddingProvider`, a *different* trait from the `ironclaw_embeddings::EmbeddingProvider` the build order referenced. The latter is wired only into the v1 `Workspace` (`src/workspace/mod.rs`, `src/app.rs::init_tools`), which the Reborn harness does not exercise. + +Conclusion: a fake `EmbeddingProvider` would have nothing to intercept. Membership assertions over memory_search are covered for free by FTS through the real `NativeMemoryService` path (the future memory-coverage slice should assert membership there, not vector rank). Re-scope a fake only if/when the Reborn memory path gains a vector-search path that consults `EmbeddingProvider` (e.g. `NativeMemoryService` stops forcing `with_vector(false)` *and* a cross-crate provider-injection accessor is added). + **Trace Commons — no new type.** `ironclaw_reborn_traces::ContributionHttpSink` looks like a separate egress, but the agent-invoked path (`HostEgressContributionSink`, `trace_commons.rs`) delegates to `RuntimeHttpEgress` — so it is *already* captured by the default `RecordingRuntimeHttpEgress`. A test asserts contribution behavior by filtering `recorded_egress.requests()` for the contribution URL. (The CLI/background-worker path uses a direct client, but that worker is not spawned in the harness — §3.6 "Secrets/OAuth" note.) No `RecordingContributionSink` is built. **P1 ergonomics (not blockers):** a URL-keyed scripting layer over `RecordingRuntimeHttpEgress` for multi-step tool-HTTP flows, and a `.with_mock_mcp(...)` constructor wiring the existing `MockMcpServer` into the Reborn `ExtensionRegistry`. Both are additive; the default capture matrix above is the required floor. @@ -142,7 +152,7 @@ Injection seam: `RebornBuildInput` → `build_local_runtime` does not expose a p There is **no single outbound chokepoint** — production has four distinct seam families, so the harness uses a **two-tier** interception model (matching the industry split: trait-level fakes for orchestration logic, request-matcher mocks for HTTP adapters): -- **Tier 1 — trait-level fakes** (for logic that runs *above* the call): `LlmProvider` (scripted `TraceLlm`, §3.1), `EmbeddingProvider` (fake), `RuntimeProcessPort` (inert `RecordingProcessPort`), and **channel delivery** (`OutboundDeliverySink` — captured by `RecordingOutboundDeliverySink` at its own trait boundary, *not* through `RuntimeHttpEgress`; see §3.6). LLM/embeddings hold their own `reqwest` clients and are deliberately **not** mocked at HTTP — mocking at the provider trait runs the real chain/loop (the "FakeChatModel" lesson: HTTP-level mocks silently pass when the SDK/loop layer above them changes). +- **Tier 1 — trait-level fakes** (for logic that runs *above* the call): `LlmProvider` (scripted `TraceLlm`, §3.1), ~~`EmbeddingProvider` (fake)~~ **— descoped: the embeddings seam is unreachable in the Reborn memory path, see §3.6 "Embeddings — no fake"**, `RuntimeProcessPort` (inert `RecordingProcessPort`), and **channel delivery** (`OutboundDeliverySink` — captured by `RecordingOutboundDeliverySink` at its own trait boundary, *not* through `RuntimeHttpEgress`; see §3.6). LLM/embeddings hold their own `reqwest` clients and are deliberately **not** mocked at HTTP — mocking at the provider trait runs the real chain/loop (the "FakeChatModel" lesson: HTTP-level mocks silently pass when the SDK/loop layer above them changes). - **Tier 2 — recording interceptor over the HTTP-egress family**: MCP, OAuth, OAuth-refresh, first-party HTTP tools (and Trace Commons) route through the single `RuntimeHttpEgress::execute(RuntimeHttpEgressRequest{ runtime, capability_id, url, method, … })` trait; WASM/network-policy tool calls go through the sibling `NetworkHttpEgress`. The harness wires both recorders (`RecordingRuntimeHttpEgress` + `RecordingNetworkHttpEgress`) — see §3.6 for the authoritative per-boundary list. Today they record a scripted FIFO body; the P1 ergonomics extension (§3.6) is a URL/`capability_id`-keyed matcher over the same recorders. **Rejected: a single HTTP interceptor for *everything*.** Routing LLM/embeddings/process through one HTTP layer would rip provider auth/retry/circuit out of `ironclaw_llm`, force non-HTTP boundaries (process, future CLI) into an HTTP shape, lose type safety (match on serialized bodies), and reintroduce the per-provider HTTP fixtures §3.1 rejects. It contradicts the locked SDK-seam requirement. diff --git a/tests/support/reborn/CLAUDE.md b/tests/support/reborn/CLAUDE.md index 92b8c4305ec..c6f62b6fa8c 100644 --- a/tests/support/reborn/CLAUDE.md +++ b/tests/support/reborn/CLAUDE.md @@ -88,3 +88,12 @@ Slice 1 ships the spine + one text-reply test. Slice 2 (this PR) ships matrix; inert process port + `.with_live_shell()` / `.with_live_http_egress()` opt-ins; outbound/HTTP/secrets/MCP capture wiring; a dedicated `assertions.rs` once the `assert_*` family grows; the pre-commit test-style check. + +**Descoped (not planned): the embeddings fake.** Slice 9 verified there is no +embeddings seam to intercept in the Reborn memory path — `NativeMemoryService` +(the only memory service the first-party memory tools dispatch through) hardcodes +`.with_vector(false)` on every search and wires no `EmbeddingProvider` on write, +so `EmbeddingProvider::embed` is never called. memory_search returns correct +**membership** via FTS with zero embeddings wiring; a future memory-coverage +test should assert membership there, not build a fake. Full evidence: design spec +§3.6 "Embeddings — no fake (Slice 9 verdict)".