t3codePublic Synced with GitHub

fix(server): launcher repoints the unit the systemd manager loads #35966

Mergedjuliusmarmingemerged 2 commits intomain fromrefs/pull/35966/headmerged by juliusmarminge 9 hours agoView on GitHub

juliusmarmingecommented 9 hours ago edited

serviceUnitFromEnvironment in apps/server/src/serviceLauncher.ts built the systemd user unit path from the service's inherited XDG_CONFIG_HOME. When environment.d gives services a different value than the manager uses, update reconciliation opened a nonexistent path (ENOENT), left ExecStart on the old runtime and skipped daemon-reload. The service-log size limit was skipped too (#35943), because serviceLogPath reads the same unit. The launcher now asks the manager for its UnitPath with the same busctl --user --json=short get-property call that installation uses (5s timeout), and falls back to XDG_CONFIG_HOME or ~/.config only when that query fails or yields nothing. The rule for picking the directory (the absolute entry ending in /systemd/user.control whose non-.control twin is also listed) moved out of cloud/bootService.ts into a pure systemdUnitDirectoryOf in cloud/serviceProtocol.ts, which both the installer and the Effect-free launcher use. The installer's readSystemdUnitDirectory lost its now-unused path parameter. serviceUnitFromEnvironment is now async and awaited in main(). No new regex. The only new parser is JSON.parse of busctl output plus an array-of-strings check, linear in input size. The repro test was kept; its PATH line was changed to vi.stubEnv("PATH", bin) because path.delimiter and a process.env read failed typecheck. The fake runtime uses an absolute node path and the fake scripts use only shell builtins, so the bin directory alone is enough.

Fixes pingdotgg/t3code#35942

Reproduction on main

On origin/main 97cfc07759 (Linux x64, Node 26.13.1): ran the reporter's repro-xdg-unit.mjs unchanged against a fresh worktree. Output: 'control: unit repointed=true; daemon-reload calls=1', 'service-xdg-override: unit repointed=false; daemon-reload calls=0', '[service-launcher] Could not point $FIXTURE/service-xdg-override/service-config/systemd/user/t3code.service at t3@1.2.4: ENOENT', 'RESULT: defect reproduced'. Then added a test to apps/server/src/serviceLauncher.test.ts that runs the real launcher main() with a committed 1.0.0 -> 1.1.0 update, a unit at $HOME/.config/systemd/user/t3code.service, the service environment's XDG_CONFIG_HOME=$HOME/service-config, and fake busctl (reporting the manager's UnitPath with the ~/.config/systemd/user.control and user pair) and systemctl on PATH. It fails on main: ExecStart stays at 1.0.0 and there is no reload. With XDG_CONFIG_HOME set to $HOME/.config the same test passes. Not run against a real systemd user manager; the VM's live t3code.service was not touched.

Evidence

Run production launcher main() on Linux x64 / Node v26.12.1 with fresh synthetic committed-update state (1.2.3 → 1.2.4). Keep the installed unit under the manager's ~/.config directory and override the service's XDG_CONFIG_HOME. Manager commands use isolated shims. Both default-environment controls pass.

Before	After
97cfc07759	5e8f3ec0ed

Reproduction driver · Base transcript · Head transcript

Images render the captured command transcripts. The synthetic child intentionally exits after receiving serve.

Not shown:

A real systemd user manager, environment.d loading, and subsequent manager restart were not exercised; busctl and systemctl were isolated executable shims.
The remote-update transaction was not driven; both runs began with its committed service-state result and a synthetic runtime executable.

Tests

Before the fix: vp test run src/serviceLauncher.test.ts -t "environment.d" failed (ExecStart stayed at 1.0.0). After: vp test run src/serviceLauncher.test.ts src/cloud/bootService.test.ts passed (91 tests). vp run check:changed: format, lint, knip, graphql, typecheck (3 packages), test (3 files, 93 tests) all pass. Not run against a real systemd user manager.

Not in this change

apps/server/src/serverOwnership.ts:123 hardcodes ~/.config/systemd/user/t3code.service when deciding whether the boot service owns a state dir; it ignores both XDG_CONFIG_HOME and the manager's UnitPath, which is a related but separate defect.

Who did the work

Written by Claude Opus 5.5, in Claude Code
Reviewed by GPT-6.1 Sol, in Codex, and by Claude Opus 5.5, each on its own
Proof captured by GPT-6.1 Sol and checked by Claude Opus 5.5

Run by the t3code factory in T3 Code.

github-actions[bot](GitHub profile, opens in a new tab) added labelsvouch:trusted9 hours ago
tanstack-source added labelsbug9 hours ago
github-actions[bot](GitHub profile, opens in a new tab) added labelssize:M9 hours ago
macroscopeapp[bot](GitHub profile, opens in a new tab) approved these changes9 hours ago
macroscopeapp[bot](GitHub profile, opens in a new tab)commented 9 hours ago
macroscopeapp[bot](GitHub profile, opens in a new tab)commented 9 hours ago

Approvability

Verdict: Approved at 5e8f3ec

Macroscope's review found this PR approvable — The launcher now locates the unit through systemd's own search path, fixing updates when the service inherits a different configuration directory. The bounded query, existing fallback, and guarded unit replacement keep the operational impact limited.
^{You can add or adjust custom eligibility rules. [Learn more](https://docs.macroscope.com/approvability#example-custom-eligibility-rules).}

github-actions[bot]commented 9 hours ago

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	21.5 KiB	21.5 KiB	0 B (0.0%)	24.4 KiB	✅
Codex	Live turn messages	1	1	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.9 KiB	21.9 KiB	0 B (0.0%)	24.4 KiB	✅
Claude	Live turn messages	1	1	0 (0.0%)	8	✅

Other scenarios

All 64 scenario measurements match main.

All 64 scenario measurements

Provider	Scenario	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	`short turn resume`	`WebSocket decoded`	10.5 KiB	10.5 KiB	0 B (0.0%)	12.7 KiB	✅
Codex	`short turn resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Codex	`long turn running, cold open`	`HTTP decoded`	281.7 KiB	281.7 KiB	0 B (0.0%)	302.7 KiB	✅
Codex	`long turn running, cold open`	`rows`	75	75	0 (0.0%)	75	✅
Codex	`long turn running, history page`	`HTTP decoded`	92.4 KiB	92.4 KiB	0 B (0.0%)	107.4 KiB	✅
Codex	`long turn running, history page`	`rows`	75	75	0 (0.0%)	75	✅
Codex	`long turn finished, cold open`	`HTTP decoded`	381.5 KiB	381.5 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	`long turn finished, cold open`	`rows`	151	151	0 (0.0%)	151	✅
Codex	`long turn finished, socket cold open`	`WebSocket decoded`	381.6 KiB	381.6 KiB	0 B (0.0%)	410.2 KiB	✅
Codex	`long turn finished, socket cold open`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Codex	`long turn finished, socket cold open`	`rows`	151	151	0 (0.0%)	151	✅
Codex	`long turn finished, work page`	`HTTP decoded`	186.2 KiB	186.2 KiB	0 B (0.0%)	200.2 KiB	✅
Codex	`long turn finished, work page`	`rows`	150	150	0 (0.0%)	150	✅
Codex	`idle thread resume`	`WebSocket decoded`	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	`idle thread resume`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Codex	`fleet shell snapshot`	`HTTP decoded`	42.4 KiB	42.4 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	`fleet shell snapshot`	`rows`	27	27	0 (0.0%)	27	✅
Codex	`fleet shell subscribe`	`WebSocket decoded`	65 B	65 B	0 B (0.0%)	200 B	✅
Codex	`fleet shell subscribe`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Codex	`fleet shell resume check`	`HTTP decoded`	18 B	18 B	0 B (0.0%)	100 B	✅
Codex	`fleet shell resume check`	`rows`	0	0	0	0	✅
Codex	`stale shell reload`	`HTTP decoded`	42.4 KiB	42.4 KiB	0 B (0.0%)	53.7 KiB	✅
Codex	`stale shell reload`	`rows`	27	27	0 (0.0%)	27	✅
Codex	`stale shell resume`	`WebSocket decoded`	140 B	140 B	0 B (0.0%)	400 B	✅
Codex	`stale shell resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Codex	`stale todo list cold open`	`HTTP decoded`	72.4 KiB	72.4 KiB	0 B (0.0%)	83.0 KiB	✅
Codex	`stale todo list cold open`	`rows`	75	75	0 (0.0%)	75	✅
Codex	`stale todo list cold open`	`nodes`	2	2	0 (0.0%)	4	✅
Codex	`stale todo list cold open`	`subagents`	2	2	0 (0.0%)	4	✅
Codex	`stale todo list cold open`	`plans`	1	1	0 (0.0%)	2	✅
Codex	`stale todo list cold open`	`providerTurns`	1	1	0 (0.0%)	2	✅
Codex	`stale todo list cold open`	`checkpoints`	1	1	0 (0.0%)	2	✅
Claude	`short turn resume`	`WebSocket decoded`	10.6 KiB	10.6 KiB	0 B (0.0%)	12.7 KiB	✅
Claude	`short turn resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Claude	`long turn running, cold open`	`HTTP decoded`	283.3 KiB	283.3 KiB	0 B (0.0%)	302.7 KiB	✅
Claude	`long turn running, cold open`	`rows`	75	75	0 (0.0%)	75	✅
Claude	`long turn running, history page`	`HTTP decoded`	93.4 KiB	93.4 KiB	0 B (0.0%)	107.4 KiB	✅
Claude	`long turn running, history page`	`rows`	75	75	0 (0.0%)	75	✅
Claude	`long turn finished, cold open`	`HTTP decoded`	384.2 KiB	384.2 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	`long turn finished, cold open`	`rows`	151	151	0 (0.0%)	151	✅
Claude	`long turn finished, socket cold open`	`WebSocket decoded`	384.3 KiB	384.3 KiB	0 B (0.0%)	410.2 KiB	✅
Claude	`long turn finished, socket cold open`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Claude	`long turn finished, socket cold open`	`rows`	151	151	0 (0.0%)	151	✅
Claude	`long turn finished, work page`	`HTTP decoded`	188.1 KiB	188.1 KiB	0 B (0.0%)	200.2 KiB	✅
Claude	`long turn finished, work page`	`rows`	150	150	0 (0.0%)	150	✅
Claude	`idle thread resume`	`WebSocket decoded`	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	`idle thread resume`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Claude	`fleet shell snapshot`	`HTTP decoded`	43.0 KiB	43.0 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	`fleet shell snapshot`	`rows`	27	27	0 (0.0%)	27	✅
Claude	`fleet shell subscribe`	`WebSocket decoded`	66 B	66 B	0 B (0.0%)	200 B	✅
Claude	`fleet shell subscribe`	`WebSocket messages`	1	1	0 (0.0%)	1	✅
Claude	`fleet shell resume check`	`HTTP decoded`	18 B	18 B	0 B (0.0%)	100 B	✅
Claude	`fleet shell resume check`	`rows`	0	0	0	0	✅
Claude	`stale shell reload`	`HTTP decoded`	43.0 KiB	43.0 KiB	0 B (0.0%)	53.7 KiB	✅
Claude	`stale shell reload`	`rows`	27	27	0 (0.0%)	27	✅
Claude	`stale shell resume`	`WebSocket decoded`	140 B	140 B	0 B (0.0%)	400 B	✅
Claude	`stale shell resume`	`WebSocket messages`	2	2	0 (0.0%)	2	✅
Claude	`stale todo list cold open`	`HTTP decoded`	73.4 KiB	73.4 KiB	0 B (0.0%)	83.0 KiB	✅
Claude	`stale todo list cold open`	`rows`	75	75	0 (0.0%)	75	✅
Claude	`stale todo list cold open`	`nodes`	2	2	0 (0.0%)	4	✅
Claude	`stale todo list cold open`	`subagents`	2	2	0 (0.0%)	4	✅
Claude	`stale todo list cold open`	`plans`	1	1	0 (0.0%)	2	✅
Claude	`stale todo list cold open`	`providerTurns`	1	1	0 (0.0%)	2	✅
Claude	`stale todo list cold open`	`checkpoints`	1	1	0 (0.0%)	2	✅

Baseline: 0c4b1d8 · PR result: 5e8f3ec · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

Codex decoded thread snapshot: 108.5 KiB
Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

coderabbitai[bot]commented 9 hours ago edited

No actionable comments were generated in the recent review.

Walkthrough

The systemd unit directory lookup now uses the user manager’s UnitPath when available. Cloud manager discovery and the service launcher use the shared directory-selection helper. The launcher retains its existing fallback and now awaits unit resolution.

Changes: Systemd unit directory discovery

Layer / File(s)	Summary
Shared directory selection `apps/server/src/cloud/serviceProtocol.ts`, `apps/server/src/cloud/bootService.ts`	A shared helper selects a directory from the manager’s `UnitPath` when both the `/systemd/user.control` path and its corresponding path are present. Cloud manager discovery now uses the helper.
Launcher directory resolution `apps/server/src/serviceLauncher.ts`, `apps/server/src/serviceLauncher.test.ts`	The launcher queries the manager’s `UnitPath` and uses the selected directory when available. If the query or response is unavailable or invalid, it uses the existing XDG-based fallback. A test checks that the manager-reported unit is updated and reloaded.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

pingdotgg/t3code#35943 — The issue also concerns locating the installed unit from the manager’s UnitPath. Its log-bounding requirement is not addressed by this change.

Possibly related PRs

pingdotgg/t3code#29244 — This PR uses the same manager UnitPath lookup and XDG-based fallback for unit directory discovery.

Suggested labels: bugfix

Suggested reviewers: marve10s, t3dotgg, voltcrash

Merge Risk: Minimal · up to 5e8f3

The launcher now finds the systemd unit using the manager's reported directory and falls back to the previous location when that lookup fails. No merge-blocking risk was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: the service launcher now repoints the unit that the systemd manager loads. It follows the conventional commit format. The changeset matches this: the launcher queries …
Linked Issues check		Issue `#35942` requires the Linux launcher to find the installed unit through the manager's unit directory, not the service's XDG_CONFIG_HOME, and to repoint the unit and run systemctl --user daemon-rel…
Out of Scope Changes check		The changes support the `#35942` fix. serviceProtocol.ts adds systemdUnitDirectoryOf, the shared directory-selection rule that the launcher uses and that bootService.ts previously held inline. The bootS…
Description check		The description explains the defect (serviceUnitFromEnvironment derived the unit path from the inherited XDG_CONFIG_HOME, causing ENOENT, a stale ExecStart, and a skipped daemon-reload), the fix (quer…

Full details: Title check

Explanation

The title names the main change: the service launcher now repoints the unit that the systemd manager loads. It follows the conventional commit format. The changeset matches this: the launcher queries the manager's UnitPath through busctl and updates that unit.

Full details: Linked Issues check

Explanation

Issue #35942 requires the Linux launcher to find the installed unit through the manager's unit directory, not the service's XDG_CONFIG_HOME, and to repoint the unit and run systemctl --user daemon-reload after a committed update. The diff to apps/server/src/serviceLauncher.ts adds readSystemdUnitDirectory, which runs busctl --user --json=short get-property on UnitPath with a 5s timeout. It passes the result to systemdUnitDirectoryOf. serviceUnitFromEnvironment awaits this result and uses it as the unit directory. The existing reload callback still runs systemctl --user daemon-reload. The author reports a committed test in apps/server/src/serviceLauncher.test.ts that fails on main and passes with the change. The test file was not inspected in this review. Limits: the fix depends on busctl output, which the tests fake. Behavior on a real systemd user manager is not established. Issue scope on downtime, reboot, and runtime pruning is not a coding requirement of this PR.

Full details: Out of Scope Changes check

Explanation

The changes support the #35942 fix. serviceProtocol.ts adds systemdUnitDirectoryOf, the shared directory-selection rule that the launcher uses and that bootService.ts previously held inline. The bootService.ts change removes the now-unused path parameter from readSystemdUnitDirectory, and its callers follow. The serviceLauncher.test.ts addition is the regression test for this defect. The PR description names apps/server/src/serverOwnership.ts:123 as a separate defect and leaves it unchanged. That file is not in the reviewed diff. No unrelated change was found in the inspected diffs.

Full details: Description check

Explanation

The description explains the defect (serviceUnitFromEnvironment derived the unit path from the inherited XDG_CONFIG_HOME, causing ENOENT, a stale ExecStart, and a skipped daemon-reload), the fix (querying the manager's UnitPath via busctl with fallback, and moving the directory rule into systemdUnitDirectoryOf), and a linked issue (#35942). It includes a reproduction on main, a before/after evidence table, test results, and an explicit list of what was not verified. It also names the authoring models. The template's required sections (Problem, Change, Scope, Verification) are all present in substance, though the headings do not match the template exactly. Scope is covered by the linked bug issue, and the Verification section is detailed and includes what was not checked. Minor gap: the template asks for a conventional commit style title, and the description does not show a title. The description is mostly complete.

Finishing Touches

Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

Source mode: Auto

Repository: pingdotgg/t3code

Branch: factory/issue-35942

File: .coderabbit.config.ts

Commit: 5e8f3ec

Layers: Repository TS config

Review profile: CHILL

Plan: Team

Run ID: `ed558128-d341-4c77-a50c-268fab20d939`

Commits

Reviewing files that changed from the base of the PR and between 0c4b1d8f277db542ac0142699748e42b80540814 and 5e8f3ec0edbc2b8696f3036c9454e828185a75de.

Files selected for processing: 4

`apps/server/src/cloud/bootService.ts`

`apps/server/src/cloud/serviceProtocol.ts`

`apps/server/src/serviceLauncher.test.ts`

`apps/server/src/serviceLauncher.ts`

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

juliusmarminge merged this99224f09 hours ago
juliusmarminge closed this9 hours ago

fix(server): launcher repoints the unit the systemd manager loads #35966

Reproduction on main

Evidence

Tests

Not in this change

Who did the work

Reviewers

Assignees

Labels

Fixes

Approvability

Thread transfer impact

Other scenarios

Reproduction on main#

Evidence#

Tests#

Not in this change#

Who did the work#

Approvability#

Thread transfer impact#

Other scenarios#

Reproduction on main

Evidence

Tests

Not in this change

Who did the work

Approvability

Thread transfer impact

Other scenarios