Add a header to each console log

[snipsnipsnip]

Summary by CodeRabbit

• Documentation

  • Updated build documentation with expanded details on preprocessing and output differences.

• Refactoring

  • Restructured internal build tooling and test infrastructure for improved organization and maintainability.

• Refactoring

  • Enhanced console logging behavior differentiation between release and development builds.

Changes

Cohort / File(s)	Summary
Build Documentation doc/building.md	Updated build graph descriptions, replacing tsdown_builtin with separate replace_plugin and copy plugin, expanded asset handling documentation, and enhanced Output differences section with preprocessing and logging details
Command Handler Documentation src/app-background/command_handler.ts	Enhanced JSDoc for CommandHandler class and runCommand method to reflect command dispatch from UI sources (toolbar, context menu, shortcuts) with parameter and return value documentation
Test Infrastructure – Async Messaging src/test/ghostbird.test.ts	Refactored IPC/messaging from direct ports to promise-based handshake with serverSocket Promise and task tracking; updated receives(), startFromToolbar(), sendsToServer(), and closeSession() to use async flows
Test Utilities src/test/startup.test.ts	Added assertion validating equality between constructor arguments and dependencies in checkDependencyUsage
Build Tool Plugin Lifecycle Refactoring tools/test_sanity.ts tools/typecheck_with_tsc.ts	Converted plugin factories to shift async completion from buildStart (now void) to new writeBundle lifecycle hook, using Promise.withResolvers for deferred resolution
Build Tool Utility Extraction tools/barrelsby.ts	Extracted runBarrelsby helper function to encapsulate barrelsby logic, improving testability by accepting a logger parameter instead of relying on PluginContext
Build Tool Configuration tools/generate_manifest.ts tools/tsdown_config.ts	Strengthened Options type to require env field; updated generateManifest signature to accept Options object; added replacePlugin for conditional console log rewriting in release mode; adjusted manifest generation invocation

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Documentation

  • Updated build documentation with expanded details on preprocessing and output differences.

• Refactoring

  • Restructured internal build tooling and test infrastructure for improved organization and maintainability.
  • Enhanced console logging behavior differentiation between release and development builds.

Walkthrough

This pull request updates build documentation, refactors test infrastructure to use promise-based async messaging, improves JSDoc for the CommandHandler class, extracts build tool utilities for testability, and shifts plugin lifecycle patterns from buildStart to writeBundle hooks for deferred async operations.

Changes

Cohort / File(s)	Summary
Build Documentation doc/building.md	Updated build graph descriptions, replacing tsdown_builtin with separate replace_plugin and copy plugin, expanded asset handling documentation, and enhanced Output differences section with preprocessing and logging details
Command Handler Documentation src/app-background/command_handler.ts	Enhanced JSDoc for CommandHandler class and runCommand method to reflect command dispatch from UI sources (toolbar, context menu, shortcuts) with parameter and return value documentation
Test Infrastructure – Async Messaging src/test/ghostbird.test.ts	Refactored IPC/messaging from direct ports to promise-based handshake with serverSocket Promise and task tracking; updated receives(), startFromToolbar(), sendsToServer(), and closeSession() to use async flows
Test Utilities src/test/startup.test.ts	Added assertion validating equality between constructor arguments and dependencies in checkDependencyUsage
Build Tool Plugin Lifecycle Refactoring tools/test_sanity.ts tools/typecheck_with_tsc.ts	Converted plugin factories to shift async completion from buildStart (now void) to new writeBundle lifecycle hook, using Promise.withResolvers for deferred resolution
Build Tool Utility Extraction tools/barrelsby.ts	Extracted runBarrelsby helper function to encapsulate barrelsby logic, improving testability by accepting a logger parameter instead of relying on PluginContext
Build Tool Configuration tools/generate_manifest.ts tools/tsdown_config.ts	Strengthened Options type to require env field; updated generateManifest signature to accept Options object; added replacePlugin for conditional console log rewriting in release mode; adjusted manifest generation invocation

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test Code
    participant GB as Ghostbird
    participant Port as serverSocket
    participant BG as Background Task

    Test->>GB: receives(text)
    activate GB
    GB->>GB: resolve serverSocket
    activate Port
    Port->>BG: register task
    GB->>Port: send editor change payload
    Port-->>GB: payload delivered
    deactivate Port
    GB-->>Test: Promise<void>
    deactivate GB
    
    Note over GB,BG: Async handshake enables<br/>concurrent operations

Loading

sequenceDiagram
    participant Build as Build System
    participant Plugin as Plugin (test_sanity/<br/>typecheck_with_tsc)
    participant Hook1 as buildStart()
    participant Hook2 as writeBundle()
    participant Resolver as Promise Resolver

    Build->>Plugin: initialize plugin
    Build->>Hook1: buildStart()
    activate Hook1
    Hook1->>Hook1: spawn task (sync)
    Hook1->>Resolver: register resolver
    Hook1-->>Build: void (immediate)
    deactivate Hook1
    
    Note over Hook1: Task continues<br/>asynchronously
    
    Build->>Hook2: writeBundle()
    activate Hook2
    Hook2->>Resolver: await resolution
    Resolver-->>Hook2: task complete
    Hook2-->>Build: Promise resolves
    deactivate Hook2

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Extra attention areas:
  • src/test/ghostbird.test.ts — Substantial async refactoring with promise-based messaging, task tracking, and multiple method signature changes; requires careful verification of async control flow and task cleanup
  • tools/test_sanity.ts & tools/typecheck_with_tsc.ts — Parallel plugin lifecycle refactoring pattern (buildStart → void + writeBundle hook); verify Promise.withResolvers usage and resolver state management across both files
  • tools/generate_manifest.ts — Type strengthening and parameter propagation changes; verify all call sites correctly adapt to the new Options object structure
  • tools/barrelsby.ts — New exported function and logger decoupling; ensure testability improvements don't introduce logging inconsistencies

Possibly related PRs

• Add an end-to-end test that exercises both event routers #49 — Modifies src/test/ghostbird.test.ts to implement promise-based messaging handshake with similar IPC refactoring patterns
• chore: bump tools version and update the webpage #47 — Alters tools/tsdown_config.ts plugin configuration and isRelease conditionals for bundler setup
• refactor: bundled the test initialization to a utility class #51 — Updates src/test/ghostbird.test.ts scaffolding with method signature changes (receives, expectServerReceives) to the Ghostbird test helper API

Poem

🐰 A rabbit hops through promiseful code,
Where ports await their async road,
buildStart yields, writeBundle takes the call,
And plugins dance through lifecycle's hall—
Refactored clean, from test to tool,
This PR hops by async rule! 🎯

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description is entirely empty with no content provided by the author. The required template specifies several sections: agreement to licensing, description of current behavior, new behavior, manual testing steps, and any additional notes. None of these sections have been filled out, making it impossible for reviewers to understand the rationale for the changes, the problem being solved, or how to verify the implementation works correctly. This omission significantly impacts the reviewability and traceability of the changeset.
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% 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 PR title "Add a header to each console log" directly corresponds to a real and significant change present in the changeset. The tools/tsdown_config.ts file introduces a replacePlugin that adds headers to console outputs (console.log, console.debug, console.info, console.warn, and console.error) in a configurable manner. The title is specific and clear enough to convey what was implemented, allowing a teammate scanning the history to understand this change without ambiguity. While the PR includes additional supporting changes across multiple files (documentation updates, JSDoc improvements, test refactoring, and plugin factory changes), the title appropriately focuses on the primary feature being added rather than attempting to enumerate all modifications.

✨ Finishing touches

• 📝 Generate docstrings

🧪 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 0% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.22%. Comparing base (eb50a17) to head (44be1ec).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

Components	Coverage Δ
app	64.20% <ø> (ø)
ghosttext	93.91% <ø> (ø)
infra	47.21% <ø> (ø)

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

doc/building.md (1)

52-56: Fix list indentation (markdownlint MD007)

Indent this sub-item by 2 spaces instead of 4.

-    - Preprocessing source files.
+  - Preprocessing source files.

🧹 Nitpick comments (6)

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

96-99: Avoid brittle ctor length equality; handle default/optional/rest params

Function length excludes params with defaults and anything after the first default, so ctor.length === deps.length can fail even when DI is correct. Prefer parsing the constructor parameter list or assert a weaker invariant.

Example safer option:

-    expect(ctor).to.have.lengthOf(deps.length)
+    // Ensure at least the required arity covers the listed deps
+    expect(ctor.length).to.be.at.least(deps.length)

Or parse param list:

const paramList = /\bconstructor\s*\(([^)]*)\)/.exec(String(ctor))?.[1] ?? "";
const params = paramList.split(",").map(s => s.trim()).filter(Boolean);
expect(params.length).to.equal(deps.length);

Please confirm whether any classes declare defaults/optionals in constructors; if yes, the strict equality is unsafe.

doc/building.md (2)

79-85: Document the new “log header” behavior

PR title suggests adding a header to console logs. The “Log output” section mentions removal of console.log in release, but not the header in development. Please clarify behavior.

Suggested addition:

 #### Log output

-- In a release build, `console.log()` are removed.
-- In a development build, `console.log()` are left in place.
+– In a release build, `console.log()` calls are removed.
+– In a development build, console calls are rewritten to include a header (e.g., `[Ghostbird]` or the build ID) via the replace plugin, to make logs searchable and attributable.
+
 Other log levels like `console.info` are not removed in either case.

Optionally link to the replace plugin config that injects the header for discoverability.

---

101-107: Clarify replace plugin configuration for logging

Since logging is modified via a built-in replacePlugin, add a brief note (pattern, header format, env gates) or a pointer to the exact config location, so readers can find and adjust it quickly.

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

200-205: Prefer dequeueReceived over clearReceived for deterministic reads.

waitReady ensures at least one message; dequeueReceived returns the earliest message without flushing the buffer, which is usually what tests want.

-    const received = s.clearReceived()
+    const received = s.dequeueReceived()

tools/generate_manifest.ts (1)

86-88: Don’t blindly drop the first char of git hash; strip the optional "g" prefix.

git-describe implementations sometimes prefix hashes with "g". Using slice(1) can corrupt hashes that don’t have it.

-  const abbrev = hash.slice(1)
+  const abbrev = hash.replace(/^g/, "")

tools/tsdown_config.ts (1)

74-79: Confirm intended behavior: only console.log is disabled in release, others are prefixed.

If the goal is “Add a header to each console log,” keeping info/warn/error active is fine, but double-check this aligns with release requirements.

If you also want to silence debug-level noise in release:

-          "console.debug(": 'console.debug("[Ghostbird]",',
+          "console.debug(": isRelease ? "(()=>{})(" : 'console.debug("[Ghostbird]",',

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb50a17 and 95ff57a.

📒 Files selected for processing (9)

• doc/building.md (5 hunks)
• src/app-background/command_handler.ts (1 hunks)
• src/test/ghostbird.test.ts (6 hunks)
• src/test/startup.test.ts (1 hunks)
• tools/barrelsby.ts (1 hunks)
• tools/generate_manifest.ts (3 hunks)
• tools/test_sanity.ts (1 hunks)
• tools/tsdown_config.ts (3 hunks)
• tools/typecheck_with_tsc.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

src/app-background/command_handler.ts (2)

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

• ComposeActionNotifier (5-58)

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

• IUiUtil (18-21)

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

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

• IMessagePort (25-60)

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

• promisifyMessageChannel (11-16)

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

• ComposeEventRouter (6-37)

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

• BackgroundEventRouter (6-56)

tools/tsdown_config.ts (1)

tools/generate_manifest.ts (1)

• generateManifest (11-38)

🪛 markdownlint-cli2 (0.18.1)

doc/building.md

52-52: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

🔇 Additional comments (6)

src/app-background/command_handler.ts (2)

5-5: Clarify the relationship between PR title and changes in this file.

The PR title indicates "Add a header to each console log", but this file contains no console log statements and only includes documentation improvements. While the JSDoc updates are valuable, there appears to be a mismatch between the stated PR objective and the actual changes in this file.

Can you confirm whether:

1. This file was included intentionally, or
2. Console log changes are present in other files not shown in this review?

Also applies to: 13-18

---

5-5: Documentation improvements look good.

The updated class documentation accurately describes the CommandHandler's role in dispatching commands from various UI sources. The added JSDoc for runCommand properly documents the parameters and return type, making the API clearer for consumers.

Also applies to: 13-18

tools/barrelsby.ts (1)

18-27: No action required — review comment is based on incorrect assumption

Barrelsby(args) in v2.8.1 is synchronous and returns void, not a Promise. The function executes and completes before rm() is called, so there is no race condition. The current code is correct as-is.

Likely an incorrect or invalid review comment.

tools/test_sanity.ts (1)

6-7: This review comment is incorrect.

Promise.withResolvers is available in Node.js starting with v22.0.0, but the codebase already uses this pattern in 7 files—including tools/typecheck_with_tsc.ts in the same directory. If Node 20 were a hard requirement, the existing code would already be failing. The absence of .nvmrc, .node-version, or package.json engines constraints suggests the project supports Node 22+ or does not enforce a specific lower bound. The code in tools/test_sanity.ts is consistent with the established pattern across the codebase and does not require a polyfill.

Likely an incorrect or invalid review comment.

tools/typecheck_with_tsc.ts (1)

6-7: Remove this review comment — conflates runtime and build tool contexts

This review comment is incorrect. Both tools/typecheck_with_tsc.ts and tools/test_sanity.ts are Rolldown build plugins that run in Node.js during the build process, not Thunderbird runtime code. The retrieved learning about Promise.withResolvers support applies only to extension runtime files in src/, not build tools. Additionally, the project has no documented Node 20 requirement in package.json (only Thunderbird >= 128). Promise.withResolvers is appropriate for build tools running on modern Node.js.

Likely an incorrect or invalid review comment.

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

155-157: This review comment's compatibility concern is not supported by evidence.

Promise.withResolvers is available in the Node environment, and the codebase already uses this pattern extensively—including in test files (src/util/lazy_then.ts:80 uses the identical pattern). The learnings confirm it's supported in the Thunderbird extension runtime and already established in multiple utility files. Suggesting a manual resolver fallback here contradicts the codebase's established conventions and is unnecessary.

Likely an incorrect or invalid review comment.