docs(reborn-itest): Slice 9 — descope embeddings fake (seam unreachable) - #5386
Closed
henrypark133 wants to merge 1 commit into
Closed
henrypark133 wants to merge 1 commit into
henrypark133 wants to merge 1 commit into
Conversation
Slice 9 set out to add a Tier-1 fake `EmbeddingProvider` for the Reborn integration-test harness. Investigation at base 6a3b10f found there is no embeddings seam to intercept in the Reborn memory path, so the fake is descoped. This records the verdict in the design spec (§3.6/§3.7) and the reborn test-support CLAUDE.md so the analysis isn't repeated. Evidence: - Reborn first-party memory tools dispatch only through NativeMemoryService (host_runtime first_party_tools/memory.rs:161); the test override is #[cfg(test)] and unreachable from the cross-crate tests/support/reborn/. - NativeMemoryService::search hardcodes .with_vector(false) (memory_native/src/service.rs:104) — embeddings never consulted on search. - build_native_backend wires embedding_provider: None and never embeds on write (build_chunk_writes persists text-only chunks). - The harness wires no EmbeddingProvider/MemoryService at all; memory_search returns correct membership via FTS with zero embeddings wiring. - Trait mismatch: the Reborn path uses ironclaw_memory_native::EmbeddingProvider, not ironclaw_embeddings::EmbeddingProvider (the latter is v1 Workspace only). Docs-only; no production or test code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5386
June 28, 2026 02:58
Destroyed
github-actions
Bot
added
size: XS
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughDesign spec and test-framework ChangesEmbeddings descope and process port hardening
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the integration test framework design specification and the CLAUDE.md documentation to descope the planned fake EmbeddingProvider, as the embeddings seam is unreachable from the Reborn harness path. The review feedback correctly identifies an incorrect file reference in the documentation, pointing out that build_native_backend is located in backend.rs rather than service.rs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| - 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. |
Contributor
There was a problem hiding this comment.
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
- 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.
|
🚅 Deployed to the ironclaw-pr-5386 environment in ironclaw-ci-preview
|
Collaborator
Author
|
Superseded by #5392 — this work is folded into the combined Reborn integration-test framework PR (slices 3–9). |
henrypark133
added a commit
that referenced
this pull request
Jun 29, 2026
…gress/HTTP matcher, inert process port, MCP/OAuth/refresh) (#5392) * wip: slice4 http matcher Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): slice4 URL-keyed HTTP matcher + egress assertion API Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn): note slice4 keyed HTTP matcher + egress assertion API Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * slice3: production visibility promotion + matrix test draft + plan * slice3: promote build_default_local_dev_database_roots pub(crate) + add test accessor * slice3: RebornThreadHarness<F=LocalFilesystem> generic + filesystem_shared_composite + prefix-param scoped_threads_fs_at * slice3: extract turns_scope_path to filesystem.rs, shrink harness.rs scoped_turns_fs * slice3: StorageMode + one-composite build + scoped_turns_fs_composite + assert_reply_persists_after_reopen * slice3: add rstest = "0.23" dev-dep + libsql feature on reborn_composition dev-dep * slice3: update CLAUDE.md + design spec (§3.2/§9 step 4 done, Option C) Mark step 4 done in §9 build order; record Option-C decision (one CompositeRootFilesystem for both InMemory and LibSql, same path layout) and the visibility promotion details in §3.2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: fix private-type-in-pub(crate)-return compile error Add `mount_default_local_dev_database_roots` (void wrapper) so test_support.rs never has to name the module-private `LocalDevDurableBackend` type. Callers of the void wrapper get the same 4-step libSQL setup without the type leaking across the module boundary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: add missing RootFilesystem where-bound on RebornThreadHarness struct FilesystemSessionThreadService<F> requires F: RootFilesystem; the struct definition needs the same bound so the field type-checks without relying solely on the impl blocks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: cargo fmt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: gate mount_default_local_dev_database_roots on test-support feature The function is only called from test_support.rs which is itself gated on feature = "test-support"; without the gate, cargo -p ironclaw_reborn_composition warns dead_code on every non-test build. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 1 — RecordingProcessPort inert process port Add `tests/support/reborn/process.rs`: `RecordingProcessPort` implements `RuntimeProcessPort` but never spawns an OS process — records each command string and returns exit 0 / empty output. Registered as `pub mod process` in the support tree's `mod.rs`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 2 — inject RecordingProcessPort + add SHELL_CAPABILITY_ID - `local_dev_host_runtime_with_registry_and_runtime_http_egress` / `with_http_egress` accept `Option<Arc<dyn RuntimeProcessPort>>` and call `.with_runtime_process_port_dyn` when `Some` (None = default LocalHostProcessPort). - `core_builtin_tools_with_network_policy` creates `RecordingProcessPort`, injects it, and stores it on `harness.process_port` (mirrors http_egress threading pattern). - `core_builtin_tools_with_live_shell()` new constructor: skips injection so the real `LocalHostProcessPort` runs (used by step 3's `.with_live_shell()`). - `SHELL_CAPABILITY_ID` added to `core_builtin_tools_from_runtime` capability surface; `SpawnProcess` added to effect_kinds so the capability port allows it. - `process_port` field + `process_commands()` accessor on `HostRuntimeCapabilityHarness`. - `recorded_process_commands()` on `HarnessCapabilityRecorder` (mirrors runtime_http_requests). - No production file touched; all changes are within tests/support/reborn/harness.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 3 — .with_live_shell() opt-in on builder Add `live_shell: bool` to `RebornIntegrationHarnessBuilder` (default false). `.with_live_shell()` sets the flag and implies `BuiltinHttpTools` capability backend. In `build()`, the `BuiltinHttpTools` arm dispatches to `core_builtin_tools_with_live_shell()` when the flag is set — routing the HostRuntime backend to use the real `LocalHostProcessPort` instead of the inert `RecordingProcessPort`. No production file touched. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 4 — assert_shell_command_recorded + assert_no_real_process_executed Add two assertion methods on `RebornIntegrationHarness`: - `assert_shell_command_recorded(substr)`: passes when a recorded command contains substr. - `assert_no_real_process_executed()`: passes when the inert port captured ≥1 command (the recording path ran, not the live-shell opt-in). Both read from `capability_recorder.recorded_process_commands()` (the new slice-5 accessor on `HarnessCapabilityRecorder`). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 5 — reborn_integration_process_port.rs test Proves builtin.shell dispatches through the inert RecordingProcessPort by default: shell_call_recorded_not_executed asserts the command was recorded and no real OS process spawned; shell_assertions_fail_when_no_shell_call_ran guards against vacuous pass on empty command lists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): slice 5 step 6 — update CLAUDE.md + design spec §3.6/§9/§10 CLAUDE.md: document process.rs + slice-5 assertions + .with_live_shell(); remove inert process port from Planned. Design spec: mark §3.6 shell row Built (slice 5), correct §3.6 injection-seam paragraph (no prod change needed; with_runtime_process_port_dyn was already pub), mark §9 step 5b DONE, add §10 resolved decision. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): slice 5 — add ExecuteCode to core_builtin_tools effect_kinds builtin.shell declares EffectKind::ExecuteCode in its manifest; the GrantAuthorizer checks effects_are_covered(descriptor.effects, grant.allowed_effects) and denied the capability because ExecuteCode was absent from the harness effect_kinds grant. This caused the shell CapabilityInvocation to be denied (outside_visible_surface) before reaching the process port, so no command was ever recorded. Fix: add EffectKind::ExecuteCode to core_builtin_tools_from_runtime's effect_kinds vec so the shell capability appears in the visible surface and dispatches through the RecordingProcessPort as intended. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): cargo fmt after slice 5 fix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 6 — MCP mock end-to-end Adds the MCP-mock tier of the Reborn integration-test framework. - `LoopbackMcpRuntimeHttpEgress`: test-only `RuntimeHttpEgress` that makes real reqwest HTTP to the loopback mock server, injects a Bearer token, and rejects URLs outside the configured mock endpoint (hermetic guard). - `LoopbackMcpRuntime` type alias + `local_dev_host_runtime_with_registry_egress_and_mcp` helper: wires the custom egress into a real `McpRuntime<McpHostHttpClient<…>>`. - `mock_mcp_extension_package`: builds a minimal `ExtensionPackage` with `ExtensionRuntime::Mcp` for the fixed `mock-mcp` provider. - `HostRuntimeCapabilityHarness::mock_mcp_tools`: async constructor that assembles all of the above into a `HostRuntimeCapabilityHarness`. - `RebornCapabilityBackend::MockMcp` variant + `with_mock_mcp(url)` builder method on `RebornIntegrationHarness`. - `assert_mcp_tool_called(tool_name)` on `RebornIntegrationHarness`: maps `"search"` → capability id `"mock-mcp.search"` and delegates to `assert_tool_invoked`. - `tests/reborn_integration_mcp.rs`: two tests — `mcp_tool_call_reaches_mock_server` (core scenario) and `assert_mcp_tool_called_fails_when_no_mcp_call_ran` (guard). - `ironclaw_mcp` added to `[dev-dependencies]` in workspace `Cargo.toml`. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(reborn-itest): use inline MCP descriptor schema to avoid $ref filesystem read mock_mcp_extension_package used from_manifest which sets parameters_schema to {"$ref": "schemas/mock-mcp/mock.input.v1.json"}. surface_descriptor in CapabilitySurface::visible_capabilities then tries to read that schema file from the host filesystem, which fails (the file doesn't exist for a test-only mock extension). This produced host_creation_failed at turn dispatch. Switch to from_host_bundled_manifest_with_inline_dynamic_schemas with an inline {"type":"object"} parameters_schema. surface_descriptor sees no $ref and returns Ok(descriptor) early, unblocking visible_capabilities → build_text_only_host_with_profiled_capabilities → create_host. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt on reborn itest slice 6 files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): document slice 6 MCP mock in harness CLAUDE.md Records the LoopbackMcpRuntimeHttpEgress, mock_mcp_extension_package (inline-dynamic-schemas fix), local_dev_host_runtime_with_registry_egress_and_mcp, .with_mock_mcp(), assert_mcp_tool_called(), and reborn_integration_mcp.rs in the authoritative authoring guide. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): mark slice 6 MCP mock done in design spec Updates §3.6 table, P1-ergonomics paragraph, built-slice-4 section, §9 step-5b trailing note, and new §9 step-5c to record that .with_mock_mcp() and assert_mcp_tool_called() shipped in slice 6 via LoopbackMcpRuntimeHttpEgress + from_host_bundled_manifest_with_inline_dynamic_schemas. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(itest-slice7): add OAuth test-support types and factory to test_support.rs Adds ScriptedOAuthTokenEgress, OAuthProductAuthTestBundle, and build_oauth_product_auth_for_test() to ironclaw_reborn_composition's test-support module (feature = "test-support"). Wires a real FilesystemAuthProductServices<InMemoryBackend> over a fixed-view ScopedFilesystem with a noop obligation handler and noop continuation dispatcher so OAuth connect-flow integration tests can drive the full claim→exchange→complete path with no network and no feature-gated deps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(itest-slice7): add reborn_integration_oauth_connect test (slice 7) Two tests exercise the OAuth connect-flow seam: - oauth_connect_flow_persists_credential_account: drives create_flow → handle_oauth_callback → get_account; asserts account persisted and exactly one scripted token-exchange HTTP call captured. - oauth_callback_without_prior_flow_fails: guard test; missing flow produces UnknownOrExpiredFlow and zero egress calls. No production files touched. No network, no services, no integration feature required. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt slice 7 test_support + oauth_connect test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(itest-slice7): update CLAUDE.md + design spec for OAuth connect-flow slice - CLAUDE.md: add Slice 7 entry describing ScriptedOAuthTokenEgress, OAuthProductAuthTestBundle, build_oauth_product_auth_for_test(), and the two new tests; remove "product/auth" from the Planned list. - Design spec §3.6: update Secrets/OAuth row to note ScriptedOAuthTokenEgress + build_oauth_product_auth_for_test() as the opt-in for full OAuth flows. - Design spec §3.8: add Slice 7 wiring exception (standalone bundle via FilesystemAuthProductServices<InMemoryBackend> + fixed ScopedFilesystem). - Design spec §9: mark step 7 DONE with Slice 7 OAuth connect-flow detail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): pub(crate) LocalDevDurableBackend to silence private_interfaces The slice-3 promotion of build_default_local_dev_database_roots to pub(crate) exposed the private LocalDevDurableBackend enum in a pub(crate) signature, tripping private_interfaces (which the CI -D warnings lane fails on). Bump the enum to pub(crate) to match; it stays crate-internal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(itest-slice8): clock injection — make sweep_once pub(crate) with now: DateTime<Utc> Production path unchanged: tick_once passes Utc::now(). Tests can pass a frozen instant to make just-created accounts appear idle, enabling deterministic keepalive-refresh assertions without sleep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(itest-slice8): add OAuth refresh test-support fixtures - ScriptedOAuthTokenEgress::with_access_and_refresh_token(): stores a refresh_token in the scripted response so the exchange phase writes a refresh secret handle. - FixedCandidateSource: crate-private struct impl CredentialRefreshCandidateSource; injects a pre-seeded account list into sweep_once without the filesystem tenant-path walk. - OAuthProductAuthTestBundle::sweep_for_refresh(): drives one sweep tick with a fixed account list and a frozen clock, wiring the always-leader lock and the real ProviderBackedCredentialAccountService refresh path. - build_google_oauth_product_auth_for_test(): same as build_oauth_product_auth_for_test but with provider_id="google", refresh_token in the egress response, and .with_provider_client() so refresh_account does not short-circuit to BackendUnavailable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(itest-slice8): gate slice-8 test support on libsql|postgres; add [[test]] entry FixedCandidateSource, sweep_for_refresh, and build_google_oauth_product_auth_for_test all depend on credential_refresh_worker which is gated on any(feature = "libsql", feature = "postgres"). Gate the new items the same way so builds without durable-backend features still compile. Add [[test]] name = "reborn_integration_oauth_refresh" with required-features = ["libsql"] to the root Cargo.toml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(itest-slice8): add reborn_integration_oauth_refresh test Two tests: - credential_refresh_sweep_refreshes_idle_google_account: positive test using frozen clock (Utc::now() + 3 days) so just-created account appears past the 2-day idle threshold; asserts egress.captured_count() == 2 (initial exchange + refresh call). - credential_refresh_sweep_skips_fresh_google_account: guard test using Utc::now() so just-created account is within the idle threshold; asserts egress.captured_count() stays at 1 (no refresh). Both tests drive the full sweep_once → ProviderBackedCredentialAccountService → HostOAuthProviderClient → ScriptedOAuthTokenEgress path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: cargo fmt reborn_integration_oauth_refresh test * docs(itest-slice8): record slice 8 (OAuth refresh + clock injection) in design spec §9 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(itest): [B1] move cfg attribute below consolidated doc block in build_google_oauth_product_auth_for_test The #[cfg(any(feature = "libsql", feature = "postgres"))] attribute was wedged inside the doc-comment (between two bullet groups), making the second group orphaned. Consolidate all /// lines into one block above the attribute, and reword the gate rationale: gated because sweep_for_refresh (the primary consumer) requires credential_refresh_worker, which is compiled only under those features. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(itest): [B2] extract build_oauth_product_auth_infra() shared preamble build_oauth_product_auth_for_test and build_google_oauth_product_auth_for_test shared a verbatim ~8-line preamble (MountView/MountGrant setup, InMemoryBackend, ScopedFilesystem::with_fixed_view, InMemorySecretStore, FilesystemAuthProductServices). Extract it into a private build_oauth_product_auth_infra() helper returning the three types both callers need. Remove the duplicated block and the "Same fixed-view mount layout as…" comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(itest): [Nit] add #[cfg(feature = \"test-support\")] to mount_local_dev_database_roots_for_test The sibling build_default_local_dev_database_roots_for_test carries the explicit attribute; this function's own doc-comment claims it is gated behind test-support but the attribute was missing. Add it for consistency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(itest): [S1] rename assert_no_real_process_executed to assert_shell_ran_through_inert_port The name implied a negative check but the body passes when ≥1 command was recorded through the inert port. Rename to the accurate positive form and update the doc to lead with the positive condition. Update both call sites in reborn_integration_process_port.rs. Behavior is identical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(itest): [MockMcp const] collapse MockMcp fields to mcp_url, extract MOCK_MCP_PROVIDER_ID The MockMcp variant carried provider_id/capability_id fields that were always the constants "mock-mcp"/"mock-mcp.search" (set only in with_mock_mcp, no other setter), and assert_mcp_tool_called independently rebuilt format!("mock-mcp.{…}"). Collapse to MockMcp { mcp_url: String }, declare const MOCK_MCP_PROVIDER_ID near the builder, and use it in both the match-arm wiring and assert_mcp_tool_called. One owner for the string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(itest): [S2] add arch-exempt annotation for large MCP block in harness.rs + CLAUDE.md note harness.rs is 4179 lines (>3000, architecture.md §5). Add arch-exempt: large_file annotation on the first line of the MCP wiring block (LoopbackMcpRuntime type alias) noting the harness_mcp.rs split as a tracked follow-up. Add a one-line note in tests/support/reborn/CLAUDE.md referencing the planned sub-module split. Do not split the file now. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt after code-review fixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): clear CI clippy — type_complexity + derivable_impls - B2 helper return type tripped clippy::type_complexity (3-tuple of nested Arcs) → return a named OAuthProductAuthInfra struct (drop the unused scoped_fs handle; durable holds its own Arc clone). - StorageMode manual Default impl tripped clippy::derivable_impls → derive Default with #[default] on InMemory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): slice 3 negative guard — reopen assertion fails on mismatch Closes the only guard-test gap the verification pass found: every other slice proves its assertion can fail (non-vacuous); slice 3's LibSql reopen read-back now has a matching guard asserting assert_reply_persists_after_reopen returns Err when the expected text is absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn-itest): fold slice 9 embeddings-descope verdict into combined spec Folds the slice-9 finding (PR #5386) into the combined framework branch: the embeddings fake is descoped because the Reborn memory path never consults EmbeddingProvider (NativeMemoryService forces with_vector(false); backend wires embedding_provider: None) and uses ironclaw_memory_native::EmbeddingProvider, not the spec-named ironclaw_embeddings one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn-itest): genuine disk-durability in assert_reply_persists_after_reopen `assert_reply_persists_after_reopen` was re-instantiating the thread service over the **same** in-process `CompositeRootFilesystem` Arc for both InMemory and LibSql modes, so `libsql_persists_reply_across_reopen` passed even when `StorageMode::LibSql` secretly used `InMemoryBackend` (the mutation stayed GREEN — the coverage gap). Fix: when `StorageMode::LibSql`, open a **genuinely fresh** `libsql::Builder::new_local(db_path)` connection independent of the live composite, run migrations (idempotent), mount via `mount_local_dev_database_roots_for_test`, and read thread history through the fresh handle. Only data serialized+committed to the `.db` file is visible through the new connection; an InMemory mutation leaves the file absent/empty, so `list_thread_history` returns nothing and `assert_final_reply` returns `Err(MissingFinalReply)` — mutation goes RED. For InMemory the existing same-handle `reopened()` path is kept (no disk involved; tests service re-instantiation, not durability). Also captures `libsql_db_path: Option<PathBuf>` on the harness via the updated `build_storage_composite` return type so the reopen path can locate the file without rediscovering it. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test(reborn-itest): prove MCP egress reaches mock server (M4 coverage gap) `assert_mcp_tool_called` checked `capability_recorder.invocations()` which fires before HTTP egress, so a wrong URL or dead server still passed. Add two assertions after `assert_mcp_tool_called` in the POSITIVE test only: 1. `assert!(!server.recorded_requests().is_empty(), …)` — verifies that the loopback mock MCP server received at least one HTTP POST. 2. `assert!(recorded.iter().any(|r| r.method == "tools/call"), …)` — verifies that at least one recorded request carries the JSON-RPC method `"tools/call"` (the field `RecordedMcpRequest.method` holds the JSON-RPC method string, captured in `handle_mcp` before dispatch). Together these prove the MCP runtime made a real HTTP round-trip to the loopback server, not just that the capability recorder fired pre-egress. URL-corruption mutations or dead-server mutations now go RED. The negative guard `assert_mcp_tool_called_fails_when_no_mcp_call_ran` is unchanged — it scripts no MCP turn so `recorded_requests` stays empty, and the new assertions are not present in that test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): make MCP tool call genuinely reach the loopback mock server The stronger slice-6 assertion (commit 02157b3) exposed that the scripted MCP tool call never reached the loopback mock server — three real blockers, all upstream of HTTP egress: 1. Trust policy. `mock_mcp_tools` wired `first_party_trust_policy()`, which only trusts the builtin first-party provider. The mock MCP provider's manifest (`/system/extensions/mock-mcp/manifest.toml`) had no trust entry, so `evaluate_invocation_trust` produced a Sandbox ceiling and dispatch was denied. Added `first_party_and_mcp_trust_policy(provider_id)` granting the mock provider `user_trusted` for `DispatchCapability` + `Network`, keyed on the same `LocalManifest` path the host runtime derives at dispatch time. 2. Network policy. `mock_mcp_tools` set `NetworkPolicy::default()` (empty `allowed_targets`). The MCP capability declares `EffectKind::Network`, so authorization attaches an `ApplyNetworkPolicy` obligation that the host runtime's `validate_network_policy_metadata` rejects when `allowed_targets` is empty — blocking the egress before any HTTP. Added `mcp_loopback_network_policy()` permitting host `127.0.0.1` (scheme http) with `deny_private_ip_ranges = false` (127.0.0.1 is loopback/private). 3. Notification status. The mock answered JSON-RPC notifications (`notifications/initialized`, id=None) with `200 OK` + empty body. Per the MCP Streamable HTTP spec a notification-only body MUST get `202 Accepted`; the real client (`send_planned_json_rpc`) only treats 202 as a valid empty-body ack and otherwise parses the empty 200 as a JSON-RPC response, which fails and aborts `initialize_session` before `tools/call` is sent. Mock now returns `202 Accepted` for notifications. Also hardens `LoopbackMcpRuntimeHttpEgress::new` (CodeRabbit #5): validate that `mcp_url`'s host is loopback (127.0.0.1 / ::1 / localhost) and error otherwise, so a typo cannot silently turn the test egress into real external network I/O. With all three fixed, `mcp_tool_call_reaches_mock_server` passes WITH the stronger assertion: the mock records `initialize`, `notifications/initialized`, and `tools/call`. The negative guard is unaffected (no MCP turn scripted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): prove OAuth refresh sweep commits the rotated credential The slice-8 refresh test asserted only egress.captured_count() == 2 — i.e. that the refresh HTTP call fired. That would still pass if the refresh made the call but silently dropped the account write-back. Strengthen the positive test to re-read the account through the durable CredentialAccountService and assert the persisted access-token handle was rewritten to the refresh-path handle (`…-oauth-refresh-access-<account_id>`, produced only by HostOAuthProviderClient::store_refreshed_tokens) and differs from the connect-exchange handle. A dropped account write would leave the connect handle in place, so both assertions fail in that case. No production change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn-itest): narrow loopback guard to 127.0.0.1 and disable redirects in LoopbackMcpRuntimeHttpEgress PR review comments 2 and 3 on #5392: - Comment 2: narrow `LoopbackMcpRuntimeHttpEgress::new` host check from accepting 127.0.0.1/::1/localhost to 127.0.0.1 only, matching `mcp_loopback_network_policy()` which also only allows 127.0.0.1. A "localhost" URL would previously pass the egress guard then fail network authorization — a latent trap. - Comment 3: add `.redirect(reqwest::redirect::Policy::none())` to the reqwest Client builder so a mock 3xx cannot redirect the client off loopback; the `starts_with(mcp_url)` hermetic guard only checked the first request URL, not redirect hops. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(reborn-mcp): capture params in RecordedMcpRequest and assert tool name in tools/call Addresses CodeRabbit PR #5392 comment: the MCP test could only assert that *some* tools/call arrived, not which tool was called. - `RecordedMcpRequest` gains `pub params: Option<serde_json::Value>`; the handler now sets it from `req.params.clone()` on every request. - `mcp_tool_call_reaches_mock_server` replaces the bare `any(tools/call)` assertion with a find + `assert_eq!` on `params["name"] == "search"`, proving the right tool was dispatched with the right wire shape. - Additive change: no other `RecordedMcpRequest` constructor exists; all other consumers only read `recorded_requests()` and compile clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): fix stale helper name in CLAUDE.md (assert_shell_ran_through_inert_port) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(reborn-itest): extract LOCAL_DEV_DB_FILENAME const; harness uses canonical name One string owns the local-dev SQLite filename. The integration-test harness no longer duplicates "reborn-local-dev.db" — it reads the constant through the public crate API so any future rename is a single-site change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(reborn-itest): add capability-keyed HTTP response matching test Scripts two responses for the same URL with different with_capability() keys. The first entry has a wrong key, so the builtin.http call falls through (capability mismatch) to the second entry, which matches. Proves that the first-match-wins matcher skips entries whose capability key doesn't match and falls through to subsequent entries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt reorder LOCAL_DEV_DB_FILENAME re-export in lib.rs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(no-panics): exempt feature-gated test_support.rs modules scripts/check_no_panics.py flagged .unwrap()/.expect() on constant literals in crates/ironclaw_reborn_composition/src/test_support.rs. That module is `#[cfg(feature = "test-support")] pub mod test_support;` — the test-support feature is enabled only via [dev-dependencies], so it ships zero bytes in production binaries (same 'never compiled in production' rationale the check already uses to exempt tests/ and tests.rs). Repo-wide convention across 5 crates. Exempt by exact filename (my_test_support.rs is NOT exempt) + unittest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): assert OAuth grant_type at the egress (comment 4) ScriptedOAuthTokenEgress now exposes captured_bodies() (body bytes only — not the ZeroizeOnDrop RuntimeHttpEgressRequest). The connect test asserts the exchange uses authorization_code; the refresh test asserts the sweep uses refresh_token. Distinguishes the two OAuth flows — a connect/refresh path mixup would otherwise pass the count-only assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci(no-panics): exempt test_support directory modules too, document scan scope Make the test-support exemption future-proof: exempt `test_support` as a path component (src/test_support/**), not just the single-file `test_support.rs`, so growing a test-support module into a directory needs no further change. Also document that the scanner only looks at src/ + crates/ — top-level tests/** integration tests and their support trees are never scanned. unittests cover the directory form + the 'test_supportish' / 'my_test_support' non-matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(check_no_panics): require src/ for test_support exemptions The test_support exemption in is_test_only_path was too broad: a hypothetical crates/foo/bin/test_support.rs (a binary, compiled into production) would have been wrongly exempted. Gate both the single-file and directory-component forms on "src" in parts, matching the docstring which already said the exemption is for src/**/test_support.rs. Add two assertFalse assertions for the bin/ cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(harness): reject non-http scheme in LoopbackMcpRuntimeHttpEgress::new `mcp_loopback_network_policy()` only permits `http`, so `https://127.0.0.1/…` would pass construction silently and fail later at network-authorization time. Add an explicit scheme check immediately after URL parse, before the host check, returning a clear error at construction with a message that names the failing scheme and explains why only `http` is accepted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: move LOCAL_DEV_DB_FILENAME off unconditional public API The constant was on the root crate surface unconditionally but is only consumed by the integration-test harness. Narrow it to pub(crate) in factory.rs, remove the root-level pub use from lib.rs, and expose it as pub const (constant expression) inside the feature-gated test_support module so builder.rs can access it via ironclaw_reborn_composition::test_support::LOCAL_DEV_DB_FILENAME. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(no-panics): tighten test_support exemption to the canonical src/ root CodeRabbit: `"src" in parts` was still too loose — `src/bin/test_support.rs` (compiled into a binary) slipped through. Require the component immediately after `src` to be `test_support` (so only `.../src/test_support.rs` or `.../src/test_support/**` is exempt); src/bin/test_support* and nested src/foo/test_support.rs are not. + regression unittests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
henrypark133
added a commit
that referenced
this pull request
Jun 30, 2026
…emory/secrets/extensions (#5402) * wip: slice4 http matcher Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): slice4 URL-keyed HTTP matcher + egress assertion API Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn): note slice4 keyed HTTP matcher + egress assertion API Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * slice3: production visibility promotion + matrix test draft + plan * slice3: promote build_default_local_dev_database_roots pub(crate) + add test accessor * slice3: RebornThreadHarness<F=LocalFilesystem> generic + filesystem_shared_composite + prefix-param scoped_threads_fs_at * slice3: extract turns_scope_path to filesystem.rs, shrink harness.rs scoped_turns_fs * slice3: StorageMode + one-composite build + scoped_turns_fs_composite + assert_reply_persists_after_reopen * slice3: add rstest = "0.23" dev-dep + libsql feature on reborn_composition dev-dep * slice3: update CLAUDE.md + design spec (§3.2/§9 step 4 done, Option C) Mark step 4 done in §9 build order; record Option-C decision (one CompositeRootFilesystem for both InMemory and LibSql, same path layout) and the visibility promotion details in §3.2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: fix private-type-in-pub(crate)-return compile error Add `mount_default_local_dev_database_roots` (void wrapper) so test_support.rs never has to name the module-private `LocalDevDurableBackend` type. Callers of the void wrapper get the same 4-step libSQL setup without the type leaking across the module boundary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: add missing RootFilesystem where-bound on RebornThreadHarness struct FilesystemSessionThreadService<F> requires F: RootFilesystem; the struct definition needs the same bound so the field type-checks without relying solely on the impl blocks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: cargo fmt Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * slice3: gate mount_default_local_dev_database_roots on test-support feature The function is only called from test_support.rs which is itself gated on feature = "test-support"; without the gate, cargo -p ironclaw_reborn_composition warns dead_code on every non-test build. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 1 — RecordingProcessPort inert process port Add `tests/support/reborn/process.rs`: `RecordingProcessPort` implements `RuntimeProcessPort` but never spawns an OS process — records each command string and returns exit 0 / empty output. Registered as `pub mod process` in the support tree's `mod.rs`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 2 — inject RecordingProcessPort + add SHELL_CAPABILITY_ID - `local_dev_host_runtime_with_registry_and_runtime_http_egress` / `with_http_egress` accept `Option<Arc<dyn RuntimeProcessPort>>` and call `.with_runtime_process_port_dyn` when `Some` (None = default LocalHostProcessPort). - `core_builtin_tools_with_network_policy` creates `RecordingProcessPort`, injects it, and stores it on `harness.process_port` (mirrors http_egress threading pattern). - `core_builtin_tools_with_live_shell()` new constructor: skips injection so the real `LocalHostProcessPort` runs (used by step 3's `.with_live_shell()`). - `SHELL_CAPABILITY_ID` added to `core_builtin_tools_from_runtime` capability surface; `SpawnProcess` added to effect_kinds so the capability port allows it. - `process_port` field + `process_commands()` accessor on `HostRuntimeCapabilityHarness`. - `recorded_process_commands()` on `HarnessCapabilityRecorder` (mirrors runtime_http_requests). - No production file touched; all changes are within tests/support/reborn/harness.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 3 — .with_live_shell() opt-in on builder Add `live_shell: bool` to `RebornIntegrationHarnessBuilder` (default false). `.with_live_shell()` sets the flag and implies `BuiltinHttpTools` capability backend. In `build()`, the `BuiltinHttpTools` arm dispatches to `core_builtin_tools_with_live_shell()` when the flag is set — routing the HostRuntime backend to use the real `LocalHostProcessPort` instead of the inert `RecordingProcessPort`. No production file touched. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 4 — assert_shell_command_recorded + assert_no_real_process_executed Add two assertion methods on `RebornIntegrationHarness`: - `assert_shell_command_recorded(substr)`: passes when a recorded command contains substr. - `assert_no_real_process_executed()`: passes when the inert port captured ≥1 command (the recording path ran, not the live-shell opt-in). Both read from `capability_recorder.recorded_process_commands()` (the new slice-5 accessor on `HarnessCapabilityRecorder`). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 5 step 5 — reborn_integration_process_port.rs test Proves builtin.shell dispatches through the inert RecordingProcessPort by default: shell_call_recorded_not_executed asserts the command was recorded and no real OS process spawned; shell_assertions_fail_when_no_shell_call_ran guards against vacuous pass on empty command lists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): slice 5 step 6 — update CLAUDE.md + design spec §3.6/§9/§10 CLAUDE.md: document process.rs + slice-5 assertions + .with_live_shell(); remove inert process port from Planned. Design spec: mark §3.6 shell row Built (slice 5), correct §3.6 injection-seam paragraph (no prod change needed; with_runtime_process_port_dyn was already pub), mark §9 step 5b DONE, add §10 resolved decision. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): slice 5 — add ExecuteCode to core_builtin_tools effect_kinds builtin.shell declares EffectKind::ExecuteCode in its manifest; the GrantAuthorizer checks effects_are_covered(descriptor.effects, grant.allowed_effects) and denied the capability because ExecuteCode was absent from the harness effect_kinds grant. This caused the shell CapabilityInvocation to be denied (outside_visible_surface) before reaching the process port, so no command was ever recorded. Fix: add EffectKind::ExecuteCode to core_builtin_tools_from_runtime's effect_kinds vec so the shell capability appears in the visible surface and dispatches through the RecordingProcessPort as intended. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): cargo fmt after slice 5 fix Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(reborn-itest): slice 6 — MCP mock end-to-end Adds the MCP-mock tier of the Reborn integration-test framework. - `LoopbackMcpRuntimeHttpEgress`: test-only `RuntimeHttpEgress` that makes real reqwest HTTP to the loopback mock server, injects a Bearer token, and rejects URLs outside the configured mock endpoint (hermetic guard). - `LoopbackMcpRuntime` type alias + `local_dev_host_runtime_with_registry_egress_and_mcp` helper: wires the custom egress into a real `McpRuntime<McpHostHttpClient<…>>`. - `mock_mcp_extension_package`: builds a minimal `ExtensionPackage` with `ExtensionRuntime::Mcp` for the fixed `mock-mcp` provider. - `HostRuntimeCapabilityHarness::mock_mcp_tools`: async constructor that assembles all of the above into a `HostRuntimeCapabilityHarness`. - `RebornCapabilityBackend::MockMcp` variant + `with_mock_mcp(url)` builder method on `RebornIntegrationHarness`. - `assert_mcp_tool_called(tool_name)` on `RebornIntegrationHarness`: maps `"search"` → capability id `"mock-mcp.search"` and delegates to `assert_tool_invoked`. - `tests/reborn_integration_mcp.rs`: two tests — `mcp_tool_call_reaches_mock_server` (core scenario) and `assert_mcp_tool_called_fails_when_no_mcp_call_ran` (guard). - `ironclaw_mcp` added to `[dev-dependencies]` in workspace `Cargo.toml`. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(reborn-itest): use inline MCP descriptor schema to avoid $ref filesystem read mock_mcp_extension_package used from_manifest which sets parameters_schema to {"$ref": "schemas/mock-mcp/mock.input.v1.json"}. surface_descriptor in CapabilitySurface::visible_capabilities then tries to read that schema file from the host filesystem, which fails (the file doesn't exist for a test-only mock extension). This produced host_creation_failed at turn dispatch. Switch to from_host_bundled_manifest_with_inline_dynamic_schemas with an inline {"type":"object"} parameters_schema. surface_descriptor sees no $ref and returns Ok(descriptor) early, unblocking visible_capabilities → build_text_only_host_with_profiled_capabilities → create_host. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt on reborn itest slice 6 files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): document slice 6 MCP mock in harness CLAUDE.md Records the LoopbackMcpRuntimeHttpEgress, mock_mcp_extension_package (inline-dynamic-schemas fix), local_dev_host_runtime_with_registry_egress_and_mcp, .with_mock_mcp(), assert_mcp_tool_called(), and reborn_integration_mcp.rs in the authoritative authoring guide. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): mark slice 6 MCP mock done in design spec Updates §3.6 table, P1-ergonomics paragraph, built-slice-4 section, §9 step-5b trailing note, and new §9 step-5c to record that .with_mock_mcp() and assert_mcp_tool_called() shipped in slice 6 via LoopbackMcpRuntimeHttpEgress + from_host_bundled_manifest_with_inline_dynamic_schemas. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(itest-slice7): add OAuth test-support types and factory to test_support.rs Adds ScriptedOAuthTokenEgress, OAuthProductAuthTestBundle, and build_oauth_product_auth_for_test() to ironclaw_reborn_composition's test-support module (feature = "test-support"). Wires a real FilesystemAuthProductServices<InMemoryBackend> over a fixed-view ScopedFilesystem with a noop obligation handler and noop continuation dispatcher so OAuth connect-flow integration tests can drive the full claim→exchange→complete path with no network and no feature-gated deps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(itest-slice7): add reborn_integration_oauth_connect test (slice 7) Two tests exercise the OAuth connect-flow seam: - oauth_connect_flow_persists_credential_account: drives create_flow → handle_oauth_callback → get_account; asserts account persisted and exactly one scripted token-exchange HTTP call captured. - oauth_callback_without_prior_flow_fails: guard test; missing flow produces UnknownOrExpiredFlow and zero egress calls. No production files touched. No network, no services, no integration feature required. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt slice 7 test_support + oauth_connect test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(itest-slice7): update CLAUDE.md + design spec for OAuth connect-flow slice - CLAUDE.md: add Slice 7 entry describing ScriptedOAuthTokenEgress, OAuthProductAuthTestBundle, build_oauth_product_auth_for_test(), and the two new tests; remove "product/auth" from the Planned list. - Design spec §3.6: update Secrets/OAuth row to note ScriptedOAuthTokenEgress + build_oauth_product_auth_for_test() as the opt-in for full OAuth flows. - Design spec §3.8: add Slice 7 wiring exception (standalone bundle via FilesystemAuthProductServices<InMemoryBackend> + fixed ScopedFilesystem). - Design spec §9: mark step 7 DONE with Slice 7 OAuth connect-flow detail. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): pub(crate) LocalDevDurableBackend to silence private_interfaces The slice-3 promotion of build_default_local_dev_database_roots to pub(crate) exposed the private LocalDevDurableBackend enum in a pub(crate) signature, tripping private_interfaces (which the CI -D warnings lane fails on). Bump the enum to pub(crate) to match; it stays crate-internal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(itest-slice8): clock injection — make sweep_once pub(crate) with now: DateTime<Utc> Production path unchanged: tick_once passes Utc::now(). Tests can pass a frozen instant to make just-created accounts appear idle, enabling deterministic keepalive-refresh assertions without sleep. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(itest-slice8): add OAuth refresh test-support fixtures - ScriptedOAuthTokenEgress::with_access_and_refresh_token(): stores a refresh_token in the scripted response so the exchange phase writes a refresh secret handle. - FixedCandidateSource: crate-private struct impl CredentialRefreshCandidateSource; injects a pre-seeded account list into sweep_once without the filesystem tenant-path walk. - OAuthProductAuthTestBundle::sweep_for_refresh(): drives one sweep tick with a fixed account list and a frozen clock, wiring the always-leader lock and the real ProviderBackedCredentialAccountService refresh path. - build_google_oauth_product_auth_for_test(): same as build_oauth_product_auth_for_test but with provider_id="google", refresh_token in the egress response, and .with_provider_client() so refresh_account does not short-circuit to BackendUnavailable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(itest-slice8): gate slice-8 test support on libsql|postgres; add [[test]] entry FixedCandidateSource, sweep_for_refresh, and build_google_oauth_product_auth_for_test all depend on credential_refresh_worker which is gated on any(feature = "libsql", feature = "postgres"). Gate the new items the same way so builds without durable-backend features still compile. Add [[test]] name = "reborn_integration_oauth_refresh" with required-features = ["libsql"] to the root Cargo.toml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(itest-slice8): add reborn_integration_oauth_refresh test Two tests: - credential_refresh_sweep_refreshes_idle_google_account: positive test using frozen clock (Utc::now() + 3 days) so just-created account appears past the 2-day idle threshold; asserts egress.captured_count() == 2 (initial exchange + refresh call). - credential_refresh_sweep_skips_fresh_google_account: guard test using Utc::now() so just-created account is within the idle threshold; asserts egress.captured_count() stays at 1 (no refresh). Both tests drive the full sweep_once → ProviderBackedCredentialAccountService → HostOAuthProviderClient → ScriptedOAuthTokenEgress path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: cargo fmt reborn_integration_oauth_refresh test * docs(itest-slice8): record slice 8 (OAuth refresh + clock injection) in design spec §9 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(itest): [B1] move cfg attribute below consolidated doc block in build_google_oauth_product_auth_for_test The #[cfg(any(feature = "libsql", feature = "postgres"))] attribute was wedged inside the doc-comment (between two bullet groups), making the second group orphaned. Consolidate all /// lines into one block above the attribute, and reword the gate rationale: gated because sweep_for_refresh (the primary consumer) requires credential_refresh_worker, which is compiled only under those features. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(itest): [B2] extract build_oauth_product_auth_infra() shared preamble build_oauth_product_auth_for_test and build_google_oauth_product_auth_for_test shared a verbatim ~8-line preamble (MountView/MountGrant setup, InMemoryBackend, ScopedFilesystem::with_fixed_view, InMemorySecretStore, FilesystemAuthProductServices). Extract it into a private build_oauth_product_auth_infra() helper returning the three types both callers need. Remove the duplicated block and the "Same fixed-view mount layout as…" comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(itest): [Nit] add #[cfg(feature = \"test-support\")] to mount_local_dev_database_roots_for_test The sibling build_default_local_dev_database_roots_for_test carries the explicit attribute; this function's own doc-comment claims it is gated behind test-support but the attribute was missing. Add it for consistency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(itest): [S1] rename assert_no_real_process_executed to assert_shell_ran_through_inert_port The name implied a negative check but the body passes when ≥1 command was recorded through the inert port. Rename to the accurate positive form and update the doc to lead with the positive condition. Update both call sites in reborn_integration_process_port.rs. Behavior is identical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(itest): [MockMcp const] collapse MockMcp fields to mcp_url, extract MOCK_MCP_PROVIDER_ID The MockMcp variant carried provider_id/capability_id fields that were always the constants "mock-mcp"/"mock-mcp.search" (set only in with_mock_mcp, no other setter), and assert_mcp_tool_called independently rebuilt format!("mock-mcp.{…}"). Collapse to MockMcp { mcp_url: String }, declare const MOCK_MCP_PROVIDER_ID near the builder, and use it in both the match-arm wiring and assert_mcp_tool_called. One owner for the string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(itest): [S2] add arch-exempt annotation for large MCP block in harness.rs + CLAUDE.md note harness.rs is 4179 lines (>3000, architecture.md §5). Add arch-exempt: large_file annotation on the first line of the MCP wiring block (LoopbackMcpRuntime type alias) noting the harness_mcp.rs split as a tracked follow-up. Add a one-line note in tests/support/reborn/CLAUDE.md referencing the planned sub-module split. Do not split the file now. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt after code-review fixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): clear CI clippy — type_complexity + derivable_impls - B2 helper return type tripped clippy::type_complexity (3-tuple of nested Arcs) → return a named OAuthProductAuthInfra struct (drop the unused scoped_fs handle; durable holds its own Arc clone). - StorageMode manual Default impl tripped clippy::derivable_impls → derive Default with #[default] on InMemory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): slice 3 negative guard — reopen assertion fails on mismatch Closes the only guard-test gap the verification pass found: every other slice proves its assertion can fail (non-vacuous); slice 3's LibSql reopen read-back now has a matching guard asserting assert_reply_persists_after_reopen returns Err when the expected text is absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn-itest): fold slice 9 embeddings-descope verdict into combined spec Folds the slice-9 finding (PR #5386) into the combined framework branch: the embeddings fake is descoped because the Reborn memory path never consults EmbeddingProvider (NativeMemoryService forces with_vector(false); backend wires embedding_provider: None) and uses ironclaw_memory_native::EmbeddingProvider, not the spec-named ironclaw_embeddings one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn-itest): genuine disk-durability in assert_reply_persists_after_reopen `assert_reply_persists_after_reopen` was re-instantiating the thread service over the **same** in-process `CompositeRootFilesystem` Arc for both InMemory and LibSql modes, so `libsql_persists_reply_across_reopen` passed even when `StorageMode::LibSql` secretly used `InMemoryBackend` (the mutation stayed GREEN — the coverage gap). Fix: when `StorageMode::LibSql`, open a **genuinely fresh** `libsql::Builder::new_local(db_path)` connection independent of the live composite, run migrations (idempotent), mount via `mount_local_dev_database_roots_for_test`, and read thread history through the fresh handle. Only data serialized+committed to the `.db` file is visible through the new connection; an InMemory mutation leaves the file absent/empty, so `list_thread_history` returns nothing and `assert_final_reply` returns `Err(MissingFinalReply)` — mutation goes RED. For InMemory the existing same-handle `reopened()` path is kept (no disk involved; tests service re-instantiation, not durability). Also captures `libsql_db_path: Option<PathBuf>` on the harness via the updated `build_storage_composite` return type so the reopen path can locate the file without rediscovering it. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test(reborn-itest): prove MCP egress reaches mock server (M4 coverage gap) `assert_mcp_tool_called` checked `capability_recorder.invocations()` which fires before HTTP egress, so a wrong URL or dead server still passed. Add two assertions after `assert_mcp_tool_called` in the POSITIVE test only: 1. `assert!(!server.recorded_requests().is_empty(), …)` — verifies that the loopback mock MCP server received at least one HTTP POST. 2. `assert!(recorded.iter().any(|r| r.method == "tools/call"), …)` — verifies that at least one recorded request carries the JSON-RPC method `"tools/call"` (the field `RecordedMcpRequest.method` holds the JSON-RPC method string, captured in `handle_mcp` before dispatch). Together these prove the MCP runtime made a real HTTP round-trip to the loopback server, not just that the capability recorder fired pre-egress. URL-corruption mutations or dead-server mutations now go RED. The negative guard `assert_mcp_tool_called_fails_when_no_mcp_call_ran` is unchanged — it scripts no MCP turn so `recorded_requests` stays empty, and the new assertions are not present in that test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reborn-itest): make MCP tool call genuinely reach the loopback mock server The stronger slice-6 assertion (commit 02157b3) exposed that the scripted MCP tool call never reached the loopback mock server — three real blockers, all upstream of HTTP egress: 1. Trust policy. `mock_mcp_tools` wired `first_party_trust_policy()`, which only trusts the builtin first-party provider. The mock MCP provider's manifest (`/system/extensions/mock-mcp/manifest.toml`) had no trust entry, so `evaluate_invocation_trust` produced a Sandbox ceiling and dispatch was denied. Added `first_party_and_mcp_trust_policy(provider_id)` granting the mock provider `user_trusted` for `DispatchCapability` + `Network`, keyed on the same `LocalManifest` path the host runtime derives at dispatch time. 2. Network policy. `mock_mcp_tools` set `NetworkPolicy::default()` (empty `allowed_targets`). The MCP capability declares `EffectKind::Network`, so authorization attaches an `ApplyNetworkPolicy` obligation that the host runtime's `validate_network_policy_metadata` rejects when `allowed_targets` is empty — blocking the egress before any HTTP. Added `mcp_loopback_network_policy()` permitting host `127.0.0.1` (scheme http) with `deny_private_ip_ranges = false` (127.0.0.1 is loopback/private). 3. Notification status. The mock answered JSON-RPC notifications (`notifications/initialized`, id=None) with `200 OK` + empty body. Per the MCP Streamable HTTP spec a notification-only body MUST get `202 Accepted`; the real client (`send_planned_json_rpc`) only treats 202 as a valid empty-body ack and otherwise parses the empty 200 as a JSON-RPC response, which fails and aborts `initialize_session` before `tools/call` is sent. Mock now returns `202 Accepted` for notifications. Also hardens `LoopbackMcpRuntimeHttpEgress::new` (CodeRabbit #5): validate that `mcp_url`'s host is loopback (127.0.0.1 / ::1 / localhost) and error otherwise, so a typo cannot silently turn the test egress into real external network I/O. With all three fixed, `mcp_tool_call_reaches_mock_server` passes WITH the stronger assertion: the mock records `initialize`, `notifications/initialized`, and `tools/call`. The negative guard is unaffected (no MCP turn scripted). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn): prove OAuth refresh sweep commits the rotated credential The slice-8 refresh test asserted only egress.captured_count() == 2 — i.e. that the refresh HTTP call fired. That would still pass if the refresh made the call but silently dropped the account write-back. Strengthen the positive test to re-read the account through the durable CredentialAccountService and assert the persisted access-token handle was rewritten to the refresh-path handle (`…-oauth-refresh-access-<account_id>`, produced only by HostOAuthProviderClient::store_refreshed_tokens) and differs from the connect-exchange handle. A dropped account write would leave the connect handle in place, so both assertions fail in that case. No production change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn-itest): narrow loopback guard to 127.0.0.1 and disable redirects in LoopbackMcpRuntimeHttpEgress PR review comments 2 and 3 on #5392: - Comment 2: narrow `LoopbackMcpRuntimeHttpEgress::new` host check from accepting 127.0.0.1/::1/localhost to 127.0.0.1 only, matching `mcp_loopback_network_policy()` which also only allows 127.0.0.1. A "localhost" URL would previously pass the egress guard then fail network authorization — a latent trap. - Comment 3: add `.redirect(reqwest::redirect::Policy::none())` to the reqwest Client builder so a mock 3xx cannot redirect the client off loopback; the `starts_with(mcp_url)` hermetic guard only checked the first request URL, not redirect hops. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(reborn-mcp): capture params in RecordedMcpRequest and assert tool name in tools/call Addresses CodeRabbit PR #5392 comment: the MCP test could only assert that *some* tools/call arrived, not which tool was called. - `RecordedMcpRequest` gains `pub params: Option<serde_json::Value>`; the handler now sets it from `req.params.clone()` on every request. - `mcp_tool_call_reaches_mock_server` replaces the bare `any(tools/call)` assertion with a find + `assert_eq!` on `params["name"] == "search"`, proving the right tool was dispatched with the right wire shape. - Additive change: no other `RecordedMcpRequest` constructor exists; all other consumers only read `recorded_requests()` and compile clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(reborn-itest): fix stale helper name in CLAUDE.md (assert_shell_ran_through_inert_port) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(reborn-itest): extract LOCAL_DEV_DB_FILENAME const; harness uses canonical name One string owns the local-dev SQLite filename. The integration-test harness no longer duplicates "reborn-local-dev.db" — it reads the constant through the public crate API so any future rename is a single-site change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(reborn-itest): add capability-keyed HTTP response matching test Scripts two responses for the same URL with different with_capability() keys. The first entry has a wrong key, so the builtin.http call falls through (capability mismatch) to the second entry, which matches. Proves that the first-match-wins matcher skips entries whose capability key doesn't match and falls through to subsequent entries. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: cargo fmt reorder LOCAL_DEV_DB_FILENAME re-export in lib.rs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(no-panics): exempt feature-gated test_support.rs modules scripts/check_no_panics.py flagged .unwrap()/.expect() on constant literals in crates/ironclaw_reborn_composition/src/test_support.rs. That module is `#[cfg(feature = "test-support")] pub mod test_support;` — the test-support feature is enabled only via [dev-dependencies], so it ships zero bytes in production binaries (same 'never compiled in production' rationale the check already uses to exempt tests/ and tests.rs). Repo-wide convention across 5 crates. Exempt by exact filename (my_test_support.rs is NOT exempt) + unittest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): assert OAuth grant_type at the egress (comment 4) ScriptedOAuthTokenEgress now exposes captured_bodies() (body bytes only — not the ZeroizeOnDrop RuntimeHttpEgressRequest). The connect test asserts the exchange uses authorization_code; the refresh test asserts the sweep uses refresh_token. Distinguishes the two OAuth flows — a connect/refresh path mixup would otherwise pass the count-only assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci(no-panics): exempt test_support directory modules too, document scan scope Make the test-support exemption future-proof: exempt `test_support` as a path component (src/test_support/**), not just the single-file `test_support.rs`, so growing a test-support module into a directory needs no further change. Also document that the scanner only looks at src/ + crates/ — top-level tests/** integration tests and their support trees are never scanned. unittests cover the directory form + the 'test_supportish' / 'my_test_support' non-matches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(check_no_panics): require src/ for test_support exemptions The test_support exemption in is_test_only_path was too broad: a hypothetical crates/foo/bin/test_support.rs (a binary, compiled into production) would have been wrongly exempted. Gate both the single-file and directory-component forms on "src" in parts, matching the docstring which already said the exemption is for src/**/test_support.rs. Add two assertFalse assertions for the bin/ cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(harness): reject non-http scheme in LoopbackMcpRuntimeHttpEgress::new `mcp_loopback_network_policy()` only permits `http`, so `https://127.0.0.1/…` would pass construction silently and fail later at network-authorization time. Add an explicit scheme check immediately after URL parse, before the host check, returning a clear error at construction with a message that names the failing scheme and explains why only `http` is accepted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: move LOCAL_DEV_DB_FILENAME off unconditional public API The constant was on the root crate surface unconditionally but is only consumed by the integration-test harness. Narrow it to pub(crate) in factory.rs, remove the root-level pub use from lib.rs, and expose it as pub const (constant expression) inside the feature-gated test_support module so builder.rs can access it via ironclaw_reborn_composition::test_support::LOCAL_DEV_DB_FILENAME. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci(no-panics): tighten test_support exemption to the canonical src/ root CodeRabbit: `"src" in parts` was still too loose — `src/bin/test_support.rs` (compiled into a binary) slipped through. Require the component immediately after `src` to be `test_support` (so only `.../src/test_support.rs` or `.../src/test_support/**` is exempt); src/bin/test_support* and nested src/foo/test_support.rs are not. + regression unittests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn-itest): commit C1-C4 coverage plan + handoff Working scaffolding for the internal-service coverage effort built on the landed #5392 in-process integration-test framework. Deleted in the final cleanup once content lands in code + tests/support/reborn/CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): C1 approval-gate harness primitives (infra) Generalize the integration harness's completion poll into wait_for_status(run_id, expected) (one loop; submit_turn waits Completed, submit_turn_until_blocked waits BlockedApproval, the auth slice will wait BlockedAuth). Add .with_live_approvals() wiring the real local-dev approval stores (file_tools_requiring_approval) and disabling the per-(tenant,user) auto-approve toggle for the run scope so a scripted builtin.write_file blocks on a real gate. Add approve_gate/deny_gate (resolve store via ApprovalResolver approve/deny, then resume_turn with/without GateResumeDisposition::Denied) and enable_auto_approve (CAS settings flip). deny_local_dev_gate added beside approve_local_dev_gate on HostRuntimeCapabilityHarness (keeps impl with the private approval fields); approval.rs re-exports GateRef (types-only). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn-itest): review-clean group-architecture design spec RebornIntegrationGroup shared-persistence design (cross-thread store sharing within a group; parallel isolated groups; Drop cleanup; failure isolation). Review-looped through thermo-nuclear + approach/local-patterns/maintainability; all findings resolved (R1-R11), both re-reviewers READY TO IMPLEMENT. Working scaffolding; deleted in final cleanup once reflected in code + CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): RebornIntegrationGroup shared-persistence infra Adds the group mechanism for cross-thread persistence tests: one Arc<GroupSharedStorage> (composite + product harness + shared Arc<HostRuntimeCapabilityHarness>) shared across per-thread runtimes, so state written by thread A (approvals/auto-approve, memory, etc.) is visible to thread B. - New tests/support/reborn/group.rs: RebornIntegrationGroup (+builder: live_approvals/builtin_tools/extension_lifecycle), GroupSharedStorage, GroupCapability, RebornThreadBuilder, ScenarioReport, run_reborn_group! macro, turn_composite()/capability_harness() accessors. - builder.rs: build() refactored to construct a one-thread GroupSharedStorage and delegate to the shared assemble_thread_runtime(); single-source scope via product_harness.scope (retired run_resource_scope); per-thread baseline_*_count so capture assertions read only their own [baseline..] slice (R2). Single-shot behavior byte-identical (75/75 existing integration tests pass). - Crate test-support accessors (ironclaw_reborn_composition): extension_installation_store_for_test, build_local_dev_secret_store_for_test. - CLAUDE.md interim 'Group tests' section; mod.rs group entry. Design + 2-round review (thermo + approach/local-patterns/maintainability) in docs/superpowers/specs/2026-06-29-reborn-itest-group-architecture-design.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): real approval-gate flow + approvals/memory/secrets coverage Fix the C1 approval infra so the REAL gate path fires (the prior primitives compiled but a BlockedApproval run went terminal Failed): - assemble_thread_runtime now wires a real ApprovalGateEvidenceStore (HarnessApprovalGateEvidence over the shared approval-request store), mirroring production runtime.rs with_approval_gate_evidence, so a blocked run is verified at loop exit and genuinely pauses. - The live-approvals capability harness executes under the run's CANONICAL binding subject user (resolve_canonical_subject_user + with_user_id) so capability dispatch, approval persistence, auto-approve keying, and the evidence lookup share one (tenant,user) — matching production. auto_approve_scope derives from that user; enable_auto_approve/live_approvals disable target it. Tests (all real stack, mock only at the SDK seam): - tests/reborn_group_approvals: gate→approve, gate→deny, and the headline approve-always-persists-cross-thread (thread A enables auto-approve via the shared CAS store → thread B runs the same tool with NO gate). Mutation-verified: skipping the enable makes thread B block (RED). - tests/reborn_group_memory: write MEMORY.md in thread A → read it in thread B over the shared store + committed non-vacuity negative guard (profile dropped: real read-back needs UserProfileSource wiring, deferred). - tests/reborn_integration_secrets (libsql): FilesystemSecretStore write → fresh-db reopen → lease+consume read-back + unknown-handle guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): cross-thread extension-lifecycle persistence reborn_group_extensions: install an extension in thread A → a DIFFERENT thread B sees it installed (extension_search result carries installation_phase:installed) over the shared installation store. Same shared-Arc capability backend as the approvals/memory groups; reuses the extension_lifecycle() group constructor. Committed non-vacuity guard (a never-installed id is absent) + mutation-verified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): C2 auth failure — revoke + invalid_grant sweep reborn_integration_auth_failure (libsql): (a) update_status(Revoked) commits to the durable product-auth store and reads back Revoked; (b) a 400 invalid_grant token response during a refresh sweep drives refresh_account → Revoked end-to-end; negative guard: a normal 200 sweep leaves the account Configured (isolates the revoke to the invalid_grant, not the sweep machinery). Additive (test-support-gated) ScriptedOAuthTokenEgress change: status field + with_error_response(status, error_code) + push_response one-shot override so a success egress can serve a 200 connect then a 400 sweep. Backward compatible; no production change. The live-401→reauth-gate arm is deferred (needs a credentialed capability backend) — noted in the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): address implementation review (3 reviewers, all SHIP) - Remove dead items: run_reborn_group! macro, HarnessCapabilityRecorder:: capability_user_id, and the unused single-shot with_live_approvals() + RebornCapabilityBackend::LiveApprovals variant (group API is canonical; removes a latent auto-approve scope-mismatch path). - Dedup the 3 group constructors via build_base()/into_group() (fixed test-scope strings now live in one place). - Strengthen assertions: extensions asserts installation_phase="installed" (value, not just key); gate_then_deny drops the vacuous scripted-reply assert and instead proves the denied write never executed (no tool result carries its content) — the denied capability is never re-dispatched. - Nits: pub mod group; corrected stale 'out of scope' comment in auth_failure. All group/secrets/auth tests green; fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn-itest): slice-agnostic CLAUDE.md + drop planning docs; fix type_complexity - Rewrite tests/support/reborn/CLAUDE.md slice-agnostic: remove the 'Implemented now vs planned' slice-1..8 prose + 'Planned' block + all slice-N labels; replace with a present-tense capability-organized reference. Keep the Group tests section. - Delete consumed working scaffolding: the C1-C4 plan, the handoff, and the group architecture design spec (rationale preserved in git history + this PR). - Resolve all-features clippy type_complexity: alias ScriptedResponseQueue for the ScriptedOAuthTokenEgress response-override queue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): address PR #5402 review comments Straightforward review fixes from coderabbit/codex/gemini + self-review: - Approve scenarios assert the REAL persisted side effect, not the scripted reply: new `assert_workspace_file_contains` / `assert_workspace_file_absent` read the actual on-disk workspace file. `gate_then_approve` and `approve_always_persists_cross_thread` now verify the approved write landed; `gate_then_deny` verifies the file is absent. (`builtin.write_file`'s capability result does not echo the written content, so the prior result-scan assertion was vacuous.) Exposes `HarnessCapabilityRecorder::workspace_file_path` as pub(crate). - builder.rs: per-thread `baseline_process_count`; shell assertions (`assert_shell_command_recorded`, `assert_shell_ran_through_inert_port`) now slice `[baseline..]` so a group thread cannot pass on an earlier thread's recorded command. - test_support.rs: replace `captured_bodies()` (leaked raw OAuth authorization code / refresh token) with redacted `captured_grant_types()`; migrate the oauth_connect / oauth_refresh callers off raw-body assertions. - oauth_connect guard test now asserts no credential account was created (real `list_accounts`) and zero token egress, matching its doc. - New FIFO + default-fallback test for `ScriptedOAuthTokenEgress` (`with_error_response` + queued `push_response`). - MCP test scripts a distinctive argument and asserts it crossed the HTTP boundary intact (not just the tool name). - Fix stale comments: memory group `report.record` rationale, `session_thread::reopened` doc, CLAUDE.md accessor reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): address multi-agent review (code-review + thermo-nuclear) Structural / fidelity: - AP1: delete the hand-mirrored `HarnessApprovalGateEvidence`; wire the REAL production `LocalDevApprovalGateEvidence` via a new test-support factory (`build_local_dev_approval_gate_evidence_for_test`) so the gate-evidence lookup can never drift from production. - A1: move `assemble_thread_runtime` from builder.rs to group.rs (fields made pub(crate)); builder.rs drops 1092 -> 870 lines. - A3: replace the `build_base()` ghost-tuple with a `GroupBaseData` struct. - A4: `live_approvals` auto-approve disable is now fail-loud (expect/let-else unreachable) instead of an always-true `if let` that could vacuously pass. - A5: remove the dead `GroupSharedStorage.storage` field. - A2: extract the byte-identical `connect_google_account` OAuth helper into `tests/support/reborn/oauth_flow.rs` (shared via the existing #[path] tree); drop both local copies and the incorrect "can't be re-exported" comment. Correctness / hygiene: - SEC1: `assert_workspace_file_contains` reports only byte length on failure, never the full file contents (CI-log secret-leak surface). - P1: poison-recovery on all `pending_approval_scopes` lock sites. - LP3: move the `gate:approval-` prefix check into `submit_turn_until_blocked`; remove the duplicated inline checks from the approve/deny scenarios. - Collapse thin auto-approve wrapper methods on HostRuntimeCapabilityHarness. Coverage: - T1: LibSql-backed approvals group test (gate/approve/deny/auto-approve-persist over a real on-disk backend, not just InMemory). - T2: extension remove -> cross-thread search-absent scenario (uses "notion" to stay independent of scenario 1's "github"), with a non-vacuity guard. Docs: probe-uniqueness comment, test_support.rs preamble, FixedCandidateSource follow-up TODO, secrets libsql-gate rationale, and name the production call site (build_local_runtime) on the extension-store test-support accessors. Deferred (with rationale): unify the two harness.rs host-runtime constructors (the tracked harness_mcp.rs extraction is the real fix), split test_support.rs (806 < 1500 threshold), FixedCandidateSource real-enumeration test (refresh path already covered at full fidelity; tracked by TODO). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(reborn-itest): address follow-up review comments on the review fixes - scenario_remove_then_absent_cross_thread.rs / reborn_group_extensions main.rs: fix stale "github" comments — the scenario operates on "notion" (kept independent of Scenario 1's "github" install); comments now match the code. - runtime.rs / test_support.rs: keep `LocalDevApprovalGateEvidence` (struct + field) private; expose a `#[cfg(feature="test-support")]` constructor `build_local_dev_approval_gate_evidence_for_test` in runtime.rs instead of widening the type to pub(crate). test_support's factory now delegates to it, so no runtime internal leaks across the crate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 9 — embeddings fake: STOP verdict (seam unreachable)
Slice 9 of the Reborn integration-test framework was scoped to add a Tier-1 deterministic fake
EmbeddingProvider(design spec §3.6/§3.7). The slice's first job was to verify the seam is real. It is not — there is no embeddings seam to intercept in the Reborn harness memory path, so the fake is descoped. This PR is docs-only: it records the verdict + evidence in the design spec and the reborn test-supportCLAUDE.mdso the analysis isn't repeated by a future slice.Evidence (base
6a3b10fa5)builtin.memory_search/memory_write/memory_read/memory_tree) dispatch only throughNativeMemoryService.MemoryCapabilityState::service_for(crates/ironclaw_host_runtime/src/first_party_tools/memory.rs:161) always buildsNativeMemoryService::from_filesystem(...); the only override (memory_service_for_test) is#[cfg(test)], internal to that crate and unreachable from the cross-cratetests/support/reborn/harness.NativeMemoryService::search(crates/ironclaw_memory_native/src/service.rs:104) hardcodes.with_vector(false)on every request — no parameter, no opt-in — soEmbeddingProvider::embedis never called on a search.build_native_backend(service.rs:415) wiresChunkingMemoryDocumentIndexer::new(repository)withembedding_provider: Noneand never calls.with_embedding_provider;build_chunk_writes(..., None)persists text-only chunks, and the backend swallows indexer errors.EmbeddingProvider/MemoryServicereference exists anywhere intests/support/reborn/. memory_search returns correct membership via FTS with zero embeddings wiring.ironclaw_memory_native::EmbeddingProvider, a different trait from theironclaw_embeddings::EmbeddingProvidernamed in the build order. The latter is wired only into the v1Workspace(src/workspace/mod.rs,src/app.rs::init_tools), which the Reborn harness does not exercise.Conclusion
A fake
EmbeddingProviderwould have nothing to intercept. Membership over memory_search is already covered for free by FTS through the realNativeMemoryServicepath — the future memory-coverage slice should assert membership there, not stand up a fake. Re-scope a fake only if the Reborn memory path later gains a vector-search path that consultsEmbeddingProvider(i.e.NativeMemoryServicestops forcingwith_vector(false)and a cross-crate provider-injection accessor is added).Changes
docs/superpowers/specs/2026-06-26-reborn-integration-test-framework-design.md— §3.6 Embeddings row + new "Embeddings — no fake (Slice 9 verdict)" paragraph; §3.7 Tier-1 list annotation.tests/support/reborn/CLAUDE.md— "Implemented now vs planned": embeddings fake marked descoped with reason.No production or test code changes. fmt/clippy/test gates are N/A (docs-only).
🤖 Generated with Claude Code