Add an end-to-end test that exercises both event routers

[snipsnipsnip]

It's hardly a unit test, but I think it will be useful.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Tests

  • Added comprehensive test suite for Ghostbird background integration, validating end-to-end communication between background and compose components.
  • Enhanced test configuration with expanded TypeScript library targets.

• Chores

  • Updated development dependencies: upgraded jsdom and added TypeScript type definitions.

Walkthrough

Adds a promisified MessagePort utility, upgrades test-related devDependencies and TS lib targets, and introduces Vitest integration tests and minor test type-cast fixes for Thunderbird/Ghostbird messaging scenarios.

Changes

Cohort / File(s)	Summary
Dependency & Test config package.json, src/test/tsconfig.json	Upgraded jsdom to ^27.0.1 and added @types/jsdom ^27.0.0; added lib entries ["DOM","DOM.Iterable","ES2024","ESNext.Collection"] to src/test/tsconfig.json.
Promisified MessagePort utility src/thunderbird/util/promisified_message_port.ts	New PromisifiedMessagePort class and promisifyMessageChannel function providing a promise/queue-based wrapper around MessagePort with lifecycle handling, send/dequeue API, and close semantics.
Ghostbird integration test src/test/ghostbird.test.ts	New Vitest test exercising background ↔ compose message flow using mocked Thunderbirds, registries, WebSocket channels, and asserting state transfer and compose body updates.
Test typing adjustment src/test/sanity.test.ts	Added explicit casts for startupCompose arguments to concrete DOM types (HTMLElement, DOMParser, Selection).

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant PMP as PromisifiedMessagePort
    participant MP as MessagePort
    participant Q as PromisifyingQueue

    App->>PMP: send(msg)
    PMP->>MP: postMessage(msg)
    MP-->>PMP: onmessage(data)
    PMP->>Q: push(data)
    App->>PMP: dequeueReceived()
    PMP->>Q: dequeue()
    Q-->>App: TResponse
    App->>PMP: close()
    PMP->>MP: postMessage(null)
    MP-->>PMP: onmessage(null)
    PMP->>PMP: handleClose()
    PMP->>Q: notifyError()
    PMP->>MP: close()

Loading

sequenceDiagram
    participant Test as Ghostbird Test
    participant BG as Background Router
    participant Compose as Compose Router
    participant Channel as MessageChannel

    Test->>Channel: create()
    Test->>BG: init with mocked injector + channel.port1
    Test->>Compose: init with mocked injector + channel.port2
    Test->>Compose: send EditorChangeResponse(initial text)
    Compose->>Channel: forward message
    Channel-->>BG: receive message
    BG->>Test: record server snapshot
    BG->>Compose: send updated content
    Compose->>Test: assert body content updated

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 I wrapped a port in threads of light,
Queues hop messages through day and night,
Tests mock thunderbirds taking flight,
Ghostbird hums and types just right,
A tiny rabbit cheers the new delight.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description is substantially incomplete compared to the required template. The author provided only a single sentence ("It's hardly a unit test, but I think it will be useful.") and omitted nearly all required template sections, including the license agreement checkbox, the "@coderabbitai" tag, "What is the current behavior?", "What is the new behavior?", manual testing steps, and the "Anything else?" section. While test-focused PRs may not require all template sections to be equally applicable, the description fails to provide meaningful context or adhere to the repository's contribution standards.
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Add a smoke test that exercises both event routers" clearly and accurately describes the primary change in the pull request. The main addition is the new ghostbird.test.ts file, which implements an end-to-end test validating coordination between background and compose routers. The title is concise, specific, and appropriately captures the developer's primary intent without unnecessary detail. While the changeset includes supporting infrastructure (the promisified message port utility, dependency updates, and test configuration changes), the title correctly prioritizes the primary motivation—adding the smoke test itself.

✨ Finishing touches

• 📝 Docstrings were successfully generated.

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 92.30769% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.85%. Comparing base (4884a84) to head (a889566).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Components	Coverage Δ
app	64.20% <ø> (+31.73%)	⬆️
ghosttext	93.91% <ø> (+44.73%)	⬆️
infra	47.13% <92.30%> (+3.13%)	⬆️

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/test/ghostbird.test.ts (1)

63-63: Consider simplifying the promise chain.

The .then(() => {}) is redundant as Promise.all already returns a promise that resolves to undefined when both promises complete.

Apply this diff:

-  let closePromise = Promise.all([backgroundPromise, composePromise]).then(() => {})
+  let closePromise = Promise.all([backgroundPromise, composePromise])

src/thunderbird/util/promisified_message_port.ts (1)

66-68: Consider adding an explicit isOpen() check.

The IMessagePort interface documentation indicates that send() should throw "Already disconnected" when the port is closed. Currently, the method relies on the native MessagePort to throw, which may produce less descriptive error messages.

Apply this diff to provide clearer error messages:

 send(msg: TRequest): void {
+  if (!this.isOpen()) {
+    throw new Error("Already disconnected")
+  }
   this.port.postMessage(msg)
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4884a84 and 1af0a54.

⛔ Files ignored due to path filters (1)

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (5)

• package.json (1 hunks)
• src/test/ghostbird.test.ts (1 hunks)
• src/test/sanity.test.ts (1 hunks)
• src/test/tsconfig.json (1 hunks)
• src/thunderbird/util/promisified_message_port.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/test/ghostbird.test.ts (12)

src/ghosttext-runner/api.ts (1)

• IHeart (11-17)

src/app-background/api.ts (3)

• IButtonMenu (52-62)
• IComposeWindowDetector (40-49)
• IUiUtil (18-21)

src/ghosttext-adaptor/api.ts (8)

• IManifestInfo (75-78)
• INotificationTray (5-8)
• IStoredOptionsLoader (89-92)
• IWebClient (11-24)
• BackgroundMessage (95-95)
• BodyState (98-98)
• ComposeDetails (27-35)
• StoredOptions (81-86)

src/thunderbird/util/promisified_message_port.ts (1)

• promisifyMessageChannel (9-14)

src/app-compose/compose_event_router.ts (1)

• ComposeEventRouter (6-37)

src/app-background/background_event_router.ts (1)

• BackgroundEventRouter (6-56)

src/ghosttext-session/types.ts (2)

• EditorChangeResponse (50-55)
• ServerInitialResponse (12-17)

src/ghosttext-runner/message.ts (1)

• IMessagePort (25-60)

src/root/util/wireless.ts (2)

• WirelessInjector (33-36)
• wireless (45-57)

src/root/startup/startup_background.ts (1)

• BackgroundCatalog (23-27)

src/root/util/registry.ts (1)

• makeRegistry (5-7)

src/root/startup/startup_compose.ts (1)

• ComposeCatalog (15-19)

src/thunderbird/util/promisified_message_port.ts (2)

src/ghosttext-runner/message.ts (1)

• IMessagePort (25-60)

src/util/promisifying_queue.ts (1)

• PromisifyingQueue (5-100)

🔇 Additional comments (3)

src/test/tsconfig.json (1)

7-14: LGTM!

The addition of DOM and modern ES library targets appropriately supports the test infrastructure that uses MessageChannel, DOMParser, and other DOM APIs introduced in this PR.

src/test/sanity.test.ts (1)

33-35: LGTM!

The type casts appropriately align mock values with the expected DOM types, improving type safety without changing test behavior.

package.json (1)

75-75: Verify jsdom v27 compatibility by testing these key breaking changes.

The upgrade from jsdom v26 to v27 introduces several breaking changes that require test verification:

• Node.js minimum is now v20 — verify your CI/dev environment meets this requirement.
• VirtualConsole API: sendTo() renamed to forwardTo(); jsdomErrors option replaces omitJSDOMErrors boolean — search your test code for these usages.
• User-agent stylesheet switched from Chromium-derived to the HTML Standard — getComputedStyle() results may change — re-run CSS/layout-related tests.
• Cookie behavior: localhost may now be treated as a secure context — verify localhost cookie expectations in tests.
• MouseEvent now exposes movementX and movementY; new event constructors added — check for related event tests.

Re-run your test suite to confirm no failures from these changes.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/thunderbird/util/promisified_message_port.ts (1)

44-52: Consider type annotation precision for the disconnect sentinel.

The MessageEvent<TResponse> type annotation doesn't reflect that null can also arrive through this channel (sent by close() at line 72). While the runtime handling is correct—the == null check appropriately catches the disconnect sentinel—the type annotation could be more precise.

Consider this refinement for type accuracy:

-  this.port.onmessage = ({ data }: MessageEvent<TResponse>): void => {
+  this.port.onmessage = ({ data }: MessageEvent<TResponse | null>): void => {
     if (data == null) {

Note: The current use of == (rather than ===) correctly handles both null and undefined, which is appropriate for the disconnect signal.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1af0a54 and a889566.

📒 Files selected for processing (4)

• src/test/ghostbird.test.ts (1 hunks)
• src/test/sanity.test.ts (1 hunks)
• src/test/tsconfig.json (1 hunks)
• src/thunderbird/util/promisified_message_port.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• src/test/tsconfig.json
• src/test/sanity.test.ts
• src/test/ghostbird.test.ts

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-24T17:53:47.113Z

Learnt from: snipsnipsnip
PR: exteditor/ghostbird#49
File: src/thunderbird/util/promisified_message_port.ts:70-74
Timestamp: 2025-10-24T17:53:47.113Z
Learning: In JavaScript, MessagePort.postMessage() does not throw exceptions when called on a closed channel. Instead, it silently drops the message. Therefore, try-catch blocks are not needed when calling postMessage() on potentially closed ports.

Applied to files:

• src/thunderbird/util/promisified_message_port.ts

🧬 Code graph analysis (1)

src/thunderbird/util/promisified_message_port.ts (2)

src/ghosttext-runner/message.ts (1)

• IMessagePort (25-60)

src/util/promisifying_queue.ts (1)

• PromisifyingQueue (5-100)

🔇 Additional comments (5)

src/thunderbird/util/promisified_message_port.ts (5)

1-14: LGTM! Clean factory function with proper type constraints.

The promisifyMessageChannel function correctly wraps a MessageChannel's ports with swapped type parameters to enable bidirectional, type-safe communication.

---

16-38: LGTM! Well-structured factory pattern.

The static listenTo factory method combined with a private constructor ensures proper initialization while maintaining good encapsulation. The type constraint extends object | string correctly prevents null or undefined from being valid message types, which is essential since null is used as the disconnect sentinel.

---

54-64: LGTM! Clean delegation to the queue.

The promise-based methods correctly delegate to PromisifyingQueue, maintaining separation of concerns.

---

66-68: LGTM! Correct implementation.

The send method correctly posts messages to the underlying port. No error handling is needed since MessagePort.postMessage() silently drops messages on closed channels rather than throwing exceptions.

Based on learnings.

---

70-79: Close sequence is correct per previous feedback.

The close implementation properly signals disconnect via postMessage(null) and performs local cleanup. As noted in the retrieved learnings, MessagePort.postMessage() won't throw on closed channels, so the current implementation is safe even during simultaneous closes from both sides.

Optional: The console.info on line 71 may be intended for debugging. Consider whether structured logging would be preferable for production code, and whether the error message on line 78 could be more descriptive (e.g., "MessagePort disconnected").

Based on learnings.

[coderabbitai]

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #50