Skip to content

fix(daemon): roll the daemon-side fixes since v1.4.241 into a fresh v53 daemon - #34042

Closed
Jinwoo-H wants to merge 1 commit into
mainfrom
fix/daemon-protocol-v53
Closed

Jinwoo-H wants to merge 1 commit into
mainfrom
fix/daemon-protocol-v53

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 0
Prod 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1

ELI5

The terminal daemon outlives app updates, and the app reuses a running daemon with the same protocol number. v1.4.241 shipped v52 and main is still v52, so users upgrading to the next release would keep v1.4.241's daemon and miss the daemon-side fixes merged since. One of them is a contract change: #33668 renamed the env var the daemon's shell wrapper restores for MiMo (ORCA_MIMOCODE_HOME → ORCA_MIMOCODE_CONFIG_DIR), so a new app on a reused v52 daemon would leave MiMo's login fix inert. This bumps to v53 so new terminals start on a fresh daemon; existing v52 terminals stay attachable.

What Changed

  • PROTOCOL_VERSION 52 → 53, with 52 added to PREVIOUS_DAEMON_PROTOCOL_VERSIONS.
  • local-build-compatibility-contract.json / .ts: daemonProtocolVersion: 53, 53 attachable.
  • daemon-foreground-confirmation-protocol.test.ts pins the new number.

No wire format change. v53 has not been used by any build.

Why

A tree-shaken bundle diff of daemon-entry.ts between v1.4.241 and main (22fd6f5) shows daemon runtime changes from #33668 (MiMo config env), #18182 (PowerShell OSC 133 C/D startup marks), #33463 (Alt+Left/Right PSReadLine binding), #33898 (ps locale pinned on macOS), #23251 (?? tty), #29762 (RLIMIT_RTTIME warning), #30996, #33671 and the agent permission tables. #33837's --max-semi-space-size flag also only applies to a freshly launched daemon.

Testing

  • 10 daemon/shared test files referencing the protocol: 117 passed.
  • oxfmt clean.

Visual Proof

N/A, no UI change.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 954588e6-9c8d-4ffc-b45d-0fefd3c3e49d
Commits

Reviewing files that changed from the base of the PR and between ae55694 and 68e8f83.

Files selected for processing: 4
  • src/main/daemon/daemon-foreground-confirmation-protocol.test.ts
  • src/main/daemon/daemon-protocol-version.ts
  • src/shared/local-build-compatibility-contract.json
  • src/shared/local-build-compatibility-contract.ts

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

Walkthrough

The daemon protocol version changes from 52 to 53. The previous-version list adds 52. The TypeScript and JSON local build compatibility contracts set the current version to 53 and include 53 in the attachable versions. The test expectations are updated to check version 53 and previous version 52.

Priority: Normal

Merge Risk: Minimal · up to 68e8f

No actionable merge-blocking risk was identified in the protocol version update.

Pre-merge checks: Passed 4 · Failed 1

Failed 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main change: bumping the daemon protocol to v53 so a fresh daemon picks up daemon-side fixes since v1.4.241. It follows the conventional fix(scope) form and is one sentence. It is …
Description check Passed The description follows the template's main sections: ELI5, What Changed, Why, Testing, Visual Proof, and Notes. It explains the protocol bump, the before and after behavior, the mechanism, and the re…
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.
Full details: Title check

Explanation

The title names the main change: bumping the daemon protocol to v53 so a fresh daemon picks up daemon-side fixes since v1.4.241. It follows the conventional fix(scope) form and is one sentence. It is somewhat long at 78 characters, but it is specific and clear.

Full details: Description check

Explanation

The description follows the template's main sections: ELI5, What Changed, Why, Testing, Visual Proof, and Notes. It explains the protocol bump, the before and after behavior, the mechanism, and the reason for the change. It also reports test results and marks Visual Proof as N/A with a reason. The Linked Issue section is missing: it says 'Fixes #' with no issue number, and the template requires an issue for outside contributors. The manual-test checkbox and the 'Which platforms were tested' detail are also not addressed. The Review, Agent skill upstream boundary, and checklist sections are not filled in. These gaps are non-critical for a maintainer-authored change, so the check passes, but the author should complete them.

Resolution

Link an issue in the Linked Issue section, or confirm the author is a maintainer and no issue is needed. Also state which platforms were tested (macOS, Linux, Windows, or SSH), and tick the manual-testing checkbox only if it was done. Complete or remove the unused template sections, including the checklist.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • 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.

@Jinwoo-H

Copy link
Copy Markdown
Contributor Author

Not needed for v1.4.242: no wire break, and the daemon-side changes since v1.4.241 are minor enough to reach users when the v52 daemon next restarts. Skipping the bump avoids running two daemons side by side.

@Jinwoo-H Jinwoo-H closed this Oct 26, 2026
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