Skip to content

test: run a structured chat against the packaged headless server - #26527

Merged
brennanb2025 merged 8 commits into
mainfrom
brennanb2025/ssh-chat-packaged-server-e2e
Oct 12, 2026
Merged

brennanb2025 merged 8 commits into
mainfrom
brennanb2025/ssh-chat-packaged-server-e2e

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​428 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​425
Prod 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​96 $\color{#cf222e}{\Huge{\mathbf{−}}}$​7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​89

Stacked on #26450; retarget to main after it merges.

ELI5

SSH Claude chats failed before their first reply because the packaged plain-Node server broke the SDK's module URL. Tests running against application source did not catch that packaging failure. This adds a chat test against a server materialized from the built desktop template, so the same startup failure is caught before users receive a build. There is no new user interface or production behavior in this PR.

What Changed

The test obtains its server slot through the desktop's existing template materializer, starts the real packaged server with pinned Node and isolated configuration, and creates, subscribes to and sends a Claude chat through the real encrypted RPC. It checks a partial reply, the complete reply, a successful completed turn and the provider's launch arguments/environment.

Linux CI builds its host-target template before this test, on eligible PR runs as well as requested template builds. A missing required template or invalid template checksum fails the check. The fixture waits for the RPC subscriber to acknowledge the first partial reply before emitting later frames, with a bounded deadline.

Why

A direct server-slot test can pass even when the template needed for desktop SSH deployment is missing. Using the existing materializer covers the real template-to-slot boundary without creating another packaging implementation. Waiting for an observed partial reply removes the scheduling assumption behind the former 150 ms pause.

Shipping hourly/daily/dev desktop builds must also require and verify the template: that packaging guard belongs to #26451. This PR tests the built template and materialized server; it does not extract a shipping desktop artifact or independently close missing-template shipping failures.

Linked Issue

Related: #26450 (server bundle fix), #26451 (shipping-template requirement), #26464 (packaged host-CLI follow-up).

Visual Proof

N/A — automated headless test coverage; no interface or interaction change.

Testing

  • Automated tests added/updated.
  • Fails without fix: let managed SSH Claude chats load the agent SDK #26450: on Linux ARM the template-connected packaged chat test passes. Against fix: let managed SSH Claude chats load the agent SDK #26450's parent it fails at session creation, with createRequire receiving undefined.
  • Failure probes: a missing template and an invalid checksum fail as intended. A missing acknowledgement stops the fixture before later frames are sent.
  • After the final sync (head 9e18690fa29, merged with fix: let managed SSH Claude chats load the agent SDK #26450, which now contains current main):
    • Typecheck: node typecheck clean.
    • Tests: 156 change-scope, compiler-cache and qualification tests pass.
    • Repo checks: orca-ci-checks passes 24/24.
    • Local packaged run: on macOS arm64 it passed 1/1 against a freshly built server and template.
  • How CI selects this test: it needs the built desktop server template, so it's listed under a new template option in config/scripts/node-server-test-paths.mjs.
    • The change-scope check includes that list, so changing the test or anything it imports triggers the server-test workflow.
    • The default --artifact runs leave it out. Those are macOS, Windows, the glibc-floor and musl jobs, and Linux before the template is built, and none has a template.
    • The workflow's Linux steps run the test by path after building the template.
    • A workflow test fails if the test is moved into the default list.
  • CI:
    • PR checks: 13 passed, 18 skipped, 0 failed.
    • The server-test workflow, dispatched on this head (run 38054070152), passed every job. Its logs show this test ran and passed on Linux x64 and Linux ARM64: "creates, streams and settles a Claude turn through the real paired RPC".
  • Not verified: Windows, WSL and macOS chat in CI, a real SSH deploy or reconnect, and a real provider.
  • CI cost: any change under src/main/runtime/ or src/main/claude/ now forces the server-test workflow, so regressions in chat startup on the packaged server are caught.

Review

The test uses the execution host's journal and cursor-based read surface, matching that part of the common pattern.

  • Intended difference: this is a credential-free local packaged-server conformance test. It exercises authenticated host RPC without an actual SSH transport, keeping the regression short and deterministic.
  • Intended difference: Orca's existing detached managed-server lifetime remains unchanged; this test adds no disconnect or service-manager policy.
  • Temporary gap: this test does not run the packaged orca command from the agent process. Fix CLI delivery and fresh launch environments for managed SSH agents #26464's live SSH check covers that by hand (the agent's own orca status reached its server), but no automated test does yet.
  • Shipping dependency: fix: ship managed SSH server in hourly and daily builds #26451 supplies the desktop-artifact packaging guard; L10 verifies the resulting built-template path.

Agent skill upstream boundary

  • Not applicable; no upstream skill source or supporting artifacts copied.

Checklist

  • Small and focused; problem, user-facing implication and mechanism described.
  • No new UI; visual proof is N/A.
  • Self-reviewed and cross-platform/SSH boundaries considered; no production wire change.
  • Queued node typecheck passed on head 391c9d5; final-head CI classification follows in the review summary.

…mplate

The chat check loaded the server bundle directly, so a desktop build missing its
server template still passed. Materialize the slot from the built template the way
the desktop does, and replace the fixed streaming pause with a test-owned
acknowledgement so the partial-reply assertion can't race.
…-orcad-esm-import-meta

# Conflicts:
#	config/scripts/build-orcad.mjs
…import-meta' into brennanb2025/ssh-chat-packaged-server-e2e

# Conflicts:
#	config/scripts/node-server-change-scope.mjs
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review summary (final head 9e18690fa29)

What this PR adds. An automated test for the bug #26450 fixes. Claude chats could not start on a managed server because the bundled agent SDK read an undefined module path. The test builds the real desktop server template, starts the packaged server, and drives a Claude chat through the real paired RPC: create, stream, and settle a turn. It uses a stand-in Claude, so no credentials are needed. It fails without #26450.

Reviews. There were two full rounds. The last one, on 391c9d5b4cf, found nothing. It confirmed the test fails against #26450's parent and that the exact-head CI run passed.

Final sync. One merge of #26450's branch, which now contains current main.

  • Conflict: one, in the server-test change-scope rule. Both intents are kept: main no longer lets a changed unit test force the workflow, and this PR forces it when an orcad build script changes.
  • Import fix: one import moved to @orca/process-host, after main deleted the old child-process module.
  • Test selection: the test needs the built template, so it sits on its own template list. Default runs, which have no template, don't pick it up; the workflow runs it by path after building the template. The change-scope check includes the list, and a workflow test guards it.

I read this round's PR-only diff against its base myself.

Checks

  • PR checks: 13 passed, 18 skipped, 0 failed. MERGEABLE.
  • Server-test workflow dispatched on this head (run 38054070152): every job passed. Logs show this test ran and passed on Linux x64 and Linux ARM64.
  • Local: node typecheck clean, 156 script tests, orca-ci-checks 24/24, packaged run 1/1 on macOS arm64.

Notes

@brennanb2025
brennanb2025 marked this pull request as ready for review October 10, 2026 13:18
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/ssh-chat-orcad-esm-import-meta to main October 11, 2026 21:35
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c50e292-7cc6-439c-88ee-f09b758c06ef

📥 Commits

Reviewing files that changed from the base of the PR and between 8864f4e and c2d3c67.


📒 Files selected for processing (1)
  • src/main/orcad/orcad-packaged-structured-chat.integration.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.



📝 Walkthrough

Walkthrough

The change adds a packaged structured-chat integration test for a materialized Orcad server. A scripted Claude CLI and scenario provide a two-part reply and completion events. Node-server test selection now includes the test for template paths, and Linux CI builds a host-architecture desktop template before running it. A native-chat test fixture is reformatted without changing its values.


Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to c2d3c

No actionable merge-blocking issue is identified. The reported validation covers Linux and macOS, while Windows/WSL and live-provider or deployment behavior remain outside this test’s scope.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 26.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 14 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly identifies the main change: adding a structured-chat test against the packaged headless server.
Description check Passed The description covers the required sections, explains the change and rationale, documents testing and limitations, marks visual proof as not applicable, and includes checklist status. It links relate…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…-packaged-server-e2e

# Conflicts:
#	.github/workflows/node-server-tests.yml
Main now sends remote runtime requests over a paired connection rather than a
pairing offer, so the test converts the server's offer once when it is ready.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Moved onto main after #26450 merged (head c2d3c6748ec).

  • Base: retargeted from fix: let managed SSH Claude chats load the agent SDK #26450's branch to main, then merged main in.
  • Conflict: one comment line in .github/workflows/node-server-tests.yml. main reworded it; the PR's two steps (build the template, run the packaged chat test) are kept above it.
  • Line limit: with main's additions, config/scripts/node-server-change-scope.test.mjs would reach 630 lines (limit 600). This PR's selection tests moved to their own file, config/scripts/node-server-packaged-chat-selection.test.mjs, with no behaviour change. The lint fails on the old merged file and passes on the split one.
  • Test updated for main's connection change: main (refactor(remote): clients connect through a paired-connection handle, not a pairing offer #27533) now sends remote runtime requests over a paired connection rather than a pairing offer. The packaged chat test still passed the offer, and failed on Linux CI with Cannot read properties of undefined (reading 'deviceToken'). It now converts the server's offer once with pairedRuntimeConnectionFromOffer.
    • Rebuilt locally, it passes 1/1 on macOS arm64.
    • The typecheck didn't catch this because *.integration.test.ts files aren't part of the typechecked projects.
  • One unrelated formatting change: the commit hook reformatted one long line in src/main/native-chat/subagent-tracker/subagent-tracker.test.ts (main has it unformatted). It's formatting only, and the hook won't let it be reverted.
  • CI on c2d3c6748ec: 26 passed, 11 skipped, 0 failed. That includes persistence (ubuntu-22.04) and persistence (ubuntu-24.04-arm), which build the template and run the packaged chat test.

@brennanb2025
brennanb2025 merged commit c9097ac into main Oct 12, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant