Skip to content

fix(updater): accept the supervised serve's other spelling of its profile path - #34570

Merged
nwparker merged 2 commits into
mainfrom
fix/serve-handoff-path-casing-34382
Oct 27, 2026
Merged

nwparker merged 2 commits into
mainfrom
fix/serve-handoff-path-casing-34382

Conversation

@nwparker

@nwparker nwparker commented Oct 27, 2026 •

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

ELI5

On a Mac, the same folder can be written Orca or orca. The background orca serve service names its profile folder one way and the app names it the other. So when an update was requested, the server decided it wasn't running under its supervisor and refused to update. Now it checks whether both names point at the same folder.

What Changed

orca serve started by the generated launchd service gets its update-handoff path from the CLI, which spells the profile ~/Library/Application Support/orca. Electron's own profile path is ~/Library/Application Support/Orca. getConfiguredHandoffPath() compared the two with path.resolve() strings, so a case-insensitive volume's two spellings of one directory read as different. The server reported manual-service-update-required and refused the update before installing anything.

It now requires the same handoff file name and compares the directories with the existing isSameUserDataPath (realpath, which returns the on-disk casing). The startup profile guard and the rest of the serve launch flow already compare profiles this way. A path to a different profile, or to a directory that doesn't exist, is still rejected. The handoff is still written to Electron's own spelling of the path, so it lands in the same file the supervisor reads.

This is @Eotel's fix from #34383, cherry-picked with their authorship kept. On top we added one test for the reported casing case; their test covers a symlink alias. This PR supersedes #34383. Thank you @Eotel for the report, the reproduction and the fix.

I also checked the other places this flow compares profile paths:

  • The service installer's launchd label (serve-service-unit.ts profileSuffix) still compares strings. That's deliberate: changing it would rename already-installed services, so uninstall would miss them and a reinstall could start a second supervisor. The reported case doesn't hit it, because both sides use the CLI's spelling there.
  • The startup guard, launch.ts, and the runtime source fence already use isSameUserDataPath.

Why

It reuses the profile-identity check the serve flow already relies on, instead of adding a second notion of "same profile".

Linked Issue

Fixes #34382

Visual Proof

N/A: no UI change. The fix is in the server's supervisor check, and the tests reproduce it.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • serve-update-handoff.test.ts: the alias test and the casing test (which runs on case-insensitive volumes) both fail on main and pass here.

  • Tests importing the changed module pass. The exception is serve-update-handoff.app-environment.test.ts, which times out on this machine identically on unmodified main.

  • pnpm tc and oxlint pass, and so does a local run of the PR static job. The Node 18 process-host smoke couldn't run because Node 18 isn't installed locally; this change doesn't touch process-host.

Review

Reviewed by Codex and an independent reviewer: no findings at P0–P2.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

macOS is where the bug shows; the handoff is only used with a macOS app bundle. Linux and Windows get the same identity check (Windows compares lowercased). No wire or compatibility changes.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 27, 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: 79690a34-8a35-4441-8750-697bcbf3d644
Commits

Reviewing files that changed from the base of the PR and between 917ef9c and f6e6ed8.

Files selected for processing: 2
  • src/main/serve-update-handoff.test.ts
  • src/main/serve-update-handoff.ts

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

Walkthrough

The handoff path comparison now checks for a matching basename and equivalent parent user-data paths. It returns the expected path when both checks pass, and null otherwise. New macOS-only tests cover a symlinked user-data directory and an alternate path casing.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to f6e6e

The change appears ready to merge after normal checks; no actionable merge-blocking risk is established.

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. Documentation-only edits and deletions within survi… 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 actual change: the supervised serve's update-handoff check now accepts a different spelling (casing) of the same profile path. It is specific, uses a conventional fix() prefix, and…
Description check Passed The description follows the repository template closely. It has the ELI5, What Changed, Why, Linked Issue (Fixes #34382), Visual Proof (N/A with reason), Testing, Review, Agent skill upstream boundary…
Linked Issues check Passed Directly linked issue #34382 requires that supervised macOS orca serve accept a handoff path whose profile directory differs from Electron's user-data path only in casing, keep the handoff filename …
Out of Scope Changes check Passed The whole-PR diff touches two files only. src/main/serve-update-handoff.ts changes the handoff path check that the issue names. Its import of isSameUserDataPath and removal of resolve are needed f…
Full details: Title check

Explanation

The title names the actual change: the supervised serve's update-handoff check now accepts a different spelling (casing) of the same profile path. It is specific, uses a conventional fix() prefix, and matches the diff and objectives. It is 78 characters, slightly long but within a reasonable range.

Full details: Description check

Explanation

The description follows the repository template closely. It has the ELI5, What Changed, Why, Linked Issue (Fixes #34382), Visual Proof (N/A with reason), Testing, Review, Agent skill upstream boundary, Notes, and Checklist sections, and it explains the mechanism and the alternatives considered. Gaps: the AI Disclosure section is blank (it is optional for maintainers, but the PR is cherry-picked from an external contributor, so attribution is noted only in prose), and the Testing section does not state which platforms were manually tested beyond a local macOS run. The 'Fixes #' placeholder is filled. The 'Review' section is filled in. Minor issues only.

Full details: Linked Issues check

Explanation

Directly linked issue #34382 requires that supervised macOS orca serve accept a handoff path whose profile directory differs from Electron's user-data path only in casing, keep the handoff filename fixed, compare the directory by filesystem identity, and still refuse a different profile. At head, getConfiguredHandoffPath() in src/main/serve-update-handoff.ts (diff vs. merge base) now requires basename(configuredPath) === basename(expectedPath) and isSameUserDataPath(dirname(configuredPath), dirname(expectedPath)). It returns expectedPath on a match and null otherwise. isSameUserDataPath (src/shared/serve-user-data-path.ts) applies realpathSync.native, which resolves the on-disk spelling, so a differently cased existing directory compares equal. A nonexistent or different directory falls back to the resolved string and does not match. The fixed handoff filename is enforced by the basename check. The test file adds accepts the service's other casing of the same profile directory (#34382), which covers the reported case on case-insensitive volumes, and a symlink-alias test. The alias test is a supporting test for the same check. Limit: the reviewer did not run the tests. The pass rests on source inspection and the helper's documented realpath behavior, and realpathSync.native returning on-disk casing on macOS is taken from the helper's design and the PR's reported test results, not independently observed.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff touches two files only. src/main/serve-update-handoff.ts changes the handoff path check that the issue names. Its import of isSameUserDataPath and removal of resolve are needed for that check. src/main/serve-update-handoff.test.ts adds the casing test for #34382 and a symlink-alias test for the same comparison, plus the mkdirSync and symlinkSync imports those tests use. No unrelated behavior, wire format, or other module changed. The PR description states that the service installer's launchd label (serve-service-unit.ts) was intentionally left as a string compare. That file is not in the diff, so no out-of-scope change exists there.

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. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

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

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.

@nwparker
nwparker merged commit c63b949 into main Oct 27, 2026
33 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.

[Bug]: macOS supervised serve cannot update when user-data path casing differs

2 participants