Skip to content

fix: preserve launch PATH for tmux - #608

Merged
Jephuff merged 15 commits into
mainfrom
preserve-launch-path
Aug 12, 2026
Merged

fix: preserve launch PATH for tmux#608
Jephuff merged 15 commits into
mainfrom
preserve-launch-path

Conversation

@Jephuff

@Jephuff Jephuff commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • capture the installation shell's exact PATH in the generated TBD.app bundle
  • pass that PATH to the initial LaunchServices open and preserve it for later relaunches
  • resolve tmux only from inherited PATH in both the app and daemon
  • remove daemon PATH augmentation and fixed package-manager search locations

Root cause

The restart script's initial open and LaunchServices crash relaunch used different environments. A relaunch could therefore lose the PATH entry containing tmux, making every new terminal fail until the app was restarted from a shell.

Design trade-off

The captured installation PATH is the daemon-wide inherited environment, not only the tmux search path. TBDDaemon inherits it from TBDApp, and every daemon descendant inherits it in turn. GitManager still launches the git executable at its pre-existing fixed /usr/bin/git path; helpers that git invokes, such as git-lfs, depend on the inherited PATH. Tmux has an explicit saved-executable fallback; other tools do not. Restoring fixed package-manager directory augmentation would reintroduce a hidden, machine-specific search policy for every subprocess, so this PR intentionally removes that behavior.

Verification

  • bash scripts/restart-environment-lib.test.sh — 9/9 passed; verifies plist-generation helper behavior, not an OS-level crash/LaunchServices relaunch end to end
  • startup diagnostics report the resolved tmux path and whether it came from PATH or the saved fallback, or report unavailable, once per AppState lifetime; this is field observability for the OS-level automation gap, not a claim of end-to-end crash-relaunch coverage
  • app resolver and bridge suites — 20 tests passed
  • daemon PATH-resolution suite — 4 tests passed
  • scripts/swift-safe build
  • swiftlint --strict

Known baseline failures

The fenced full suite was run and reported nine issues, including one known quarantined issue. The remaining assertions reproduce in unchanged Markdown URL-policy, scratchpad base-path, and transcript-estimator tests.

Scope

PATH is the sole authority for tmux discovery before its explicit saved fallback. This PR does not run a login shell, invoke path_helper, persist a separate environment file, or search standard package-manager locations.

@cheapsteak cheapsteak left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 🙏 🙏 (Did you restart your computer? XD )

@Jephuff
Jephuff force-pushed the preserve-launch-path branch from d3af207 to df06d54 Compare August 11, 2026 15:09
@Jephuff
Jephuff force-pushed the preserve-launch-path branch from df06d54 to c06ce42 Compare August 11, 2026 19:28
@Jephuff
Jephuff marked this pull request as ready for review August 11, 2026 21:12
@Jephuff

Jephuff commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator Author

Noo, the other two PRs caused random crashes, and when that happened it would restore without tmux :D I guess a restart would be the same!

@Jephuff
Jephuff enabled auto-merge (squash) August 11, 2026 21:12
@Jephuff
Jephuff disabled auto-merge August 11, 2026 21:27
@Jephuff
Jephuff enabled auto-merge (squash) August 11, 2026 21:27
@tbd-claude-reviewer

This comment has been minimized.

@tbd-claude-reviewer

This comment has been minimized.

@tbd-claude-reviewer

This comment has been minimized.

@tbd-claude-reviewer

This comment has been minimized.

@tbd-claude-reviewer

This comment has been minimized.

@tbd-claude-reviewer

Copy link
Copy Markdown

✅ Looks good — no High or Medium severity findings.

Both review lenses came back clean:

  • Correctness (bugs, edge cases, error handling, test coverage claims): no findings. This PR is guard/safety-shaped (it changes tmux executable discovery and touches control-mode version gating), so the specialist ran a full premise audit against the PR description's factual claims about existing code — daemon PATH inheritance from the app, GitManager's fixed /usr/bin/git path, the tmux-only saved-executable fallback, and the removal of daemon PATH augmentation. All four were verified against the diff and surrounding code with file:line evidence; none were refuted. Interception completeness (every tmux invocation site routes through the new resolver), fail-direction analysis (PATH-miss + missing fallback fails closed, not open), the saved-fallback staleness/proxy-signal lifecycle, and the claims-vs-tests reconciliation (the PR is explicit that its tests cover the plist-generation helper, not an OS-level crash/relaunch end-to-end, and no other part of the PR oversells this) all checked out.
  • Conventions (CLAUDE.md compliance): no findings. This is a bug fix (PATH lost on LaunchServices relaunch) rather than new autonomous/destructive behavior, so the default-off-flag rule doesn't apply, and the control-mode version-check short-circuit added in this PR preserves the existing gate's behavior exactly (tested both ways). No TUI screen-scraping, no leaked private/machine-specific context, no DB migration in scope, and the new design spec (docs/specs/2026-08-11-tmux-executable-resolution-design.md) substantively documents the design decisions in the diff, including the resolution order, fallback precedence, and the gate short-circuit — so nothing here reads as an undocumented compiled theory.

No invalid or persnickety feedback was filtered out during merge (both specialists returned empty findings arrays, so there was nothing to dedupe or drop).

Finding dispositions

No findings were reported by either specialist, so there are no dispositions to record.

Review diagnostics

No tool calls failed or were denied for either specialist, as far as reported back to the orchestrator. The correctness specialist recorded a detailed premise-audit notes file (correctness-notes.md) confirming .git/shallow contains only the pinned merge-base commit; it judged this boundary irrelevant since its conclusions concern the diff and current file state rather than history beyond that point. Both specialists confirmed the pinned-SHA diff (git diff a70f942b54d193175dbfdf02e44985ae19c1454a HEAD) resolved without error, and neither reported the infrastructure_failure fallback. The orchestrator's own specialist launches initially returned as background acknowledgments rather than foreground output; a file-existence poll was used instead to confirm completion, with no effect on review quality.

Posted by the claude-review check — the review of this PR's diff at patch-id c373b5a375b1ad55769757d1c7e1aa987a9cfa5e. A newer review comment supersedes this one.

@Jephuff
Jephuff merged commit 56fa912 into main Aug 12, 2026
5 checks passed
@Jephuff
Jephuff deleted the preserve-launch-path branch August 12, 2026 03:18
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.

2 participants