Skip to content

fix(shell-env): strip prompt noise and normalize CLAUDE_CONFIG_DIR#1676

Merged
arnestrickmann merged 2 commits intomainfrom
emdash/feat-user-release-prs-13d
Apr 6, 2026
Merged

fix(shell-env): strip prompt noise and normalize CLAUDE_CONFIG_DIR#1676
arnestrickmann merged 2 commits intomainfrom
emdash/feat-user-release-prs-13d

Conversation

@arnestrickmann
Copy link
Copy Markdown
Contributor

@arnestrickmann arnestrickmann commented Apr 6, 2026

Summary

Harden shell-sourced environment detection: isolate printenv output with explicit markers, strip ANSI/OSC noise (e.g. iTerm prompt sequences), and normalize CLAUDE_CONFIG_DIR so only sensible absolute paths are used—including ~/ expansion and dropping relative values.

Changes

  • getShellEnvVar: Wraps values with __EMDASH_SHELL_VALUE_START__ / __EMDASH_SHELL_VALUE_END__, runs stripAnsi (OSC/bell/other escapes), then extracts the substring between markers so prompt noise cannot corrupt SSH_AUTH_SOCK, CLAUDE_CONFIG_DIR, etc.
  • normalizeClaudeConfigDir: Trims input, expands ~ and ~/… via os.homedir(), rejects non-absolute paths, normalizes with path.normalize.
  • initializeShellEnvironment: Applies normalization to existing and shell-detected CLAUDE_CONFIG_DIR; clears invalid/relative values instead of forwarding them.
  • ptyManager: When passing CLAUDE_CONFIG_DIR through the agent env allowlist for direct spawn and SSH PTYs, uses the same normalization.

Tests

  • Mocks updated for the new marker-based shell output format.
  • Coverage for OSC escape noise around values, dropping relative CLAUDE_CONFIG_DIR, and expanding ~/.… from the shell.

Summary by CodeRabbit

  • Bug Fixes

    • Improved propagation of authentication-related environment variables into terminal sessions.
    • Enhanced handling of the configuration directory: trims, expands ~ to home, rejects relative paths, and omits invalid values.
    • Hardened shell output parsing to ignore terminal escape sequences and reliably extract values.
  • Tests

    • Expanded and updated tests for shell environment parsing, auth-socket discovery, and config-dir normalization.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Apr 6, 2026 2:26pm

Request Review

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 6, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f1b8339d-3fb4-4e90-8918-71e5abf599a8

📥 Commits

Reviewing files that changed from the base of the PR and between d67be83 and a0910ef.

📒 Files selected for processing (1)
  • src/main/utils/__tests__/shellEnv.test.ts

📝 Walkthrough

Walkthrough

Updated shell environment handling: marker-wrapped and ANSI-cleaned extraction of shell variables, new normalizeClaudeConfigDir() to expand ~ and reject relative paths, and passthrough of agent auth env plus normalized CLAUDE_CONFIG_DIR into spawned PTY environments.

Changes

Cohort / File(s) Summary
Shell env utilities
src/main/utils/shellEnv.ts
getShellEnvVar() now wraps shell output with sentinel markers and strips ANSI/OSC/control sequences before extracting values; added normalizeClaudeConfigDir(value) to expand ~ and validate absolute paths; initializeShellEnvironment() uses normalization and removes invalid CLAUDE_CONFIG_DIR.
PTY spawn integration
src/main/services/ptyManager.ts
startSshPty() and startDirectPty() now read and forward agent auth environment variables from process.env (preserving falsy-skipping behavior) and apply normalizeClaudeConfigDir() before setting CLAUDE_CONFIG_DIR in the PTY env.
Tests — shell env behavior
src/main/utils/__tests__/shellEnv.test.ts, src/test/main/shellEnv.test.ts
Replaced raw printenv mocks with shellValue() sentinel-wrapped outputs; added tests for ANSI/OSC noise tolerance; added cases for CLAUDE_CONFIG_DIR handling (reject relative, expand ~, drop empty); adjusted mocks for SSH auth socket detection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I nibble markers in the shell’s bright stream,
I hop through tildes, making paths a dream,
I chase away the noisy ANSI light,
I carry auth and settings into the night,
A tiny rabbit, proud—deploying things just right. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main objectives of the pull request: stripping prompt noise from shell environment detection and normalizing CLAUDE_CONFIG_DIR handling. It is concise, specific, and clearly identifies the primary changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch emdash/feat-user-release-prs-13d

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@arnestrickmann arnestrickmann merged commit 6701eb0 into main Apr 6, 2026
4 checks passed
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