Conversation
Problem: `echo '~'` and `echo "~"` print the session HOME on main, identical to unquoted `echo ~`. `apply_tilde_expansion` and `value_to_string_with_tilde` (kernel.rs, interpreter/eval.rs) expand any `Value::String` starting with `~` after it has already been evaluated, once quoting information no longer exists. A quoted `rm -r '~/x'` would target the real home directory instead of a literal `~` directory - a data-safety hazard. Evidence: captured bash's actual behavior (HOME=<path> bash -c ...) for every case the fix must cover - quoted forms, assignment values, `cd`, `[[ -f ]]`/`test -f`, `for`, `case`, list literals, heredoc bodies (bash never expands these), redirect targets, and `execute_argv` (argv tokens carry no quoting, so they should never expand either). New crates/kaish-kernel/tests/tilde_expansion_tests.rs is table-driven against those captured values; it fails 17 of 37 cases on the current code. Added matching rows to shell_compat_tests.rs's shell_compat! harness, noting where its hermetic KernelConfig::transient() can't reproduce the bug at all (no session HOME means neither side expands a bare `~`) versus where it can (quoted forms must stay literal on both kaish and bash regardless of HOME).
Decision: lexer::Token::Tilde/TildePath are emitted only for an unquoted source word - a quoted string never produces them. The parser used to collapse both into a plain Expr::Literal(Value:: String(..)), indistinguishable from a quoted literal by the time any evaluator saw it, which is the root cause the previous commit's tests pin. Add Expr::TildePath(String) (ast/types.rs, mirroring NumericLiteral's raw-text pattern) so the AST itself carries "this was an unquoted tilde word" through to evaluation. expand_tilde now runs only where this node is evaluated - the sync Evaluator::eval (interpreter/eval.rs) and the async Kernel::eval_expr_async (kernel.rs) - against the session HOME, never on an already-evaluated Value. Removed: apply_tilde_expansion and every one of its call sites across kernel.rs (build_args_flat, [[ -f ]]'s FileTest, and bind_tool_args/consume_flag_positionals's positional/Named/ WordAssign arms in both the typed and raw-argv forms), the now-dead value_to_string_with_tilde, and the ArgValueSource::home() trait method (and its three impls) that fed them - they were the bug, applied after the fact to any Value::String starting with `~` regardless of where it came from. Kernel::execute_argv's doc comment claimed tilde expansion "for consistency with the string door" as a deliberate exception to its otherwise-literal argv tokens. That claim was itself downstream of the bug: an argv token carries no quoting, so expanding it was equivalent to always treating it as an unquoted word. It now matches the door's other literal-token rules (no glob, no interpolation) - argv tokens never tilde-expand, same as a quoted source word. Updated the doc comment and execute_argv_tests.rs to match. Side effect: several contexts that never called apply_tilde_expansion at all - redirect targets, case subjects, list-literal elements, for-loop items - now expand a bare `~` correctly for the first time, since the fix applies uniformly at every Expr::TildePath evaluation site instead of at call sites that had to opt in individually. Updated existing tests whose AST assertions encoded the old Expr::Literal shape (tilde_assignment_words_tests.rs, literal_path_words_tests.rs, two insta snapshots). One test (keyword_literal_words_tests.rs) had a passing assertion for `~/a:b` that turned out to depend on the exact bug fixed here: a colon-adjacent `~` (`~/a:b`, `x=a:~/b`) is a separate, pre-existing gap - lexer::merge_colon_adjacent fuses it into a plain Ident before the parser ever sees a TildePath token - that kaish has never supported and this change does not add. The test now pins that as current, unsupported behavior instead of silently depending on the bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add docs/LANGUAGE.md's "Tilde expansion" subsection - correct example first, per this file's own writing rules - covering the quoted-vs-unquoted rule, ~user lookup, the hermetic HOME contract, and the word-start-only rule (foo~bar, a/~ never expand). Add the same contrast to the "Paths" syntax fragment (kaish-help/src/ fragments.rs) and regenerate syntax.md with `cargo run -p kaish-help --example regen_syntax` so the drift test stays clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Problem: a code review of the round-1 fix (47b6c0f, f6c727e) traced five more places where a bare `~`/`~/path` either regressed or was never covered - alias bodies, glob words, the background-job command display, two validator checks, and Plan::free_variables. Evidence, captured with real values before writing any fix: - Alias bodies: `alias e='echo ~'; e` and `alias ll='cd ~'; ll` stopped expanding. Alias invocation splits the stored text on whitespace and wraps every piece as Expr::Literal (kernel.rs's execute_command_depth), so no Expr::TildePath node could exist - main expanded these through the value-level sink round 1 removed. A quoted `'~'` inside an alias body was ALREADY broken on main (prints the literal quote marks, `'~'` - alias splitting has never stripped quotes), confirmed by running it pre-round-1 too: not a regression, a separate, unrelated, pre-existing limitation this fix leaves alone. - Glob words: `ls ~/src/*.rs` / `echo ~/*` / `for f in ~/src/*.rs` fail with "no matches" - `~/src/*.rs` lexes as one GlobWord (lexer::is_glob_mergeable folds Tilde/TildePath into a glob run), so it becomes Expr::GlobPattern("~/src/*.rs"), a node neither round-1's fix nor the deleted apply_tilde_expansion ever taught to expand (every apply_tilde_expansion call site special-cased Expr::GlobPattern and `continue`d past it before reaching the value-level sink) - confirmed broken on main too, not a round-1 regression. `x=~/src/*.rs; echo $x` also needs checking: bash expands the tilde-prefix of an assignment value but never pathname-expands it, so the correct answer keeps the literal `*`. - format_expr's catch-all renders a TildePath as `...`, so `cat ~/f &` shows `cat ...` in /v/jobs/N/command. - The validator's literal_path (E023, "same file in and out") and check_numeric_literal_operand (W008) both match only Expr::Literal, so `sort < ~/f > ~/f` and `[[ ~ -eq 1 ]]` silently lost their plan-time reports. - Plan::free_variables's collect_expr never counted a TildePath as reading HOME, making the module doc's "complete by construction" claim false for a bare tilde word. New/updated tests fail on this commit's parent (round 1's tip, 9db94d2): 8 in tilde_expansion_tests.rs (alias x3, glob x4, job display x1), 1 each in redirect_open_tests.rs, validation_tests.rs, and plan_program_tests.rs - 11 total, confirmed by stashing the fix commits that follow and re-running. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Alias bodies: execute_command_depth's alias expansion splits the
stored text on whitespace and wraps every piece as
Expr::Literal(Value::String(..)) - no re-lex, so no Expr::TildePath
node could ever result, and a bare `~` piece stopped expanding when
round 1 removed the value-level sink that used to catch it by
accident. classify_alias_word fixes this by lexing each piece in
isolation and reclassifying one that lexes to exactly one
Tilde/TildePath token as Expr::TildePath - reusing the exact
unquoted-word rule the string door uses, without giving alias bodies
quote-awareness, glob expansion, or $VAR interpolation they never
had. A quoted `'~'` piece keeps its quote marks verbatim either way
(split_whitespace doesn't strip them, so the piece starts with `'`,
never `~`) - unrelated, pre-existing, left alone.
Glob words: lexer::is_glob_mergeable folds Tilde/TildePath into a
glob run, so `~/src/*.rs` lexes as one GlobWord and becomes
Expr::GlobPattern("~/src/*.rs") - a node that has never expanded its
tilde prefix, on main or after round 1 (every apply_tilde_expansion
call site special-cased Expr::GlobPattern and skipped past it before
reaching the value-level sink). Bash expands the tilde-prefix of any
word before globbing it, so three call sites now do the same:
the for-loop's own glob branch, build_args_flat's own glob branch
(external-command argv), and KernelArgSource::expand_glob (covers
every glob-matching path inside the shared bind_tool_args core).
eval_expr_async's Expr::GlobPattern arm also expands the prefix now,
independent of whether a glob later matches anything - this is what
bash does for an assignment value (`x=~/src/*.rs` expands `~` but
never pathname-expands the `*`, since assignment values are never
glob-expanded) and for a session with globbing disabled.
Job display: format_expr's catch-all rendered Expr::TildePath as
"...", so a backgrounded `cat ~/f &` showed "cat ..." at
/v/jobs/N/command instead of the source word - add the same
raw-text arm NumericLiteral already has, matching
ast::plan::render_expr's "unexpanded" contract for the string-door
plan surface.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The round-1 fix added Expr::TildePath but only taught the two
evaluators (interpreter/eval.rs, kernel.rs's eval_expr_async) to
read it - three static-analysis surfaces that pattern-match Expr
shapes without evaluating them still matched only Expr::Literal, so
a bare `~` silently fell out of their coverage.
validate_redirect_input_is_output's literal_path (E023, "same file
as input and output") now also matches Expr::TildePath, comparing
its raw text - the validator has no session HOME to expand against,
and doesn't need one: `sort < ~/f > ~/f` is two identical unquoted
spellings, which resolve to the same path whatever HOME turns out to
be at runtime. Without this, the check silently stopped catching the
tilde form.
check_numeric_literal_operand (W008, "this comparison cannot
succeed") now also treats a TildePath's raw text as a string
operand - a path is never a valid number literal, expanded or not,
so `[[ ~ -eq 1 ]]` refuses the same way `[[ "abc" -eq 1 ]]` does,
regardless of what `~` resolves to.
ast::plan::collect_expr's Expr::TildePath arm now lists HOME as a
read variable. A bare `~`/`~/path` reads the session HOME to expand,
the same dependency an explicit `$HOME` would name - an embedder
that peeks HOME before running `cd ~` is judging the statement
against the value it actually depends on, and the module's own doc
comment already claims the read-set is "complete by construction"
("kaish has no eval and no indirect expansion"), which was false for
a tilde word until now. `~user` reads /etc/passwd instead of HOME,
but that isn't a session variable an embedder can peek either way,
so listing HOME unconditionally is a safe over-approximation rather
than a per-form special case.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
execute_argv's tilde "exception" (~ expands "even in quotes") was the round-1 fix's whole subject - kernel.rs's own doc comment for this function was already updated in that fix (f6c727e); this file carried a second, unsynced copy of the same claim. Reworded to match the corrected contract: argv tokens carry no quoting, so they are never tilde-expanded, same as a quoted source word. expand_tilde's example called it with one argument (`expand_tilde("~/projects/myrepo")`), which doesn't compile - the real signature is `expand_tilde(s: &str, home: Option<&str>)`. Fixed the example and noted why the second argument exists: the kernel is hermetic and never reads the host $HOME on its own. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
One Fixed bullet for the user-visible rule (only an unquoted `~` in the source expands, now including globs, redirects, `for` items and alias bodies) and one Changed bullet for embedders: execute_argv tokens no longer tilde-expand. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
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
1 participant
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.
kaish expanded a quoted
~.echo '~',echo "~", andx='~'; echo "$x"all printed$HOME, becauseapply_tilde_expansionandvalue_to_string_with_tilderewrote any string value that started with~after evaluation, when quoting was already gone. This is a data-safety hazard:rm -r '~/x'would target the real home directory.The parser now makes the decision. An unquoted source word that lexes as a tilde becomes
Expr::TildePath, and only that node expands, against the session HOME. The value-level sinks andArgValueSource::home()are deleted.Expris#[non_exhaustive], so the new variant is not a breaking change.Because every evaluation site now handles the node, a bare
~also expands in redirect targets,casesubjects,foritems, list elements, and alias bodies. The validator's E023 and W008 checks seeTildePath,Plan::free_variableslistsHOME, and/v/jobs/N/commandshows the source word.For embedders:
Kernel::execute_argvno longer expands a leading~. Its tokens carry no quoting, so they stay literal like globs and$VAR. Expand paths before passing them in. A word that an embedder quotes ("~/x") now stays literal, as in bash.Known gaps, left as they were: a colon-adjacent
~(x=a:~/b) does not expand, and alias bodies have no quote handling (alias e="echo '~'"prints the quote marks).Pinned by
tilde_expansion_tests.rs: 17 of 37 cases fail on main, and 11 more tests fail before the second round of fixes.🤖 Generated with Claude Code