Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,24 @@ 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** |

**NEW — inert process port (locked, safety requirement).** Today `HostRuntimeServices::new()` hardcodes `process_port: Arc::new(LocalHostProcessPort::new())` (`crates/ironclaw_host_runtime/src/services.rs:329`), which runs **real OS processes** via `tokio::process::Command`. A scripted model `tool_call("builtin.shell", …)` would execute on the developer's machine — the exact incident class that motivated hermes-agent's live-system guard. The harness default must be a `RecordingProcessPort` (impl `RuntimeProcessPort`, in `tests/support/reborn/process.rs`) that returns scripted/empty output and records the attempted command; **no real process ever runs by default**. Real shell is explicit per-test opt-in via `.with_live_shell()`. This is required by the zero-setup local-run guarantee (§4.3): a default test can never mutate the dev machine.

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<dyn RuntimeProcessPort>` — 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function build_native_backend is located in crates/ironclaw_memory_native/src/backend.rs (at line 415), not in service.rs. Update the file reference to prevent confusion for developers tracing this path.

Suggested change
- `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.
- build_native_backend (backend.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.
References
  1. Ensure documentation precisely reflects the current implementation of traits and methods; remove or explicitly mark as 'planned' any references to features that were renamed or not implemented during development.

- 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.
Expand All @@ -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.
Expand Down
9 changes: 9 additions & 0 deletions tests/support/reborn/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)".
Loading