fix(interpreter): prefix env assignments visible to commands#200
fix(interpreter): prefix env assignments visible to commands#200
Conversation
In bash, `VAR=value command` temporarily sets VAR in the command's environment. bashkit was storing prefix assignments in self.variables but not self.env, so builtins like printenv couldn't see them. Fix: temporarily inject prefix assignments into self.env for the duration of command execution, then restore. Also ensure prefix assignments don't persist in shell variables after the command. Extracted dispatch code into execute_dispatched_command to enable single-point save/restore around the call. https://claude.ai/code/session_01HNrHwsipGrStdSYThP77Rk
…docs - Add 6 spec tests in variables.test.sh (positive + negative) - Add 6 differential edge cases in proptest_differential.rs - Add proptest strategy + property test for prefix assignments - Add 5 unit tests in lib.rs (empty value, multiple vars, persistence) - Update 005-builtins.md: document prefix env assignment behavior - Update 009-implementation-status.md: test counts, partially impl note - Update compatibility.md: prefix env assignment status table https://claude.ai/code/session_01HNrHwsipGrStdSYThP77Rk
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 318c83981a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (name, old) in var_saves { | ||
| match old { | ||
| Some(v) => { | ||
| self.variables.insert(name, v); | ||
| } | ||
| None => { | ||
| self.variables.remove(&name); |
There was a problem hiding this comment.
This unconditional rollback reverts any variable updates the executed command makes to names that also appeared in prefix assignments. For example, X=tmp export X=perm (or X=tmp readonly X) should leave X set after the builtin runs, but this loop restores/removes X from var_saves, so the builtin’s write is lost. The regression happens whenever a builtin/function mutates a prefixed variable name, and it diverges from bash behavior plus the export builtin’s own contract of setting variables.
Useful? React with 👍 / 👎.
2 participants
Summary
VAR=value commandnow temporarily injects VAR into the command's environment, matching bash behavior. Previously prefix assignments were stored in shell variables only, invisible to builtins likeprintenv.execute_dispatched_commandmethod for clean save/restore around command execution.Test plan
lib.rs(positive: visible in env, multiple vars, empty value; negative: not persistent, not found without prefix)variables.test.shcovering positive, negative, and edge casesproptest_differential.rscargo fmt --checkcleancargo clippy --all-targets --all-features -- -D warningscleancargo test --all-featuresall pass (934+ unit, 573 spec, 53 proptest)