smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/farm/d3b4991c/fix-windows-env-colon-vars'

Browse Source
This commit is contained in:
can1357
2026-05-15 07:05:42 +02:00
parent 7af87f773d 737288676c
commit d4f9b5d497
3 changed files with 112 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pi-shell/src/shell.rs
+103 -50
View File
@@ -538,6 +538,7 @@ async fn create_session(config: &ShellConfig) -> Result<ShellSessionCore> {
.map_err(|err| Error::msg(format!("Failed to set env: {err}")))?;
}
}
apply_env_fallback(&mut shell)?;
#[cfg(windows)]
configure_windows_path(&mut shell)?;
@@ -578,31 +579,7 @@ async fn run_shell_command(
.map_err(|err| Error::msg(format!("Failed to set cwd: {err}")))?;
}
let mut env_scope_pushed = false;
if let Some(env) = options.env.as_ref() {
session
.shell
.env_mut()
.push_scope(EnvironmentScope::Command);
env_scope_pushed = true;
for (key, value) in env {
let normalized_key = normalize_env_key(key);
if should_skip_env_var(normalized_key) {
continue;
}
let mut var = ShellVariable::new(ShellValue::String(value.clone()));
var.export();
if let Err(err) =
session
.shell
.env_mut()
.add(normalized_key, var, EnvironmentScope::Command)
{
let _ = session.shell.env_mut().pop_scope(EnvironmentScope::Command);
return Err(Error::msg(format!("Failed to set env: {err}")));
}
}
}
let env_scope_pushed = apply_command_env(&mut session.shell, options.env.as_ref())?;
let minimizer_mode = if let Some(config) = options.minimizer.as_ref() {
minimizer::engine::mode_for(&options.command, config)
@@ -824,31 +801,7 @@ async fn run_shell_command_streams(
.map_err(|err| Error::msg(format!("Failed to set cwd: {err}")))?;
}
let mut env_scope_pushed = false;
if let Some(env) = options.env.as_ref() {
session
.shell
.env_mut()
.push_scope(EnvironmentScope::Command);
env_scope_pushed = true;
for (key, value) in env {
let normalized_key = normalize_env_key(key);
if should_skip_env_var(normalized_key) {
continue;
}
let mut var = ShellVariable::new(ShellValue::String(value.clone()));
var.export();
if let Err(err) =
session
.shell
.env_mut()
.add(normalized_key, var, EnvironmentScope::Command)
{
let _ = session.shell.env_mut().pop_scope(EnvironmentScope::Command);
return Err(Error::msg(format!("Failed to set env: {err}")));
}
}
}
let env_scope_pushed = apply_command_env(&mut session.shell, options.env.as_ref())?;
let (stdout_reader, stdout_writer) = pipe_to_files("stdout")?;
let (stderr_reader, stderr_writer) = pipe_to_files("stderr")?;
@@ -1087,6 +1040,56 @@ fn terminate_background_jobs(shell: &BrushShell) {
targets.signal(process::KILL_SIGNAL);
});
}
/// Apply per-command environment variables onto a freshly pushed
/// `Command` scope. Returns `true` when a scope was pushed (so the caller
/// can pop it after the command runs), `false` when there were no vars and
/// the existing scopes remain untouched.
fn apply_command_env(shell: &mut BrushShell, env: Option<&HashMap<String, String>>) -> Result<bool> {
let Some(env) = env else {
return Ok(false);
};
shell.env_mut().push_scope(EnvironmentScope::Command);
for (key, value) in env {
let normalized_key = normalize_env_key(key);
if should_skip_env_var(normalized_key) {
continue;
}
let mut var = ShellVariable::new(ShellValue::String(value.clone()));
var.export();
if let Err(err) = shell
.env_mut()
.add(normalized_key, var, EnvironmentScope::Command)
{
let _ = shell.env_mut().pop_scope(EnvironmentScope::Command);
return Err(Error::msg(format!("Failed to set env: {err}")));
}
}
Ok(true)
}
/// Define `env` as a shell variable expanding to the literal `$env` so that
/// brush-core's POSIX parameter expansion preserves PowerShell-style
/// `$env:NAME` references when commands are dispatched through brush to a
/// PowerShell (or any) subprocess. The variable is not exported, so it only
/// influences brush's own expansion; the child process environment is
/// unaffected.
///
/// User-driven assignments (`env=prod; echo "$env:8080"`) push their own
/// binding in the command scope and shadow this global default, preserving
/// the bash POSIX contract for callers that genuinely use a variable named
/// `env`.
fn apply_env_fallback(shell: &mut BrushShell) -> Result<()> {
if shell.env().get("env").is_some() {
return Ok(());
}
let var = ShellVariable::new(ShellValue::String("$env".to_string()));
shell
.env_mut()
.set_global("env", var)
.map_err(|err| Error::msg(format!("Failed to set env fallback: {err}")))
}
fn should_skip_env_var(key: &str) -> bool {
if key.starts_with("BASH_FUNC_") && key.ends_with("%%") {
return true;
@@ -1880,4 +1883,54 @@ mod tests {
.expect("execute should succeed");
assert_eq!(result.exit_code, Some(0));
}
/// Brush expands `$env:NAME` against the `env` shell variable by default,
/// collapsing PowerShell references like `Write-Host $env:OMPCODE` to
/// `:OMPCODE`. The session-level fallback below defines `env=$env` so the
/// expansion is the literal `$env:OMPCODE`, preserving the PowerShell
/// token when the command is forwarded to a child shell.
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread")]
async fn powershell_env_reference_survives_brush_expansion() {
let (tx, mut rx) = mpsc::unbounded_channel::<Bytes>();
let options = ShellExecuteOptions {
command: "printf '%s' \"$env:SystemRoot\"".to_string(),
..Default::default()
};
let streams = StreamSinks { stdout: Some(tx), stderr: None };
let result = execute_shell_streams(options, streams, CancelToken::default())
.await
.expect("execute should succeed");
assert_eq!(result.exit_code, Some(0));
let mut stdout = Vec::new();
while let Some(chunk) = rx.recv().await {
stdout.extend_from_slice(&chunk);
}
assert_eq!(stdout, b"$env:SystemRoot");
}
/// A user assignment to `env` in the command itself must shadow the
/// session-level fallback so callers that genuinely use a POSIX variable
/// named `env` see their value, not the literal `$env`.
#[cfg(unix)]
#[tokio::test(flavor = "multi_thread")]
async fn user_env_assignment_shadows_powershell_fallback() {
let (tx, mut rx) = mpsc::unbounded_channel::<Bytes>();
let options = ShellExecuteOptions {
command: "env=prod; printf '%s' \"$env:8080\"".to_string(),
..Default::default()
};
let streams = StreamSinks { stdout: Some(tx), stderr: None };
let result = execute_shell_streams(options, streams, CancelToken::default())
.await
.expect("execute should succeed");
assert_eq!(result.exit_code, Some(0));
let mut stdout = Vec::new();
while let Some(chunk) = rx.recv().await {
stdout.extend_from_slice(&chunk);
}
assert_eq!(stdout, b"prod:8080");
}
}
packages/coding-agent/CHANGELOG.md
+5
View File
@@ -32,6 +32,11 @@
- Fixed startup fallback-chain validation to recognize cached runtime-discovered standard provider models, including Ollama Cloud models listed by `--list-models`, so `retry.fallbackChains` no longer warns that valid `ollama-cloud/<model>` selectors are unknown. ([#1052](https://github.com/can1357/oh-my-pi/issues/1052))
- Fixed `discoverAgents()` ignoring `disabledProviders` for the `claude-plugins` provider. Plugin roots from `~/.claude/plugins/` were scanned unconditionally, so agents from Claude Code marketplace plugins continued to appear in `/agents` and the Agent Control Center even when `disabledProviders: [claude-plugins]` was set. The discovery path now checks `isProviderEnabled("claude-plugins")` before calling `listClaudePluginRoots()`, matching how every other capability respects the disabled-providers set. ([#1075](https://github.com/can1357/oh-my-pi/issues/1075))
### Fixed
- Fixed `$env:VAR` PowerShell variables being mangled on Windows when commands invoked PowerShell as a subprocess (e.g. `powershell -Command "Write-Host $env:SystemRoot"`). Brush-core applied POSIX parameter expansion to `$env` before spawning the child, leaving a dangling `:NAME`. The fix lives in `pi-shell` at env-var application time: every brush session now defines `env=$env` as an internal shell variable so `$env:NAME` expands to the literal `$env:NAME` token that PowerShell expects. The fallback is not exported, only influences brush's own expansion, and is shadowed by any user assignment to `env` (e.g. `env=prod; echo "$env:8080"` still prints `prod:8080`), so the POSIX bash contract is preserved. ([#1079](https://github.com/can1357/oh-my-pi/issues/1079))
## [15.0.1] - 2026-05-14
### Breaking Changes
packages/natives/CHANGELOG.md
+4
View File
@@ -11,6 +11,10 @@
- Fixed `<sym> is not a function` crashes on Windows after `bun install -g @oh-my-pi/pi-coding-agent` updates while an `omp` process was running. Bun cannot overwrite a locked `node_modules/@oh-my-pi/pi-natives/native/pi_natives.win32-x64.node` and silently keeps the old binary alongside the new ESM wrapper, so the next launch loads mismatched code. The loader now mirrors the addon into `~/.omp/natives/<version>/` on Windows npm installs and prefers that copy at load time — each version gets its own filesystem path, so future updates land in `node_modules` unchallenged. The new version sentinel detects any remaining drift up front.
### Fixed
- Fixed `$env:NAME` PowerShell references being collapsed to `:NAME` when brush forwarded a command to a PowerShell (or any) subprocess. `pi-shell` now defines `env=$env` as a non-exported global on every brush session so the bash parameter expansion of `$env` yields the literal `$env`, leaving `$env:NAME` intact. User-driven assignments (`env=prod`) push their own command-scope binding and shadow the fallback, preserving the bash POSIX contract. ([#1079](https://github.com/can1357/oh-my-pi/issues/1079))
## [15.0.1] - 2026-05-14
### Breaking Changes