test(airc-remote): end-to-end integration through real substrate stack (task #118) - #1563
Merged
Merged
Conversation
…k (task #118 slice E) Closes the load-bearing gap PR #1560's wire test left open. That test proved the wire SHAPE was correct (parse_envelope + send_reply accept the right envelope shapes + headers) but HAND-CANNED the response substrate-side. The substrate's actual CommandExecutor + ServiceModule + AIProviderAdapter dispatch chain was never exercised across the airc wire. This commit closes that gap with three integration tests that run peer_a as the FULL substrate stack: CommandRequestHandler::on_envelope -> parse_envelope [REAL] -> process_request_via(&executor) [REAL] -> CommandExecutor::execute_with_caller -> AuthPolicy::gate (AllowAll) [REAL] -> TestInferenceModule::handle_command -> {Heuristic|AlwaysFailing}InferenceAdapter::generate_text [REAL] -> AircCommandResponse::ok(result) or ::error(msg) -> send_reply [REAL] No canned responses. peer_b runs AircRemoteInferenceAdapter via AircLiveTransport and observes whatever the substrate actually produces. Three test cases: 1. end_to_end_heuristic_dispatch_through_substrate_stack — happy path. HeuristicInferenceAdapter answers; assert response.text starts with "[heuristic:" prefix (the adapter's deterministic signature, which no canned test stub can produce) AND contains the prompt ("hello grid") AND response.provider == "airc-remote" (added by the caller-side adapter layer). Proves a real adapter executed substrate-side end-to-end. 2. end_to_end_peer_adapter_failure_surfaces_as_typed_error — error path. AlwaysFailingAdapter returns Err substrate-side; AircCommandResponse::error propagates through; caller's AircLiveTransport classifies as RemoteInferenceError::PeerAdapterFailed. Closes task #218 (deferred from PR #1560 R3-N3 review). 3. end_to_end_missing_module_returns_typed_error — observability test. peer_a's executor has NO module registered. Surfaces what the substrate CURRENTLY does, which is to fall through to the legacy TypeScript bridge at /tmp/jtag-command-router.sock. The test pins THAT behavior so any future change is loud. The fall-through is itself a [[no-fallbacks-ever]] violation worth a dedicated slice (filed as task #219). Design notes captured in the test file doc-block: - Why a test-only ServiceModule (TestInferenceModule) instead of AIProviderModule: the latter uses a process-global AdapterRegistry that fights with multi-test parallelism. The substrate's wire path is module-agnostic, so any ServiceModule registered for "ai/generate" exercises the same dispatch chain. TestInferenceModule wraps an Arc<dyn AIProviderAdapter> directly so each test owns its own adapter instance, no global mutation, tests run in parallel. - Why TestInferenceModule and not just inline canned modules: gives the test access to the real AIProviderAdapter trait, which is what a production deployment uses. Swapping HeuristicInferenceAdapter for AlwaysFailingAdapter (in the same TestInferenceModule shape) keeps the per-test wiring identical and isolates the test variable to JUST the adapter behavior. - Why we clone the Arc<TranscriptEvent> at the on_envelope boundary: airc-lib's subscribe() yields broadcast-shape Arc<TranscriptEvent> so multiple subscribers see the same event. ConsumerAdapter::on_envelope takes by owned value, so we deref-clone. A productized dispatch loop does the same — the Arc only exists for fan-out, not for ownership transfer. Closes: - Task #118 (peer-side handler — receive remote inference envelopes, route to local adapter) - Task #218 (PR #1560 follow-up: AircCommandResponse::Error variant integration coverage) Surfaces (filed as follow-ups, not addressed here): - Task #219 (CommandExecutor TS-bridge fallback is a [[no-fallbacks-ever]] violation — should hard-error on missing module in headless-Rust deployments) Verified: cargo test -p continuum-core --features metal,accelerate,test-fixtures --test airc_remote_inference_end_to_end -> 3/3 passed (0.96s) Net diff: +405 lines new test file. Zero production-code changes — the proof that the wire is right is itself the new artifact. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…issing-module assertions R1 LGTM-WITH-NOTES (88 conf, round 1) on PR #1563 flagged two assertion shapes that gate less than declared: N1 (error path): `err.contains("peer adapter failed") OR "the model exploded"`. The first branch literally NEVER matches because the Display impl in `RemoteInferenceError::PeerAdapterFailed` is `"remote peer's adapter failed: ..."` (apostrophe-s). The test passed only via the message-passthrough branch, which means it gated "the error message propagated" but NOT "the typed variant was taken." A future change that surfaced "the model exploded" through a different variant (e.g. `Transport { message }` wrapping the substrate error) would pass too. Fix: AND-pin both `"peer's adapter failed"` (the correct apostrophe-s surface) AND `"the model exploded"`. Now the test fails loudly if either the variant changes OR the inner message gets swallowed/ rewritten. N2 (missing module): 5-branch OR including `"ai/generate"` matched almost any plausible error. Author explicitly noted the test was meant to pin TODAY's behavior so when task #219 lands (CommandExecutor hard-errors on missing module instead of falling through to TS bridge) the test SHOULD fail and force maintenance. As written it wouldn't — `"ai/generate"` matches even a silent 200-on-nothing if the success payload happened to include the path. Fix: pin EXACTLY the TS-bridge connect-failure surface (`"commandrouterserver"` OR `"jtag-command-router"`). When #219 lands the test fails loudly with a self-documenting panic message telling the maintainer exactly what changed and what to update. The looser branches removed: `"ai/generate"`, `"no handler"`, `"no module"`. Both findings are observability bugs in the test — the test author declared the test caught X but it actually caught X-or-Y. Per [[every-error-is-an-opportunity-to-battle-harden]] these tighten to catch exactly X. Verified: cargo test -p continuum-core --features metal,accelerate,test-fixtures --test airc_remote_inference_end_to_end -> 3/3 passed (0.94s) Net diff: 2 hunks, +30 / -15 in the existing test file. Same three tests; stricter pins per test. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
joelteply
added a commit
that referenced
this pull request
Jun 9, 2026
…ode + docs Joel called this out: is 5090 coded into the repo? Yes — GPU-specific narrative and operator-personal labels leaked into substrate source, test fixtures, and doc-strings across today's PRs (#1560, #1561, #1563, #1564). None were load-bearing, but the repo should be hardware-agnostic. Replacements (one PR, surgical edits): core/continuum-core/src/inference/airc_remote/adapter.rs - doc: "route to Joel's 5090" -> generic remote-inference-peer description - test fixture: "joels-5090" -> "test-remote-peer" core/continuum-core/src/inference/airc_remote/transport.rs - test fixture: "joels-5090" (2 sites) -> "test-remote-peer" core/continuum-core/tests/airc_remote_inference_roundtrip.rs - doc: "airc://<rtx5090>/ai/generate" -> "airc://<remote-peer>/ai/generate" - peer labels generic: "remote inference host" / "local caller" - canned response: "pong from the remote peer" / "test-model" / "test-remote-llamacpp" apps/cli/src/main.rs - Generate doc: "(e.g., the operator's 5090)" -> generic GPU-rich grid host apps/cli/src/grid_smoke.rs - module doc + ai/generate row comment: "constrained-locally host dispatches at a GPU-rich peer" / "If the target is a GPU host running a real LLM" Out of scope: - Older codebase doctrine attributions ("Joel's never-swallow-errors") stay — those name doctrine origin, fine. - Task #85 mentioning 5090 stays — it's a real airc bug ticket about that hardware. Verified: grep -rn "5090|joels-" on touched files -> zero hits cargo check -p continuum-cli -> clean (1.94s) The pattern lesson: any hardware-specific identity is narrative scaffolding, not substrate truth. The substrate is hardware-agnostic; tests use neutral labels; docs describe categories, not specific units. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
joelteply
added a commit
that referenced
this pull request
Jun 9, 2026
…ode + docs (#1565) Joel called this out: is 5090 coded into the repo? Yes — GPU-specific narrative and operator-personal labels leaked into substrate source, test fixtures, and doc-strings across today's PRs (#1560, #1561, #1563, #1564). None were load-bearing, but the repo should be hardware-agnostic. Replacements (one PR, surgical edits): core/continuum-core/src/inference/airc_remote/adapter.rs - doc: "route to Joel's 5090" -> generic remote-inference-peer description - test fixture: "joels-5090" -> "test-remote-peer" core/continuum-core/src/inference/airc_remote/transport.rs - test fixture: "joels-5090" (2 sites) -> "test-remote-peer" core/continuum-core/tests/airc_remote_inference_roundtrip.rs - doc: "airc://<rtx5090>/ai/generate" -> "airc://<remote-peer>/ai/generate" - peer labels generic: "remote inference host" / "local caller" - canned response: "pong from the remote peer" / "test-model" / "test-remote-llamacpp" apps/cli/src/main.rs - Generate doc: "(e.g., the operator's 5090)" -> generic GPU-rich grid host apps/cli/src/grid_smoke.rs - module doc + ai/generate row comment: "constrained-locally host dispatches at a GPU-rich peer" / "If the target is a GPU host running a real LLM" Out of scope: - Older codebase doctrine attributions ("Joel's never-swallow-errors") stay — those name doctrine origin, fine. - Task #85 mentioning 5090 stays — it's a real airc bug ticket about that hardware. Verified: grep -rn "5090|joels-" on touched files -> zero hits cargo check -p continuum-cli -> clean (1.94s) The pattern lesson: any hardware-specific identity is narrative scaffolding, not substrate truth. The substrate is hardware-agnostic; tests use neutral labels; docs describe categories, not specific units. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
joelteply
added a commit
that referenced
this pull request
Jun 9, 2026
… command dispatch (install module; bootstrap wiring follows in #222) PR #1560 + #1563 proved the cross-grid command wire works end-to-end IN TESTS. Both tests constructed a CommandRequestHandler + spawned a subscribe loop INLINE. Production never installed the handler anywhere. `grep -rn CommandRequestHandler::new core/continuum-core/src/ | grep -v test` returns ZERO hits today. So a real running substrate SILENTLY ignores incoming AircCommandRequest envelopes — peer_b can dispatch `ai/generate` at peer_a, the envelope arrives, no one listens. This commit lands the install MODULE — `PersonaCommandInboundPump`. The bootstrap-wiring step (call `spawn()` from `PersonaAircRuntime::bootstrap`) is task #222 and is the remaining work before any real running substrate becomes addressable for commands. The commit DOES NOT claim to move the doctrine on its own: the test exercises the install path, but no production code path calls `PersonaCommandInboundPump::spawn` until #222 lands. Framing per R2 round-1 review of this PR: this is the install module, not the install. The doctrine moves when both this PR and #222 are merged. ## What lands PersonaCommandInboundPump — a per-persona tokio task that: - subscribes to the persona's own airc handle (broadcast: multiple subscribers see the same events, verified in PR #1563 R3 review + cross-referenced at airc-lib's messaging.rs:204-211) - filters for command-shaped envelopes (HEADER_CONTINUUM_BODY_HINT == COMMAND_REQUEST_BODY_HINT) — skips self-events + non-command envelopes so the chat pump's `body.as_text()` filter at airc_persona_conversation.rs:166 keeps owning text - hands matching envelopes to a CommandRequestHandler bound to the substrate's CommandExecutor — the SAME handler PR #1563's e2e test wires manually - returns `Result<Self, AircError>` from `spawn()` so subscribe failure surfaces at the CALL SITE (per R2 round-1: the previous shape logged `error!` + exited the task silently — "loud-once", which the doctrine says isn't loud enough) Subscribe is now synchronous in `spawn`: opens the EventStream before tokio::spawn, moves the stream into the task. Caller bails immediately on failure rather than declaring the persona ready while it's actually unaddressable. Per `[[personas-are-citizens-airc-is-identity-provider]]`: the substrate has no airc identity of its own — only personas do. So the pump binds to a persona's `Arc<Airc>`, not a substrate-level singleton. When peer_b dispatches `airc://<persona-uuid>/ai/generate`, THAT persona's pump receives the envelope. Two-task pattern (chat pump + command pump on the same airc handle) is doctrinally cleaner than splicing command dispatch into the chat loop: - separation of concerns — chat behavior + cognition / lag shouldn't tangle with command-envelope dispatch - composability — future per-persona inbound subscribers (event- subscribe responses, future bus shapes) add as more peer tasks, not as branches of an existing one - airc-lib does the broadcast fan-out for free ## What the integration test proves `tests/persona_command_inbound_pump.rs`: - sets up peer_a as a persona-shaped substrate (ModuleRegistry + TestInferenceModule wrapping HeuristicInferenceAdapter + CommandExecutor) - calls PersonaCommandInboundPump::spawn ONCE (the production-shape install path; the test never constructs CommandRequestHandler or spawns a manual subscribe loop) - peer_b dispatches ai/generate via AircRemoteInferenceAdapter + AircLiveTransport - asserts response.text starts with `[heuristic:` (signature prefix proves the substrate's FULL dispatch chain ran — pump -> CommandRequestHandler -> CommandExecutor -> TestInferenceModule -> HeuristicAdapter) - asserts the prompt echoed - asserts response.provider == "airc-remote" - calls pump.shutdown() to verify the clean-shutdown path The test pins the MODULE's contract. The PRODUCTION install lands in #222. ## What's deliberately deferred - Wiring PersonaCommandInboundPump::spawn() into PersonaAircRuntime::bootstrap. Filed as task #222. The actual install. Until #222 lands, no real running substrate is addressable for cross-grid commands — this PR's test is the only thing exercising the install path. - Promoting TestInferenceModule to a system fixture (task #221; third inline copy now lives in this PR's test). ## Verified cargo check -p continuum-core --features metal,accelerate --lib -> clean cargo test -p continuum-core --features metal,accelerate,test-fixtures --test persona_command_inbound_pump -> 1/1 (0.90s) Net diff (after R2 round-1 fixes): src/persona/command_inbound_pump.rs (NEW) +201 lines src/persona/mod.rs +1 line tests/persona_command_inbound_pump.rs (NEW) +191 lines ## Process note Original commit message overclaimed ("this IS the install step"). R2 round-1 called it out cleanly: the module is correct, but until #222 lands a real running substrate STILL silently ignores command envelopes — the exact gap the commit said it closed. This amendment softens the claim to match what actually shipped. The doctrine moves when #222 lands, not before. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
joelteply
added a commit
that referenced
this pull request
Jun 9, 2026
… command dispatch (install module; bootstrap wiring follows in #222) (#1567) PR #1560 + #1563 proved the cross-grid command wire works end-to-end IN TESTS. Both tests constructed a CommandRequestHandler + spawned a subscribe loop INLINE. Production never installed the handler anywhere. `grep -rn CommandRequestHandler::new core/continuum-core/src/ | grep -v test` returns ZERO hits today. So a real running substrate SILENTLY ignores incoming AircCommandRequest envelopes — peer_b can dispatch `ai/generate` at peer_a, the envelope arrives, no one listens. This commit lands the install MODULE — `PersonaCommandInboundPump`. The bootstrap-wiring step (call `spawn()` from `PersonaAircRuntime::bootstrap`) is task #222 and is the remaining work before any real running substrate becomes addressable for commands. The commit DOES NOT claim to move the doctrine on its own: the test exercises the install path, but no production code path calls `PersonaCommandInboundPump::spawn` until #222 lands. Framing per R2 round-1 review of this PR: this is the install module, not the install. The doctrine moves when both this PR and #222 are merged. ## What lands PersonaCommandInboundPump — a per-persona tokio task that: - subscribes to the persona's own airc handle (broadcast: multiple subscribers see the same events, verified in PR #1563 R3 review + cross-referenced at airc-lib's messaging.rs:204-211) - filters for command-shaped envelopes (HEADER_CONTINUUM_BODY_HINT == COMMAND_REQUEST_BODY_HINT) — skips self-events + non-command envelopes so the chat pump's `body.as_text()` filter at airc_persona_conversation.rs:166 keeps owning text - hands matching envelopes to a CommandRequestHandler bound to the substrate's CommandExecutor — the SAME handler PR #1563's e2e test wires manually - returns `Result<Self, AircError>` from `spawn()` so subscribe failure surfaces at the CALL SITE (per R2 round-1: the previous shape logged `error!` + exited the task silently — "loud-once", which the doctrine says isn't loud enough) Subscribe is now synchronous in `spawn`: opens the EventStream before tokio::spawn, moves the stream into the task. Caller bails immediately on failure rather than declaring the persona ready while it's actually unaddressable. Per `[[personas-are-citizens-airc-is-identity-provider]]`: the substrate has no airc identity of its own — only personas do. So the pump binds to a persona's `Arc<Airc>`, not a substrate-level singleton. When peer_b dispatches `airc://<persona-uuid>/ai/generate`, THAT persona's pump receives the envelope. Two-task pattern (chat pump + command pump on the same airc handle) is doctrinally cleaner than splicing command dispatch into the chat loop: - separation of concerns — chat behavior + cognition / lag shouldn't tangle with command-envelope dispatch - composability — future per-persona inbound subscribers (event- subscribe responses, future bus shapes) add as more peer tasks, not as branches of an existing one - airc-lib does the broadcast fan-out for free ## What the integration test proves `tests/persona_command_inbound_pump.rs`: - sets up peer_a as a persona-shaped substrate (ModuleRegistry + TestInferenceModule wrapping HeuristicInferenceAdapter + CommandExecutor) - calls PersonaCommandInboundPump::spawn ONCE (the production-shape install path; the test never constructs CommandRequestHandler or spawns a manual subscribe loop) - peer_b dispatches ai/generate via AircRemoteInferenceAdapter + AircLiveTransport - asserts response.text starts with `[heuristic:` (signature prefix proves the substrate's FULL dispatch chain ran — pump -> CommandRequestHandler -> CommandExecutor -> TestInferenceModule -> HeuristicAdapter) - asserts the prompt echoed - asserts response.provider == "airc-remote" - calls pump.shutdown() to verify the clean-shutdown path The test pins the MODULE's contract. The PRODUCTION install lands in #222. ## What's deliberately deferred - Wiring PersonaCommandInboundPump::spawn() into PersonaAircRuntime::bootstrap. Filed as task #222. The actual install. Until #222 lands, no real running substrate is addressable for cross-grid commands — this PR's test is the only thing exercising the install path. - Promoting TestInferenceModule to a system fixture (task #221; third inline copy now lives in this PR's test). ## Verified cargo check -p continuum-core --features metal,accelerate --lib -> clean cargo test -p continuum-core --features metal,accelerate,test-fixtures --test persona_command_inbound_pump -> 1/1 (0.90s) Net diff (after R2 round-1 fixes): src/persona/command_inbound_pump.rs (NEW) +201 lines src/persona/mod.rs +1 line tests/persona_command_inbound_pump.rs (NEW) +191 lines ## Process note Original commit message overclaimed ("this IS the install step"). R2 round-1 called it out cleanly: the module is correct, but until #222 lands a real running substrate STILL silently ignores command envelopes — the exact gap the commit said it closed. This amendment softens the claim to match what actually shipped. The doctrine moves when #222 lands, not before. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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
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.
What this PR adds
Three integration tests that close the gap PR #1560's wire test left open. PR #1560 proved the wire SHAPE round-trips (parse_envelope + send_reply accept the right envelope), but hand-canned the response substrate-side. The substrate's actual dispatch chain —
CommandExecutor->AuthPolicy.gate()->ServiceModule->AIProviderAdapter::generate_text()-> reply — was never exercised across airc.This PR exercises ALL of it. peer_a runs the FULL stack. No canned responses.
Test cases
end_to_end_heuristic_dispatch_through_substrate_stack— happy path.HeuristicInferenceAdapteranswers; assertresponse.text.starts_with("[heuristic:")(signature prefix no canned stub can produce) AND contains the prompt ANDresponse.provider == "airc-remote". Proves a real adapter ran end-to-end.end_to_end_peer_adapter_failure_surfaces_as_typed_error— error path.AlwaysFailingAdapterreturns Err; substrate wraps asAircCommandResponse::error; caller's transport classifies asRemoteInferenceError::PeerAdapterFailed. Closes task Implement Shared Pin Feature for Team Memory and RAG Integration #218 (deferred from PR feat(ai): AircRemoteInferenceAdapter — cross-grid inference as first-class AIProviderAdapter (task #108 slice C) #1560 R3-N3 review).end_to_end_missing_module_returns_typed_error— observability test. peer_a's executor has NO module forai/generate. Surfaces what the substrate CURRENTLY does: falls through to the legacy TS bridge at/tmp/jtag-command-router.sock(returns the socket connect error). The test pins this behavior so any future change is loud. Filed task Implement Shared Pin Feature for Collaborative Team Memory #219 to fix the substrate-side fall-through (it's a[[no-fallbacks-ever]]violation inCommandExecutorthat should hard-error on missing module in headless-Rust deployments).Design decisions documented in the test file
TestInferenceModuleinstead ofAIProviderModule: the latter uses a process-globalAdapterRegistrythat fights with multi-test parallelism. The substrate's wire path is module-agnostic, so anyServiceModuleregistered forai/generateexercises the same dispatch chain.TestInferenceModulewraps anArc<dyn AIProviderAdapter>directly; each test owns its own adapter instance.Arc<TranscriptEvent>at the on_envelope boundary: airc-lib'ssubscribe()yields broadcast-shapeArc<TranscriptEvent>for fan-out.ConsumerAdapter::on_envelopetakes owned value. A productized dispatch loop does the same.What's closed by this PR
What this PR surfaces (filed as follow-ups, not addressed here)
CommandExecutorfalls through to TS bridge for missing Rust modules.[[no-fallbacks-ever]]violation worth a dedicated slice. Pinned by test Build(deps): Bump actions/stale from 8 to 9 #3 so the fix lands loudly.Verified
Net diff: +463 lines new test file. Zero production-code changes — the proof IS the artifact.
Generated with Claude Code