Skip to content

fix: stabilize PR272 new-session UX, profiles gating, and env safety #388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
leeroybrun wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

fix: stabilize PR272 new-session UX, profiles gating, and env safety #388

leeroybrun wants to merge 20 commits into from
+10,391 −7,573

Conversation

@leeroybrun
Copy link
Contributor

@leeroybrun leeroybrun commented Jan 17, 2026

Context / Goal

This PR is a follow-up to slopus/happy#272, which introduced a large new-session + profiles + env-var/settings/sync surface.
The goal here is to keep the feature work, but restore the pre-PR272 “standard New Session” UX and make the new profiles/env work coherent + safe.

I’m trying to keep this factual (no blame): this is mostly stabilization work after a very large merge.

Summary (what this PR does)

  • Restores the pre‑PR272 New Session UX: /new is modal again, prompt-first layout is back, and the compact chip-based controls are restored (placeholder + keyboard behavior aligned with pre-merge).
    • Makes PR272’s new session work truly optional and coherent:
      • Separate settings for Profiles vs Enhanced Wizard (can enable either independently).
      • Machine/path search in pickers is opt-in via feature settings.
    • Integrates Profiles into the standard New Session modal (when enabled):
      • Profile selection chip + picker.
      • Env var preview chip + modal.
      • Clear UX when AI backend is locked by the selected profile (no “dead click”).
    • Hardens Profiles correctness + navigation safety:
      • Prevents persisting invalid profiles (e.g. id: ''), uses id-based routing, removes callback/URL-serialized JSON edit flow, and avoids unmount/
        deeplink fragility.
      • Adds profile grouping/favorites behavior in a consistent way.
    • Fixes env var resolution + previews with safer behavior:
      • Removes the global “fetch all ${VAR} across all profiles” behavior in /new.
      • Env previews use a new secure daemon RPC call, avoid querying secret-like vars, and preserve correctness for ${VAR} / ${VAR:-fallback} templates.
      • Remote fetch parsing is more robust (JSON protocol when available, safe fallback parsing; preserves empty-string vs unset).
    • Improves settings + sync robustness introduced/made risky by PR272’s expanded settings surface:
      • Settings parsing is tolerant (one bad profile field no longer nukes all settings).
      • Logging avoids dumping decrypted settings / sensitive payloads; noisy detection logs gated to DEV.
      • Tool parsing/normalization is hardened so structured tool outputs don’t crash/drop messages.
    • Removes dead/duplicative PR272 additions (e.g. placeholder sync code and unused wizard component) and reduces duplication by reusing existing/shared UI primitives.
    • i18n cleanup: separates translation types vs content and fixes lingering untranslated strings in several locales.

What PR272 introduced (and what we’re fixing here)

Profiles/editor correctness

  • Profile edit screen could create/persist an invalid profile with id: ''.
    • Fixed via id-based navigation + UUID-backed “empty profile” creation

Navigation coherence (web + native + unmount safety)

  • Hybrid approach (module-level callbacks + URL-serialized JSON) was used for profile editing, while other pickers used params.
  • Fixed by switching profile edit to id-based routing and removing the callback/URL-JSON pattern.

Global daemon env fetch (and secret exposure risk)

  • Was using a global envVarRefs/daemonEnv fetch in /new that queried the daemon for all ${VAR} references across all profiles (including secrets).
  • This can pull secret values into JS memory unnecessarily (even if only used for small previews).
  • Fixed by removing the global query and keeping env resolution scoped with secret-like filtering in preview paths.

New Session (standard flow) regressions

  • New Session was effectively treated as a screen rather than the prior modal presentation.
  • The prompt-first input area was changed to show an oversized “context card” (machine/path), losing the compact chip UI.
  • Placeholder/keyboard offset behavior drifted from the pre-merge UX.

Wizard + pickers inconsistent with existing UI

  • Wizard sections (working directory, permission mode) and pickers (machine/path) used new layouts/styles that didn’t match existing Happy patterns and duplicated logic.
  • Pickers pass selections back via expo-router params (no react-navigation SET_PARAMS dispatch).

Profiles “leaked” into default behavior

  • Profiles & env vars could affect session spawning, even when they were disabled in the settings
  • Profiles management UI diverged from existing settings layout conventions.

Non-UI safety/reliability issues (PR272 logic surface)

  • Sensitive settings/logging risk: decrypted settings can now include profile env vars / keys; existing logs became unsafe.
  • Tool result dropping risk: PR272’s message normalization could reject tool results when output is structured JSON (common for Codex/Gemini), causing messages to be dropped.
  • Settings parse fragility: “whole settings parse fails → defaults” becomes much more dangerous once settings include user-edited profiles/env vars.
  • New env-var querying hooks had edge cases (whitespace trimming, unset vs empty ambiguity).
  • Dead/placeholder code added (e.g. profileSync.ts, `NewSessionWizard) that wasn’t actually wired.

Even if Codex was detected on the machines, the Codex profiles stayed disabled

  • We can even see it in the PR 272 demo itself:
    IMAGE

What we implemented to fix/improve it (what’s in this branch)

1) New Session UX restored (pre-PR272 feel, still compatible with new features)

  • Restored modal presentation for New Session.
  • Restored prompt-first “standard” flow layout and compact chips.
  • Restored original translated placeholder (t('session.inputPlaceholder')) and legacy keyboard behavior.

2) Wizard + pickers refactored to reduce duplication and improve coherence

  • Extracted reusable selectors and used them in both:
    • wizard flow
    • modal pickers

3) Profiles made truly optional (separate feature flag) + coherent selection UX

  • Introduced independent useProfiles flag (separate from useEnhancedSessionWizard).
  • Gated:
    • profiles settings entry
    • profiles UI in New Session (standard and wizard)
    • profile selection/persistence logic
    • profile env var injection
  • Added a lightweight profile picker modal for standard flow (chip opens picker; no forced wizard).

4) Profile identity persisted/displayed safely

  • Sessions now persist profileId and show a read-only chip in session UI.
  • Handles missing/unknown profile IDs gracefully.

Non-UI “logic hardening” improvements tied to PR272’s new surface area

Most changes are in the PR272-touched surface area, with a few adjacent safety/robustness fixes (auth storage hardening, app config fallback, and i18n cleanup).

A) Prevent leaking secrets via logs

  • Removed/limited logging of full decrypted settings (can contain API keys / profile env vars).
  • Avoid logging raw full message payloads on validation errors.
  • Gated noisy CLI detection logs to __DEV__.

B) Fix tool-result robustness (don’t drop messages; don’t break on structured outputs)

  • Tool results can be strings or structured objects (esp. Codex/Gemini). PR272’s schema/normalization could reject these, dropping messages.
  • Updated tool-result parsing/normalization so non-string outputs are accepted and safely stringified for display.

C) Make settings parsing tolerant (avoid “one bad profile nukes all settings”)

  • PR272 expanded settings and added profiles/env-var editing. A single invalid field should not reset known settings to defaults.
  • Updated settingsParse() to parse fields individually and filter invalid profile entries rather than failing the entire settings object.

D) Unify env-var template parsing + preserve correctness in remote env querying

  • Standardized supported template syntax to ${VAR} and ${VAR:-default} consistently.
  • Improved env var fetching:
    • avoid trimming stdout (which can corrupt values)
    • distinguish unset vs empty string using a JSON protocol via node when available (fallback remains safe)
    • fixed a TS template-literal escape bug in fallback

G) Env preview RPC + parsing (new)

  • Preview env values are fetched via a new secure daemon preview flow (scoped to the user’s current selection, not a global ‘all profiles’ query).
  • Parsing is resilient: prefers JSON protocol output when available and falls back to a line parser if JSON is malformed/noisy.
  • Preserves correctness for unset vs empty-string values; avoids trimming stdout to prevent corrupting values.
  • Filters secret-like variable names (token/key/secret/auth/password/pass/cookie) to avoid remote querying/display.

E) Remove unused placeholder sync service & component added by PR272

  • profileSync.ts & components/NewSessionWizard.tsx were added but not referenced; removed to prevent future confusion.

F) Conservative security: avoid querying remote values for secret-like vars

  • Strengthened secret heuristics to include PASS|PASSWORD|COOKIE in addition to token/key/secret/auth.
  • This prevents accidental remote querying/display of sensitive env vars.

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

  1. /new is no longer presented as a modal (becomes a normal screen)
  1. Prompt placeholder is hardcoded (loses i18n key)
  1. Profiles/env injection is on the default spawn path (no opt-in gate)
  1. Global daemon env fetch in /new queries ${VAR} refs across all profiles (no secret filtering)
  1. Profile editor env-var cards tell you to “select a machine”, but Profiles settings editor has no machine picker
  1. Secret-handling messaging mismatch
  1. Profile editor route can create a new profile with id: ''
  1. Profile edit navigation uses URL-serialized JSON payloads + module-level callbacks (unmount/deeplink fragile)
  1. Startup Bash Script is stored/edited, but not part of spawn RPC (unwired)
  1. CLI detection + profile availability gating can false-negative Codex/Gemini
  1. Tool-result normalization can drop structured tool outputs
  1. Settings parsing is “whole-object” safeParse (invalid settings fall back to defaults for all known fields)
  1. useEnvironmentVariables trims stdout + collapses empty-string to null
  1. profileSync.ts is effectively a placeholder (TODOs for actual daemon RPCs)
  1. tmux “empty sessionName means current/most recent” is stated in UI
  1. i18n English duplication (“dedicated en file” + tooling imports it)
  1. Large wizard component introduced but unused

@leeroybrun leeroybrun force-pushed the slopus/pr/pr272-followup-fixes branch from ac56180 to 5f4ee63 Compare January 17, 2026 13:31
@leeroybrun
fix(auth): harden tokenStorage web persistence
a2c26be
@leeroybrun
feat(sync): add permission mode types and mapping
438b221
@leeroybrun
feat(persistence): scope storage and validate draft modes
8d235e7
@leeroybrun
fix(session): clamp configurable model modes
fabb336
@leeroybrun
fix(tools): harden ACP tool parsing and titles
b7b412a
@leeroybrun
refactor(sync): centralize outgoing message metadata
8d1d69f
@leeroybrun
feat(storage): persist session model modes
bb67dfe
@leeroybrun
fix(settings): make parsing tolerant for profiles
01ec9a6
@leeroybrun
fix(dev): gate CLI detection logging
40ac8f7
@leeroybrun
fix(command-palette): include navigate dependency
48e6253
@leeroybrun
feat(env): add env var template parsing
c902b17
@leeroybrun
fix(env): improve remote env resolution and previews
a9e2a6e
@leeroybrun
feat(env): update env var list, cards, and preview modal
0f3a374
@leeroybrun
refactor(i18n): separate translation types and content
08b7937
@leeroybrun
fix(new-session): restore standard modal flow
5ed6ddc
@leeroybrun
fix(profiles): harden routing, grouping, and editing
e203dcc
@leeroybrun leeroybrun force-pushed the slopus/pr/pr272-followup-fixes branch from 5f4ee63 to f288210 Compare January 17, 2026 13:41
@leeroybrun leeroybrun force-pushed the slopus/pr/pr272-followup-fixes branch from f288210 to f999a1f Compare January 17, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@leeroybrun