Skip to content

Conversation

@leeroybrun
Copy link
Contributor

@leeroybrun leeroybrun commented Jan 17, 2026

Context / Goal

This PR is a follow-up to slopus/happy-cli#107 (profiles/env expansion + tmux + runtime/offline changes).
The goal is to make tmux spawning correct and deterministic, align the spawn RPC contract with reality, restore reliable offline buffering, and harden logging.

Summary (what this PR does)

  • Fixes tmux env injection (no value mutation), makes empty session selection deterministic, and properly wires TMUX_TMPDIR for tmux client invocations.
  • Restores offline buffering for socket sends (removes PR107 early-return drops).
  • Makes Bun subprocess spawning coherent and consistent across normal spawn and tmux spawn.
  • Updates the spawn RPC typing to accept arbitrary env var maps (what the GUI actually sends).
  • Tightens DEBUG gating/redaction for logging and makes offline stubs safer.
  • Adds a new RPC: preview-env (daemon-side env preview with secrets policy) so the UI can safely preview “daemon-effective” env without shelling out to machine bash, and enforcing secrets redaction daemon-side.
  • Adds a new RPC: detect-cli (daemon-side CLI detection) so the UI can determine whether claude / codex / gemini are resolvable on the daemon’s PATH reliably without relying on a shell.

What #107 introduced (and what we’re fixing here)

tmux correctness

  • PR107 passed tmux -e vars as KEY="escapedValue" (tmux treats quotes/backslashes literally), corrupting env values.
  • PR107 treated empty/undefined session name as “pick first session from list-sessions” (nondeterministic).
  • PR107 treated exitCode=null (killed by signal) as success (0).
  • PR107 tmux $TMUX parsing was wrong (treats server pid as “session.window”)
  • PR107 tmux command string built via args.join(' ') (breaks quoting/spacing)
  • TMUX_TMPDIR was exposed/mapped in the UI, but in PR107 tmux client invocation/socket selection didn’t honor it (contract gap / no-op for selecting a non-default tmux server).

TMUX_TMPDIR contract gap

  • PR107 exposed TMUX_TMPDIR in profile/env plumbing, but tmux client command execution did not actually honor it.
    Fixed by passing TMUX_TMPDIR into the tmux subprocess env and plumbing it through tmux utilities.

Offline behavior (“don’t send if disconnected”)

  • Pre-PR107 code logged “Message will be lost” but still emitted.
  • PR107 introduced early returns in send paths (complete message loss).
    Fixed by removing the forced return, as Socket.IO will buffers the messages in case of deconnections and will send them on reconnect

CLI detection was flaky

  • PR107 CLI detection was flaky
    Fixed by adding a new RPC: detect-cli (daemon-side CLI detection) so the UI can determine whether claude / codex / gemini are resolvable on the daemon’s PATH reliably without relying on a shell.

Bun runtime support

  • PR107 auto-selected bun in some cases but still used Node-only invocation patterns, and tmux spawning hardcoded node.

Provider config objects (ghost overrides / leftovers?)

RPC typing drift

  • PR107 typed environmentVariables as a narrow object with a few keys, but in practice the GUI sends arbitrary env maps.

What this series fixes / improves

New RPC: preview-env (safe daemon-effective env preview)

  • Problem: the UI needs to preview environment variables exactly as the daemon will actually apply them, but calling a “machine bash” RPC is unsafe and can differ from daemon-applied env.
    • Concretely: “machine bash” spawns a brand new process with its own init/login semantics, so values like PATH can differ (and it can never reliably reflect the daemon’s current process.env).
  • Solution: add a new session-scoped RPC handler preview-env that returns the daemon-effective env values for a requested set of keys, using the same expansion logic used at spawn time (single source of truth) with a secrets-visibility policy applied.
    • This removes the previous “duplicated env preview logic” where the UI tried to approximate daemon behavior with template parsing + remote shell probing.
  • Protection / policy: controlled by HAPPY_ENV_PREVIEW_SECRETS, so the user can actually set on the daemon-side the secrets protection policy he wants to enforce.
    • none: do not return sensitive values (they are hidden) - default
    • redacted: return redacted placeholders for sensitive values
    • full: return full sensitive values (intended only for trusted debugging)
  • Enforced sensitivity: the daemon always applies its own secret-name heuristic (TOKEN/KEY/SECRET/AUTH/PASS/PASSWORD/COOKIE), and returns metadata in the response so the UI can show “forced secret” state consistently.
    • Optional override on daemon-side for advanced users: HAPPY_ENV_PREVIEW_SECRET_NAME_REGEX (string regex pattern). If invalid/empty, daemon fails closed to the default heuristic.
  • Fallback: The UI still fallsback to shell commands for env preview when preview-env is not available (older daemons).

1) Stop unintended profile application from CLI settings

  • Problem: daemon implicitly fell back to settings.activeProfileId when GUI didn’t provide env vars, meaning profiles could affect sessions even when the GUI profiles feature was disabled or not used by users (in "standard" non-wizard new session modal, when wizard was disabled in settings).
  • Fix: profile env vars are only applied when explicitly provided in SpawnSessionOptions.environmentVariables.

2) Persist profile identity per session (cross-device visibility)

  • Problem: without persistence, a session started with a profile on machine A couldn’t reliably show “which profile” on machine B.
  • Fix: thread profileId?: string | null through spawn → daemon → child env (HAPPY_SESSION_PROFILE_ID) → session metadata (non-secret, used for display/debugging).

3) Align CLI profile schema + env template support

  • Fixes:
    • Accept URL-or-template strings for provider endpoints (e.g. ${VAR} / ${VAR:-default})
    • Accept non-UUID ids for built-in profiles (string min(1))
    • Support ${VAR:=default} expansion (CLI env-template parity)

4) Prevent secret leakage in daemon + RPC logs

  • Problem: logging spawn params/options could leak tokens and env var values into ~/.happy/logs/*.
  • Fix:
    • Log only env var keys/counts and token presence, never values.
    • The new preview-env RPC also enforces a secrets policy via HAPPY_ENV_PREVIEW_SECRETS to prevent accidental secret exposure when the UI previews env.

5) Prevent secret leakage in happy doctor

  • Problem: doctor printed settings.json verbatim, including profile env var values.
  • Fix: redact/mask secret-like fields; keep ${VAR} templates visible; avoid printing legacy caches.

6) Remove unwired / misleading functionality (strict cleanup)

  • Changes:
    • Removed startupBashScript from the CLI profile schema (it was never executed/passed to spawn by the UI and not wired anywhere in the CLI).
    • Removed localEnvironmentVariables from CLI settings (schema bump + migration deletes legacy key).
    • Removed unused helper APIs that relied on the cache (no call sites existed).

7) Tmux: added optionnal integration tests to validate the real behavior against real tmux process

  • Implemented opt-in “real tmux” integration tests in the happy-cli worktree that exercise our actual tmux-calling code (TmuxUtilities.spawnInTmux) against real tmux processes to validate fixes for PR107 tmux bad assumptions.
  • Run (opt-in) with:
    • HAPPY_CLI_TMUX_INTEGRATION=1 yarn test

Links to the detailled issues in slopus/happyPR107 (links go to the PR diff)

  1. tmux new-window -e env injection corrupts values (quotes/backslashes become literal)
  1. tmux $TMUX parsing is wrong (treats server pid as “session.window”)
  1. tmux “empty session name” selection is “first session from list-sessions” (nondeterministic)
  1. tmux command string built via args.join(' ') (breaks quoting/spacing)
  1. tmux runner treats exitCode=null (killed by signal) as success
  1. TMUX_TMPDIR “contract gap”: profile mapping sets it, but tmux client selection is only via -S socketPath
  1. Daemon falls back to CLI-local activeProfileId when GUI doesn’t supply env vars
  1. localEnvironmentVariables cache stored in settings (can contain secrets)
  1. Profile schema strictness (UUID ids + URL-only endpoints)
  1. expandEnvVars logs expanded values/defaults and uses non-bash empty semantics
  1. Bun runtime selection is incoherent in subprocess spawning
  1. SpawnSessionOptions.environmentVariables type is too narrow vs reality
  1. Deterministic message drop on disconnect for Claude session messages (early return)
  1. Offline stub is a plain object cast to ApiSessionClient

@leeroybrun leeroybrun marked this pull request as ready for review January 17, 2026 11:58
Validation:

- Verified TMUX/TMUX_PANE values inside an isolated tmux server (custom socket)

- Verified kill-window syntax and new-session -t semantics via tmux manpage

Changes:

- Correct TMUX env parsing (socket_path + server_pid) and prefer TMUX_PANE for pane id

- Avoid invalid -t injection for commands that already specify a target; do not treat new-session as targetable

- Fix kill-window invocation to use -t session:window

- Align Machine.metadata typing with runtime (nullable)

- Make socket send behavior consistent when disconnected; tighten misc scripts/tests
Remove the hard send guard that dropped messages while disconnected and rely on socket.io client buffering (tests updated).
Make debugLargeJson a true DEBUG-only path and avoid logging expanded env values/defaults; tighten doctor masking for default templates.
Align happy-cli profile schema with the app: profiles are env-var based only. Migrate any legacy provider config fields into environmentVariables during parsing to avoid data loss.
Add a single runtime invocation builder for node vs bun and allow overriding via HAPPY_CLI_SUBPROCESS_RUNTIME.
The GUI sends a profile-derived env var map that may include provider-specific keys and tmux knobs; type it as Record<string,string>.
@leeroybrun leeroybrun force-pushed the slopus/pr/pr107-followup-fixes branch from ce27c90 to df67085 Compare January 17, 2026 12:05
@leeroybrun leeroybrun marked this pull request as draft January 17, 2026 12:05
Handle codex --version output variations without misreporting 'not installed' and remove stdout logging of elicitation payloads.
Provide an EventEmitter-compatible stub and a focused unit test so offline mode can't crash on basic session events.
- Treat TMUX_TMPDIR as a directory via tmux client env (not -S socket path)

- Pass per-window env via new-window -e KEY=value without shell-style quoting

- Make empty sessionName resolve deterministically (attached/most-recent)

- Preserve failure semantics for signal-terminated tmux commands
@leeroybrun leeroybrun force-pushed the slopus/pr/pr107-followup-fixes branch from df67085 to 0ff0b37 Compare January 17, 2026 15:22
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.

1 participant