fix(server): t3 service install no longer moves a --base-dir service onto an empty ~/.t3#23878

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Propagate non-NotFound unit read failures from BootService.install.

Effect.option converts permission errors from fs.readFileString(unitPath) to None, so the base-directory guard is skipped. If fs.exists(unitPath) returns true, a direct BootService.install call can stop and rewrite the existing unit without checking its base directory.

Treat only a missing unit as absent. Propagate other read failures through BootServiceInstallError.

The normal CLI path calls status before install, and status reads the unit without Effect.option, so this issue is limited to direct service callers.

✅ Addressed in commit ca05875

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Name which t3 service commands leave another home's service alone.

t3 service install and t3 service status leave the service unchanged and name the home it serves. t3 service restart leaves it unchanged but reports that the service is not installed, without naming its home. t3 service uninstall removes the unit even when run with another base directory. Name each command so users do not expect uninstall to leave the service alone.

✅ Addressed in commit ca05875

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Parse quoted systemd assignments before treating T3CODE_HOME as absent.

T3 does not generate the whole-assignment form. However, a valid manually edited unit can use Environment="T3CODE_HOME=/other/home". bootServiceBaseDirOf misses that form, so install can overwrite a service for another home. Accept both forms and add an installation test for the quoted form.

Leaving this out. T3 writes the unit itself and always uses Environment=T3CODE_HOME=.... The quoted form only shows up in a hand-edited unit, and restart, withStopped, t3 update and t3 uninstall read the home with the same parser. Supporting it would change all of them, which is beyond this fix. A unit with no recognised T3CODE_HOME keeps the old repair behaviour, as the PR body says.

@shivamhwp Understood. T3’s renderer uses Environment=T3CODE_HOME=..., which the parser supports. This PR intentionally preserves repair behavior when no T3CODE_HOME is recognized.

I withdraw this finding for this PR. Support for hand-edited assignment formats is a separate enhancement. Would you like a follow-up issue for that?

✅ Review thread resolved.

_{You are interacting with an AI system.}

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Serialize the unit home check and unit write.

BootService.install checks unitPath only once. Two separate CLI processes can both observe that the unit is absent, complete the asynchronous installation work, and later write their own unit configuration. The later writeDurably(unitPath, manager.render(plan)) can replace the service with the other process's baseDir.

Protect the per-user unit installation with a cross-process lock. Re-read and validate the unit home while holding that lock, before changing service files. An in-process semaphore is not sufficient for separate CLI processes.

Leaving this out. Two t3 service install runs at the same moment from different homes race the same way on main, before and after this PR. The issue is a single install run hours after the service was set up, and that is what this guards. A cross-process lock around the unit is a separate change.

@shivamhwp Agreed. This PR guards an existing service against a later install from a different home. The concurrent-install race is a separate concern, not a regression introduced here. I withdraw the cross-process locking requirement for this PR.

Would you like me to open a follow-up issue for that race?

✅ Review thread resolved.

_{You are interacting with an AI system.}

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use the canonical home comparison for maintenance operations.

install accepts a symlink alias because it compares canonical paths. restart and withStopped still compare path.resolve values, so the alias can differ from the installed target. Both operations can skip the service. withStopped can therefore run its operation without stopping the service.

Reuse otherBaseDirOf while preserving the existing skip behavior for units without T3CODE_HOME.

✅ Addressed in commits 96d8f4d to defd523

Fixed in defd523f04: restart and withStopped now use otherBaseDirOf, so a symlinked home counts as the same one there too. Units without T3CODE_HOME are still skipped. The restart test now also covers the alias.

@shivamhwp The supplied code confirms that restart and withStopped now use otherBaseDirOf for canonical home comparison. Both still skip units without T3CODE_HOME. This addresses the finding.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟠 High cloud/bootService.ts:619

Installing with --base-dir set to a symlink for the already-installed home fails with BootServiceOtherBaseDirError, and status reports that home as otherBaseDir. path.resolve only normalizes path segments, so compare fs.realPath results before treating the paths as different homes.

Leaving this as is on purpose. A symlinked --base-dir now gets a refusal, not data loss: install stops before touching anything, and the error names the exact --base-dir from the unit, which works. restart and uninstall on main already compare with path.resolve the same way. Symlink handling was in an earlier revision and was cut to keep this PR to the fix for #23858.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Do not treat every unit-read failure as an absent unit.

Effect.option also hides permission and other read failures. A direct BootService.install call can then pass the home guard and replace an existing unit. Treat only a missing unit as absent; map other failures to BootServiceInstallError. This repeats a previously addressed review finding.

Leaving this on purpose. The unit lives in the user's own ~/.config/systemd/user or ~/Library/LaunchAgents. If it exists but this user can't read it, install's later write to the same path fails too, so the unit isn't replaced. The fail-closed read was in an earlier revision and was cut to keep this PR minimal.

@shivamhwp Understood that you removed this check to keep the PR minimal.

One detail changes the safety argument: writeDurably writes a temporary file, then calls fs.rename(tempPath, filePath). If the parent directory permits replacement, that rename can replace an unreadable unit. It does not require read or write permission on the existing file.

The concern therefore remains valid, but I respect your decision to leave this unchanged. Would you like a follow-up issue for the fail-closed read?

_{You are interacting with an AI system.}

You're right, my reason was wrong: the rename can replace an unreadable unit. Still leaving it out of this PR. The unit sits in the user's own config directory, so it is only unreadable if they removed their own read permission, which is not the path in #23858. No follow-up issue needed for now.

@shivamhwp Understood. You are leaving the unit-read behavior unchanged to keep this PR focused on #23858. I won’t press for this change or open a follow-up issue.

_{You are interacting with an AI system.}

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restore canonical home comparison.

If input.baseDir is a symlink to the installed home, path.resolve still returns different path strings. status then reports another T3 home, and install refuses a service that already serves the requested home. Compare real paths, with a fallback for paths that do not exist yet. Based on learnings, equivalent filesystem paths must not produce a false mismatch.

Source: Learnings

Leaving this on purpose, same as the Macroscope thread above. A symlinked --base-dir gets a refusal that changes nothing, and the error names the --base-dir that works. Symlink handling was cut from this PR to keep it to the fix for #23858.

@shivamhwp Understood. The symlink refusal leaves the service unchanged, and the error gives the installed --base-dir to use. I withdraw the request to add symlink handling to this PR; it is outside the fix for #23858.

Would you like a follow-up issue for symlink handling?

✅ Review thread resolved.

_{You are interacting with an AI system.}