Skip to content

Phase 3: Testing wiring (CLI process, ACP mock-runtime, hooks) - #37

Merged
soorya-u merged 2 commits into
mainfrom
phase-3-testing-wiring
Jul 11, 2026
Merged

soorya-u merged 2 commits into
mainfrom
phase-3-testing-wiring

Conversation

@soorya-u

@soorya-u soorya-u commented Jul 11, 2026 •

Copy link
Copy Markdown
Owner

Implements #32

Summary

  • Add ACP mock prompt runtime for CLI integration tests
  • Add CLI integration coverage for login gate and mock-driven runTurn
  • Add hooks cache integration test for optimistic user messages
  • Add mock data-channel helpers for PR WebRTC paths

Test plan

  • bun test:unit
  • bun test:integration
  • bun check:types

Stacked on #36

Closes #32
Part of #29

Summary by CodeRabbit

  • Tests

    • Added CLI integration coverage for mocked prompt flows and startup failures.
    • Added hook tests covering optimistic conversation updates.
    • Added mock data-channel utilities and tests for connection state events.
  • Documentation

    • Updated testing guidance with Phase 3 coverage and integration test practices.
  • Chores

    • Added dedicated unit and integration test commands for CLI and hooks packages.

@vercel

vercel Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cyrus Ready Ready Preview, Comment Jul 11, 2026 8:10am

@coderabbitai

coderabbitai Bot commented Jul 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@soorya-u, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ff72bcf-2d6d-4615-87c8-e73bdb6b7b4d

📥 Commits

Reviewing files that changed from the base of the PR and between 3eb4f21 and 31c3b85.

📒 Files selected for processing (3)
  • apps/cli/__tests__/integration/wiring.test.ts
  • tooling/test/mocks/README.md
  • tooling/test/mocks/data-channel.ts
📝 Walkthrough

Walkthrough

Adds Phase 3 testing infrastructure for ACP prompt streams, CLI process integration, optimistic hooks cache updates, and mocked RTC data channels, along with package scripts, Bun type configuration, and testing documentation updates.

Changes

Phase 3 testing

Layer / File(s) Summary
ACP and CLI integration wiring
apps/cli/__tests__/helpers/acp-runtime.ts, apps/cli/__tests__/integration/wiring.test.ts, apps/cli/package.json, docs/testing.md
Adds a mock ACP event generator, validates runTurn event handling, tests CLI login-gate process exit with an isolated CYRUS_HOME, and separates CLI unit and integration test commands.
Hooks cache test configuration
shared/hooks/package.json, shared/hooks/tsconfig.json, shared/hooks/src/connection/use-controller-threads.test.ts
Enables Bun testing and types, and verifies optimistic user-message insertion in the conversations query cache.
Mock data-channel fixture
tooling/test/mocks/data-channel.ts, tooling/test/mocks/README.md
Adds mock RTC data-channel state transitions and listener handling, with coverage for the open event and updated Phase 3 mock documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • soorya-u/cyrus issue 29 — Covers the same Phase 3 testing strategy areas, including ACP, CLI, hooks, and shared mocks.
  • soorya-u/cyrus issue 31 — Relates to CLI Bun testing seams, runTurn, and mock ACP integration.
  • soorya-u/cyrus issue 33 — Plans reuse of the CLI process tests and mock WebRTC infrastructure added here.

Possibly related PRs

  • soorya-u/cyrus#23 — Introduces the conversation and turn-cache APIs exercised by the new optimistic cache test.

Sequence Diagram(s)

sequenceDiagram
  participant WiringTest
  participant runTurn
  participant threadCoordinator
  participant MockPromptStream
  WiringTest->>runTurn: execute turn
  runTurn->>threadCoordinator: call prompt
  threadCoordinator->>MockPromptStream: request events
  MockPromptStream-->>threadCoordinator: token and message_completed
  threadCoordinator-->>runTurn: return prompt result
  runTurn-->>WiringTest: Ok result and event sequence
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main testing wiring changes across CLI, ACP, and hooks.
Linked Issues check ✅ Passed The PR adds the requested ACP mock runtime, CLI login-gate integration, hooks optimistic-cache test, and mock data-channel helpers.
Out of Scope Changes check ✅ Passed The added scripts, docs, and type config changes support the testing work and don't appear unrelated.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch phase-3-testing-wiring

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.

@soorya-u

Copy link
Copy Markdown
Owner Author

Tracks sub-issue #32 (epic #29).

@soorya-u
soorya-u force-pushed the phase-2-testing-seams branch from a7aed5f to 83d89a6 Compare July 11, 2026 06:36
Base automatically changed from phase-2-testing-seams to main July 11, 2026 07:56
@soorya-u soorya-u linked an issue Jul 11, 2026 that may be closed by this pull request
15 tasks
Introduce mock ACP prompt streams, CLI login-gate integration coverage, optimistic thread cache tests, and mock data-channel helpers for PR CI WebRTC paths.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
apps/cli/__tests__/integration/wiring.test.ts (1)

35-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer a typed partial mock over as never.

as never is a valid escape hatch but obscures intent. A Partial<Runtime> cast or a dedicated mock factory would make it clearer that only threadCoordinator.prompt is being stubbed, and would catch future contract changes at compile time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/__tests__/integration/wiring.test.ts` around lines 35 - 39, Replace
the `as never` cast in the `runtime` mock with a typed partial mock, such as
`Partial<Runtime>`, or use a dedicated mock factory that explicitly stubs only
`threadCoordinator.prompt`. Preserve the existing `createMockPromptStream`
behavior while ensuring future `Runtime` contract changes are checked by the
type system.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/cli/__tests__/helpers/acp-runtime.ts`:
- Around line 9-26: Add a test in the existing runTurn wiring tests that invokes
createMockPromptStream with failAfterToken enabled, verifies the thrown runtime
error is handled, and asserts that the emitted event sequence includes
turn_interrupted rather than successful completion. Use the existing test
helpers and assertions in wiring.test.ts.

In `@apps/cli/__tests__/integration/wiring.test.ts`:
- Around line 64-65: In the child-process options for the integration test,
change both stdout and stderr from "pipe" to "ignore" so output is not buffered
in unread streams; preserve the existing exit-code assertions and other process
configuration.

In `@tooling/test/mocks/data-channel.ts`:
- Around line 1-59: Add the DOM library to the compiler configuration in
tooling/typescript/tsconfig.base.json so tooling/test/mocks/data-channel.ts
resolves RTCDataChannelState, Event, and EventListener while retaining the
existing Bun type configuration.

---

Nitpick comments:
In `@apps/cli/__tests__/integration/wiring.test.ts`:
- Around line 35-39: Replace the `as never` cast in the `runtime` mock with a
typed partial mock, such as `Partial<Runtime>`, or use a dedicated mock factory
that explicitly stubs only `threadCoordinator.prompt`. Preserve the existing
`createMockPromptStream` behavior while ensuring future `Runtime` contract
changes are checked by the type system.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d1b136f-3aa3-4f44-9ecd-097b30477945

📥 Commits

Reviewing files that changed from the base of the PR and between ff1d019 and 3eb4f21.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • apps/cli/__tests__/helpers/acp-runtime.ts
  • apps/cli/__tests__/integration/wiring.test.ts
  • apps/cli/package.json
  • docs/testing.md
  • shared/hooks/package.json
  • shared/hooks/src/connection/use-controller-threads.test.ts
  • shared/hooks/tsconfig.json
  • tooling/test/mocks/README.md
  • tooling/test/mocks/data-channel.ts

Comment thread apps/cli/__tests__/helpers/acp-runtime.ts
Comment thread apps/cli/__tests__/integration/wiring.test.ts Outdated
Comment thread tooling/test/mocks/data-channel.ts
Cover the mock interruption path, avoid hanging CLI spawn pipes, and
keep data-channel mocks free of shared DOM tsconfig changes.

Co-authored-by: Cursor <cursoragent@cursor.com>
@soorya-u

Copy link
Copy Markdown
Owner Author

Addressed CodeRabbit review in the latest push:

  • failAfterToken — added wiring test that asserts turn_interrupted when the mock stream fails
  • CLI spawn pipes — switched stdout/stderr to ignore so unread pipes can't hang the process
  • data-channel DOM types — used local mock state/listener types instead of adding DOM to tsconfig.base.json (would pollute non-browser packages)

Skipped:

  • as never → typed partial mock nit — same escape hatch already used in Phase 2 runTurn unit tests; leaving consistent for now

@soorya-u
soorya-u merged commit 57d8af6 into main Jul 11, 2026
7 checks passed
@soorya-u
soorya-u deleted the phase-3-testing-wiring branch July 11, 2026 08:11

This branch was successfully deployed

1 active deployment
Preview — 31c3b853 Deployed Jul 11, 2026 by vercel[bot]
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.

Phase 3: Testing wiring (CLI process, hooks, ACP mock-runtime)

1 participant