refactor(desktop): share the dev userData profile across worktrees

[Astro-Han]

Problem

The opt-in TCC bundle and plain npm run dev disagree about where the developer's data lives:

• plain dev → ~/Library/Application Support/Maka Dev
• TCC bundle → ~/Library/Application Support/Maka Dev-<WORKTREE_ID>

Turning MAKA_DEV_TCC on or off therefore silently switched the database: sessions, settings, and the single-instance lock all moved. TCC isolation comes from the bundle identity (DEV_BUNDLE_ID, per-worktree cdhash), not from the data root — so the data root should be shared.

Sharing the root means two launches can now collide on one profile. Electron's single-instance lock already decides that collision; what was missing was the dev launcher consuming the lock's answer instead of guessing at it.

What changes

Shared profile. DEV_USER_DATA_DIR drops the WORKTREE_ID suffix — both launch shapes use the shared Maka Dev root. On first launch a console notice reports a legacy per-worktree root if one exists; nothing is deleted.

Reclaim this worktree's own instances before launching. Both launch paths call ensureNoRunningDevelopmentApp first, which covers the three process shapes a single plain launch produces (the npm shim, the resolved Electron child, and the TCC bundle). It matches this worktree's own literal REPO_ROOT paths only — it never touches another worktree's windows.

Consume the lock's verdict. When requestSingleInstanceLock() fails in dev, the app writes a marker line and exits with code 42; packaged builds keep exiting 0. plain launches read the exit code; the TCC bundle has no exit-code channel, so the launcher reads the marker from the log. All four of the monitor's terminal paths consult the marker, so an absorbed launch is reported as absorbed rather than misread as a crash or a silent no-op. Unless the launcher itself handled the conflict, the losing instance also shows a native dialog.showErrorBox naming the profile — that is the only Electron API documented as safe to call before ready, and on Linux it degrades to stderr.

What was removed, and why

An earlier revision of this PR added a pre-flight probe that tried to answer "which worktree currently owns this data directory?" by running pgrep -f and pattern-matching the results. That entire mechanism is deleted here (dev-app-profile.mjs, its test, and the probe/pattern/owner chain in dev-app-runtime.mjs), because the question it asked cannot be answered from a command line:

• the TCC profile is not on the command line at all — it lives in dev-env.json, which is mutable between launch and check;
• flat ps output has lost quoting, so a real value equal to target + space + more is indistinguishable from target followed by a positional argument;
• the npm shim reports argv[0] as node;
• helper, GPU, and about-to-exit processes all match the same pattern;
• an app started by LaunchServices (Dock, Finder) never presents the expected argv shape.

The single-instance lock already answers this authoritatively and atomically. Two mechanisms for one question, where the cheaper one is a heuristic that can disagree with the authority, is worse than one. Net effect on the branch is about −480 lines.

If we later want to name the blocking instance, the criterion has to be the lock itself — reading the SingletonLock symlink target, or having the lock holder write its own owner.json — and it should be a message shown after the lock has spoken, not a pre-flight gate. That is deliberately not in this PR. Background and the dead ends we ruled out are recorded in #3539.

Two smaller fixes ride along:

• handleMonitorOutcome's default branch is now fail-closed (exit 1). It previously exited 0, so an unrecognized outcome reported success.
• splitDevelopmentCliArgs now strips the launcher flag unconditionally. It was conditional, which meant a user passing the flag by hand could persist it into dev-env.json and permanently silence the conflict dialog for every later Dock launch.

Data visibility (read before upgrading)

Existing TCC data lives in ~/Library/Application Support/Maka Dev-<WORKTREE_ID>. After this change the app reads the shared Maka Dev root instead. The old directory is not deleted — it stays on disk. To recover from it, copy it over the shared root or point --user-data-dir at it; to reclaim the space, remove it yourself.

Known limitations (not regressions)

• Windows keeps main's behaviour; the reclamation step is a macOS/Posix dev concept.
• A hung dev instance can be SIGKILLed by the next launch. Chromium's NotifyOtherProcessOrCreate() passes kill_unresponsive=true; a main process that cannot turn its event loop cannot ACK, and after 20 seconds the new instance kills the holder and takes the lock. Reproduced on macOS against Electron 43.2.0 / Chromium 150.0.7871.129. This is not introduced here: on the merge base plain npm run dev already used the shared Maka Dev root, so plain↔plain across worktrees could already race. Sharing the profile widens which pairs can collide; it does not create the behaviour. The README records which parts of this we reproduced directly and which are inferred from the Chromium mechanism.

Verification

• apps/desktop/scripts/dev-app-runtime.test.mjs: 13 tests covering the four-cell marker judgment, the ordinary (non-absorbed) counterpart of each cell, the loser-contract import failure, and the unconditional flag stripping across all three call shapes.
• packages/core/src/__tests__/dev-single-instance.test.ts: the dialog gate defaults to showing and is silenced only by the launcher flag.
• Mutation-checked in both directions: forcing the marker predicate false reds 3 tests; forcing it true reds 4. Neither direction leaves the suite green.
• npx tsc --noEmit -p tsconfig.main.json clean for src/main/main.ts; full CI (build, typecheck, Knip, unit, e2e) green.
• The two one-line couplers in dev.mjs and start-dev-app.mjs are the only uncovered surface, and are marked as such in the scripts.

Follow-up on naming the blocking instance is tracked in #3539.

[hqhq1025]

Reviewed the current revision across the TCC bootstrap, the plain Desktop and repository CLI profile contracts, and runtime cache invalidation. The shared Maka Dev data root now matches the other development entry points while the worktree-scoped bundle identity remains unchanged; the previous marker is invalidated by the new burned-in path. The script syntax and focused marker/bootstrap assertions passed, and the required CI check is green.

Codex-assisted review performed under the maintainer-approved review workflow.

[jackwener]

Automated Codex review — blocking finding on exact head c17e155f47d8ff5cae02cb20a41f5a9f91950b01.

P2: the shared profile invalidates the launcher's worktree-local stale-owner recovery. resolveMacosDevelopmentLaunch() still calls ensureNoRunningDevelopmentApp(), but that function probes and kills only this worktree's DEV_EXECUTABLE. After this PR, a TCC app from worktree A and a launch from worktree B use the same Maka Dev single-instance lock while having different executable paths. If A is still running—or was orphaned after its terminal/Vite process died—B cannot see or stop it. B's Electron instance is absorbed by A's lock and exits, while B's monitor only watches B's path and eventually reports never-started. This is the same stale-window failure the ownership step's comment says it prevents, now moved to the cross-worktree case.

The README partly exposes the mismatch: it says another worktree's app survives and holds the shared lock, then says a launch reclaims an app left by a hard-killed session. The latter is no longer true across worktrees.

Before merge, make launch ownership profile-scoped as well as userData profile-scoped (or fail immediately with an explicit current-owner contract), and add a two-worktree regression that proves a stale owner cannot absorb the new launch. Keep DEV_BUNDLE_ID worktree-scoped for TCC as this PR already does.

Required conclusions:

1. Optimal for the actual problem: not yet; the data-root correction is sound, but ownership follows the old isolation boundary.
2. Production code to delete: none identified.
3. Tests to delete/replace: none; current marker smoke coverage misses the cross-worktree lock behavior.
4. Deeper refactor: no; align the existing stale-owner recovery with the new shared-profile boundary.
5. Ready to merge: no, despite green test and a current-head committer approval.
6. Residual risk/gaps: stale/other-worktree app absorption and 30-second false never-started failure.

This is a developer-workflow behavior change; the existing independent human review remains necessary under CONTRIBUTING.md after the blocker is resolved.

[jackwener]

Re-review of unchanged exact head c17e155f47d8ff5cae02cb20a41f5a9f91950b01 against current main@d62857a8357e9160926726a2a13096bc2dc2b91d. Converting the existing blocking automated comment into an explicit changes-requested gate because the head has not changed.

P2 — shared userData leaves stale-owner recovery worktree-local. resolveMacosDevelopmentLaunch() still asks ensureNoRunningDevelopmentApp() to find/kill only this worktree's DEV_EXECUTABLE. After the PR, worktree A and B use one Maka Dev single-instance lock but retain different executable paths. A running or orphaned app from A is invisible to B's recovery; B is absorbed by A's lock, exits, and its own-path monitor eventually reports never-started. Make launch ownership profile-scoped too, or fail immediately with an explicit current-owner contract, and add a two-worktree regression.

Required conclusions remain:

1. Optimal: not yet; data ownership and process ownership use different boundaries.
2. Production code to delete: none identified.
3. Tests to delete/replace: none; add cross-worktree ownership coverage.
4. Deeper refactor: no; align the existing recovery seam with the shared profile.
5. Ready to merge: no, despite green test and the earlier human approval.
6. Residual risks/gaps: stale-window absorption and a false 30-second never-started failure; developer-workflow behavior needs fresh independent human review after repair.

[Astro-Han]

Pushed a fix for the blocking finding — new head 21e165b87, rebased onto 4acfa2693.

@jackwener your P2 was right: sharing the userData root made Electron's single-instance lock cross-worktree while ensureNoRunningDevelopmentApp() still probed and killed only this worktree's DEV_EXECUTABLE. We verified the absorption path end to end before changing anything.

We took the second of the two options you offered — fail immediately with an explicit current-owner contract — rather than making the kill profile-scoped. The reason: a profile-scoped kill would mean worktree B's launch script terminates a window someone may be actively using in worktree A. Sharing a data root does not confer disposal rights over another worktree's process, so naming the owner and stopping is both smaller and more honest than expanding what we are willing to kill.

What changed:

• assertNoCrossWorktreeOwner() runs before ensureNoRunningDevelopmentApp() — establish that no foreign owner holds the lock first, then clean up our own leftovers.
• sharedDevelopmentAppCommandLines() probes via pgrep -af and treats exit codes 0 and 1 as the only valid answers; anything else throws rather than being read as "nothing is running", so a probe failure cannot be mistaken for a clear lock.
• The error names the owner's full command line and says what to do about it (Quit it (Cmd-Q)), because the failure this replaces was exit-0 plus a never-started report 30 seconds later — indistinguishable from a normal launch.
• apps/desktop/README.md now says same-worktree reclaim still works while cross-worktree fails fast with the owner named, replacing the claim you flagged as no longer true.

The two-worktree regression you asked for is in apps/desktop/scripts/dev-app-runtime.test.mjs — 7 cases covering: another worktree's command line reported as owner, our own not reported, none running, a foreign owner producing a throw whose message carries both the guidance and the owner path, and a path that merely shares a suffix still classified as foreign. Wired into the desktop test:dist chain so it runs in CI rather than only locally.

DEV_BUNDLE_ID stays worktree-scoped for TCC, as you asked.

One note on the diff you will see: the branch's merge base predated the ASF header sweep, so the rebase onto current main picked up headers that already exist upstream. The only substantive commit is 21e165b87.

---

This work was AI-assisted. The finding was reproduced and the fix verified against the exact head above; please push back where we got it wrong.

P2 is addressed on 21e165b: assertNoCrossWorktreeOwner fails fast when another worktree holds the shared Maka Dev lock, with tests for foreign vs own paths. Dismissing the stale CHANGES_REQUESTED.

[jackwener]

Independent review of 21e165b87. Did not treat Astro-Han authorship as evidence.

1. Problem. TCC dev userData was Maka Dev-<worktreeHash> while CLI and plain npm run dev use Maka Dev. Enabling TCC silently switched the database and broke attaching to the same host.

2. Solution. Share ~/Library/Application Support/Maka Dev. Keep DEV_BUNDLE_ID worktree-scoped for TCC. Burn userDataDir into the runtime marker so old cached apps rebuild.

3. The earlier P2 (worktree-local stale-owner recovery vs shared lock) is fixed: assertNoCrossWorktreeOwner pgrep's every Maka Dev.app Electron, treats a non-own path as owner, and fails immediately instead of being absorbed. Own-path prefix is not confused with a sibling suffix. Tests cover that. Own-worktree leftovers still go through ensureNoRunningDevelopmentApp.

Approve. Merge stays with the author.

[jackwener]

Follow-up after independently checking Darwin pgrep on macOS 26. My earlier APPROVE was too early.

pgrep -af on Darwin is not Linux procps. Here -a means include ancestors, and without -l stdout is PIDs only. -f -l is what prints PID + full argv. The production probe therefore returns bare PIDs, startsWith(ownExecutable) never matches, and this worktree's leftover TCC app is treated as a foreign owner — fail-fast runs before ensureNoRunningDevelopmentApp(). That is a real P1 on this head.

Second P1: the probe only matches Maka Dev.app/.../Electron. Plain npm run dev is Electron.app/.../Electron with app.setName('Maka Dev'), which still holds the shared profile lock. That process is invisible to the new owner check, so the original absorption/never-started hole is not actually closed.

P2: --user-data-dir is applied after the owner probe, so an isolated launch can be blocked by an unrelated default-profile owner.

The shared Maka Dev root and worktree-scoped DEV_BUNDLE_ID are still the right data-plane cut. The process-owner probe has to key on the profile, with Darwin pgrep -fl, not on one bundle suffix.

Not converting this to REQUEST_CHANGES (comment-only). I no longer consider this head merge-ready.

Darwin pgrep -af does not print argv; own leftover TCC app is misclassified as foreign owner, and plain Electron.app npm-run-dev is invisible to the probe. Independent re-check with kabi-sol. Dismissing this APPROVE; head 21e165b is not merge-ready.

[jackwener]

Independent re-review of 459995ac. The pgrep-then-ps split is the right Darwin fix (I reproduced pgrep -f PIDs + ps -p pid -o command= on this Mac; BSD -a is not involved). The owner judgment still has two P1s. Do not merge this head. GitHub APPROVED is a leftover hqhq1025 review on c17e155f, not a current GO.

P1: worktreeFromCommandLine splits on whitespace. Paths contain spaces. This file already documents ~/Dropbox (Personal) as a pgrep-regex case. On that command line the /apps/desktop and /node_modules/ markers no longer start with /, so the function returns undefined. The owner then does not recognise self and ?? DEV_USER_DATA_DIR treats the process as a foreign holder of the shared lock — self-blocking, same shape as pgrep -af. Maka Dev.app still parses only because /apps/desktop appears before that space; that is luck. Scan the raw string for the marker, then walk left to a / (space+slash) argv boundary; do not tokenise.

P1: the scan is any npm Electron, not Maka Dev. The rough filter matches node_modules/.bin/electron and Electron.app/Contents/MacOS/Electron. Another repo's Electron recovers a worktree, has no maka dev-env.json, and is attributed to the shared Maka Dev profile. Launching then fail-fasts as a cross-worktree owner. Missing env file ≠ "uses the Maka default profile". Only Maka Dev.app, or a worktree that actually published a maka env file, should participate in the lock judgment.

Tests still use synthetic argv and say real Darwin pgrep/ps samples are "still being collected". That is the same self-proving fixture failure as the previous startsWith(node_modules/.bin/electron) head. Cover a space-containing root and an unrelated node_modules/electron process.

[jackwener]

P1: split(/\s+/) drops any marker that does not stay an absolute token. Maka Dev.app happens to still work because /apps/desktop is before that space; /Users/me/Dropbox (Personal)/maka/apps/desktop/... returns undefined. Same class this file already escapes for pgrep. Search the raw line for /apps/desktop / /node_modules/, then walk left to a / argv boundary.

[jackwener]

P1: resolveProcessUserDataDir(...) ?? DEV_USER_DATA_DIR plus the broad Electron pgrep filter attributes every unmatched npm-electron process to the shared Maka Dev lock. A missing env file is not evidence that the process is using this profile. Default only for the unique Maka Dev.app shape, or when the env file actually exists and omits userDataDir.

[jackwener]

Integration pass on cbdd2cc2 (not an approve).

TCC and plain both hit assertNoCrossWorktreeOwner then ensureNoRunningDevelopmentApp. Data-root drop of WORKTREE_ID plus the legacy-dir warn matches the README shutdown/owner text. CI runs test:dist, which includes the new script tests.

P1: Linux startDevelopmentApp now probes this worktree's node_modules/.bin/electron shim, but quitMacosDevelopmentApp still no-ops off darwin. A leftover same-worktree npm run dev therefore throws could not be stopped. On main, liveness only looked at the Darwin bundle path, so this path did not fail-closed. Either teach quit to signal the Linux shim, or keep shim/real reclaim Darwin-only.

GitHub APPROVED is still the stale hqhq1025 review on an older head.

[jackwener]

P1 on Linux: this new plain-path reclaim calls isDevelopmentAppRunning, which now pgrep's the npm shim. quitMacosDevelopmentApp returns false when platform !== 'darwin', so a leftover same-worktree shim throws instead of being ignored (main) or killed. Windows is skipped in the probe; Linux is not.

[jackwener]

Integration re-review of 4f6e838d (not an approve).

P1: post-spawn recheck on appeared → exited and plain child exit 0 conflates absorption with a normal session end. Cmd-Q of a TCC app takes the appeared-then-exited path; a finished npm run dev takes exit-0. recheckAfterAbsence then reports any remaining foreign/false-positive holder as "absorbed this launch". That is only valid when this launch never became the lock owner (never-started, or an immediate second-instance exit). After the app has appeared, the lock was ours; a later process is not this launch being absorbed.

Tests currently require the appeared→exited path to print the conflict. That encodes the false positive.

Fix: recheck only on never-started (TCC) and on a short-lived exit-0 (plain). Do not recheck after a session that actually ran.

[jackwener]

P1: this is the Cmd-Q / normal end path (appeared was true). Recheck here will print an absorption error if anything else still matches holdsProfile (including a foreign apps/desktop Electron). Recheck belongs on never-started only.

[jackwener]

P1: Electron second-instance absorption AND a normal Cmd-Q both exit 0. Rechecking every exit-0 will false-alarm a finished session whenever another holder is still visible. Gate this on a short-lived child (startup window), not every zero exit.

[Astro-Han]

Heads-up: this branch was force-pushed twice just now, so anyone with a local copy will need to re-fetch. Current head is 8d3e52f70bfb4e3dd74ad77d5dff078a987e066c.

What changed in the code. The launcher's pre-flight owner probe is gone. It tried to answer "which worktree currently owns this dev data directory?" by matching pgrep -f output against an expected bundle path and argv shape. That premise does not hold: the TCC profile is not on the command line (it lives in a mutable dev-env.json), plain values contain spaces, the npm shim reports argv[0] as node, and helper, GPU, and about-to-exit processes all match the same pattern. A LaunchServices-started app never presents the argv shape the probe expected at all. Rather than keep patching the heuristic, we removed it and rely on the single-instance lock, which is the actual authority for that question. dev-app-profile.mjs and its test are deleted; net effect on the branch is roughly −480 lines.

Two smaller fixes went in with it: the monitor's default branch is now fail-closed instead of exiting 0 on an unrecognized outcome, and splitDevelopmentCliArgs now strips the launcher flag unconditionally. The latter was conditional before, which meant a user passing the flag by hand could persist it into dev-env.json and permanently silence the conflict dialog for Dock launches.

Some context on the rejected approach is in #3539, including what we verified about Chromium's NotifyOtherProcessOrCreate behaviour and where our conclusions are inferences rather than direct observations.

Why the second force-push. Twenty-eight of the thirty commits were authored as Ubuntu <[email protected]> — the default git identity on the machine the work ran on, not the actual author. They are now correctly attributed. The tree is byte-identical across the rewrite (git diff between the pre- and post-rewrite heads is empty) and the commit count is unchanged; only author and committer metadata differ.

@hqhq1025 — sorry for the churn. Your approval is on c17e155f from Aug 20, which predates all of this, so it no longer reflects what is here. Whenever you have time, a fresh look at 8d3e52f7 would be appreciated. No rush from our side.