Claude/advance project y32 it#16
Merged
Merged
Conversation
Two long-standing correctness holes in shell-function support:
1. Function bodies containing control structures (`if/fi`, `for/done`,
`while/done`, `case/esac`) were fragmented. `parse_function_def`
split the body naively on `;` and `\n`, so `f() { if x; then y; fi; }`
became `["if x", "then y", "fi"]` — three strings that failed to
parse individually. The body is now stored as a raw string between
the outermost braces, with proper brace-depth tracking that respects
single/double quotes. At call time `execute_function_call` uses the
existing control-structure-aware `split_on_semicolons` so a function
body parses the same as any other script fragment.
2. `return` inside nested control structures did nothing. The function
executor detected returns with `cmd_str.starts_with("return")`,
which never matched when `return` was buried inside an `if`, `for`,
or `while`. Introduces `ExecutionResult::Return { exit_code }` — a
sentinel that propagates through every control-structure handler
(`Command::If`, `WhileLoop`, `ForLoop`, `LogicalOp`, `Source`,
`execute_block`) until it reaches `execute_function_call`, which
converts it to a regular exit-code result. The fragile string-match
detection is gone.
Also:
- `tests/function_control_flow_tests.rs`: 8 regression tests covering
if/for/case in function bodies, `return` from nested `if`/`for`, and
a brace-in-quoted-string edge case.
- `tests/security_tests.rs`: `security_no_privilege_escalation` now
skips cleanly when running as root (with an optional
`VSH_ALLOW_ROOT_TESTS=1` override) instead of panicking. The test was
a meta-check about the test environment, not correctness, and was
making the suite non-portable to containerised CI.
Test results: 703 passing, 0 failing, 14 ignored (up from 602).
https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
…source
Previously `execute_script_content` and `Command::Source` iterated
`content.lines()` and parsed each line in isolation, so the canonical
POSIX shape
if true
then
mkdir d
fi
shredded into three lines that failed to parse individually. The same
went for `for/do/done`, `while/do/done`, `case/esac`, and multi-line
function bodies.
Changes:
- `parser.rs`: extract `split_on_top_level` from `split_on_semicolons`
and add `split_on_statement_separators`, which additionally treats
top-level `\n` as a statement boundary. It also tracks brace depth
(scoped to the statement-separator variant) so a `;` inside a
function body — `foo() { a; b; }` — does not end the statement.
Normal `${VAR}` expansions balance themselves and do not affect the
depth.
- `parser.rs::parse_command_block`: use the new splitter so control
structure BODIES can span multiple lines too.
- `main.rs::execute_script_content`: strip whole-line comments, then
feed the entire content through the new splitter.
- `executable.rs::Command::Source`: same fix for sourced files.
- `state.rs::next_fifo_id`: switch to a process-wide atomic counter.
The per-state counter started at 0 for every `ShellState`, so
parallel tests with the same PID collided on
`/tmp/vsh-fifo-<pid>-0`, causing `test_fifo_creation` /
`test_fifo_path_unique` to race intermittently.
- `tests/multiline_script_tests.rs`: 9 regression tests — 4 unit tests
for the splitter (newline, quoted newline, multi-line if/fi, function
defs) and 5 end-to-end tests for sourced scripts (multi-line if,
multi-line for, multi-line function defs, comment-only lines, and
mixed single-/multi-line statements).
Test results: 712 passing, 0 failing, 14 ignored (up from 703).
https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
Tilde expansion was previously hard-coded only inside the `cd`
command. It now runs as the first step of `expand_variables`, so every
command that uses variable expansion — `mkdir`, `touch`, `echo`,
`cp`, `mv`, external commands, etc. — gets tilde expansion for free.
Supported forms (per POSIX §2.6.1):
~ → $HOME
~/path → $HOME/path
~+ → $PWD
~+/path → $PWD/path
~- → $OLDPWD
~-/path → $OLDPWD/path
Not expanded inside single or double quotes ('~' / "~" stay literal).
This is enforced by `quoted_word_to_string` escaping `~` → `\~` in
quoted contexts; `expand_tilde` recognises the escape and emits a
literal `~`.
~user (getpwnam lookup) is not yet implemented; the tilde is left
literal in that case.
The `cd`-specific `~/` handling is removed since it's now redundant.
Test results: 721 passing, 0 failing, 14 ignored (up from 712).
https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
hyperpolymath
added a commit
that referenced
this pull request
Apr 27, 2026
* feat(rust-cli): fix function control-flow and return propagation
Two long-standing correctness holes in shell-function support:
1. Function bodies containing control structures (`if/fi`, `for/done`,
`while/done`, `case/esac`) were fragmented. `parse_function_def`
split the body naively on `;` and `\n`, so `f() { if x; then y; fi; }`
became `["if x", "then y", "fi"]` — three strings that failed to
parse individually. The body is now stored as a raw string between
the outermost braces, with proper brace-depth tracking that respects
single/double quotes. At call time `execute_function_call` uses the
existing control-structure-aware `split_on_semicolons` so a function
body parses the same as any other script fragment.
2. `return` inside nested control structures did nothing. The function
executor detected returns with `cmd_str.starts_with("return")`,
which never matched when `return` was buried inside an `if`, `for`,
or `while`. Introduces `ExecutionResult::Return { exit_code }` — a
sentinel that propagates through every control-structure handler
(`Command::If`, `WhileLoop`, `ForLoop`, `LogicalOp`, `Source`,
`execute_block`) until it reaches `execute_function_call`, which
converts it to a regular exit-code result. The fragile string-match
detection is gone.
Also:
- `tests/function_control_flow_tests.rs`: 8 regression tests covering
if/for/case in function bodies, `return` from nested `if`/`for`, and
a brace-in-quoted-string edge case.
- `tests/security_tests.rs`: `security_no_privilege_escalation` now
skips cleanly when running as root (with an optional
`VSH_ALLOW_ROOT_TESTS=1` override) instead of panicking. The test was
a meta-check about the test environment, not correctness, and was
making the suite non-portable to containerised CI.
Test results: 703 passing, 0 failing, 14 ignored (up from 602).
https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
* feat(rust-cli): support multi-line control structures in scripts and source
Previously `execute_script_content` and `Command::Source` iterated
`content.lines()` and parsed each line in isolation, so the canonical
POSIX shape
if true
then
mkdir d
fi
shredded into three lines that failed to parse individually. The same
went for `for/do/done`, `while/do/done`, `case/esac`, and multi-line
function bodies.
Changes:
- `parser.rs`: extract `split_on_top_level` from `split_on_semicolons`
and add `split_on_statement_separators`, which additionally treats
top-level `\n` as a statement boundary. It also tracks brace depth
(scoped to the statement-separator variant) so a `;` inside a
function body — `foo() { a; b; }` — does not end the statement.
Normal `${VAR}` expansions balance themselves and do not affect the
depth.
- `parser.rs::parse_command_block`: use the new splitter so control
structure BODIES can span multiple lines too.
- `main.rs::execute_script_content`: strip whole-line comments, then
feed the entire content through the new splitter.
- `executable.rs::Command::Source`: same fix for sourced files.
- `state.rs::next_fifo_id`: switch to a process-wide atomic counter.
The per-state counter started at 0 for every `ShellState`, so
parallel tests with the same PID collided on
`/tmp/vsh-fifo-<pid>-0`, causing `test_fifo_creation` /
`test_fifo_path_unique` to race intermittently.
- `tests/multiline_script_tests.rs`: 9 regression tests — 4 unit tests
for the splitter (newline, quoted newline, multi-line if/fi, function
defs) and 5 end-to-end tests for sourced scripts (multi-line if,
multi-line for, multi-line function defs, comment-only lines, and
mixed single-/multi-line statements).
Test results: 712 passing, 0 failing, 14 ignored (up from 703).
https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
* feat(rust-cli): implement POSIX §2.6.1 tilde expansion globally
Tilde expansion was previously hard-coded only inside the `cd`
command. It now runs as the first step of `expand_variables`, so every
command that uses variable expansion — `mkdir`, `touch`, `echo`,
`cp`, `mv`, external commands, etc. — gets tilde expansion for free.
Supported forms (per POSIX §2.6.1):
~ → $HOME
~/path → $HOME/path
~+ → $PWD
~+/path → $PWD/path
~- → $OLDPWD
~-/path → $OLDPWD/path
Not expanded inside single or double quotes ('~' / "~" stay literal).
This is enforced by `quoted_word_to_string` escaping `~` → `\~` in
quoted contexts; `expand_tilde` recognises the escape and emits a
literal `~`.
~user (getpwnam lookup) is not yet implemented; the tilde is left
literal in that case.
The `cd`-specific `~/` handling is removed since it's now redundant.
Test results: 721 passing, 0 failing, 14 ignored (up from 712).
https://claude.ai/code/session_01EMHrh5Jq32pb98KXoSKLA4
---------
Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.