From 557cd1c242f1c9e3f19b2a6478da8faf230235a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96zhan=20Gebe=C5=9Fo=C4=9Flu?= Date: Thu, 21 May 2026 17:17:46 +0300 Subject: [PATCH] fix(executor): honor redirections for builtins, propagate exit codes, fix $VAR expansion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tier 1 scripting fixes (all TDD, 248 tests pass): - executor: process_command_line now returns execute_ast status, so `kishi -c "false"` exits 1; non-interactive mode exits with last status - builtins: kishi_exit honors `exit N` code (ValueError -> 1); goodbye to stderr - executor: single builtin/function path applies stdout/stderr redirection via contextlib (_run_redirected) — pure-Python, identical on Linux & Windows - executor: redirect targets expanded (Expander.expand globbing=False) so `> $VAR` and `> ~/x` resolve correctly - expander: only a bare `$VAR` token takes the drop-if-empty path; `$VAR/suffix` and `$VAR.txt` now expand the variable portion and keep the suffix - main: `-c` and piped/non-interactive modes no longer run .kishirc command lines (bash -c parity); aliases/exports still load Co-Authored-By: Claude Opus 4.7 --- ...2026-05-21-tier1-redirect-exitcode-plan.md | 232 ++++++++++++++++++ kishi/builtins.py | 11 +- kishi/executor.py | 67 ++++- kishi/expander.py | 5 +- kishi/main.py | 14 +- tests/test_executor.py | 175 +++++++++++++ tests/test_expander.py | 19 ++ tests/test_main.py | 35 +++ 8 files changed, 542 insertions(+), 16 deletions(-) create mode 100644 docs/superpowers/plans/2026-05-21-tier1-redirect-exitcode-plan.md diff --git a/docs/superpowers/plans/2026-05-21-tier1-redirect-exitcode-plan.md b/docs/superpowers/plans/2026-05-21-tier1-redirect-exitcode-plan.md new file mode 100644 index 0000000..25769f8 --- /dev/null +++ b/docs/superpowers/plans/2026-05-21-tier1-redirect-exitcode-plan.md @@ -0,0 +1,232 @@ +# Tier 1 Bugfix Planı — Redirect & Exit Kodu + +> Kapsam: Kishi-Shell v2.0.0.3'te **scripting'i kurtaran 4 bug**. Hepsi reprodüksiyonla doğrulandı (2026-05-21). +> Çapraz platform (Linux + Windows) hedeflenir. Yöntem: builtin/function çıktısı için **`contextlib.redirect_stdout/redirect_stderr`** (saf Python, OS-bağımsız). +> Her faz ayrı bir oturumda çalıştırılabilir. Önce TDD (kırmızı test), sonra düzeltme. + +--- + +## Phase 0 — Bulgular & İzin Verilen API'ler (ALWAYS FIRST) + +### Kaynaklar (okundu + reprodüksiyon yapıldı, bu oturum) +- `kishi/executor.py` — tamamı okundu; `execute_pipeline` (sat. 21-282), `process_command_line` (sat. 388-428). +- `kishi/main.py` — `_main_inner` `-c` dalı (sat. 207-213), non-interactive dal (sat. 219-230). +- `kishi/builtins.py` — `kishi_exit` (sat. 40-42). +- `kishi/tui_dashboard.py` — **kopyalanacak desen**: `_run_builtin`/`_run_function` `contextlib.redirect_stdout` kullanımı (sat. 430-453). +- `kishi/expander.py` — `Expander.expand(arg_list, globbing=True)` imzası (sat. 7-8); `globbing=False` tilde+değişkeni açar, glob'u açmaz. +- `tests/test_executor.py` — `clean_state` fixture (sat. 22-41), `_make_pipeline` helper (sat. 44-52), `TestProcessCommandLine` (sat. 632-649). +- `tests/test_builtins.py` — `capsys`/`tmp_path` desenleri, `kishi_pwd` çıktısı testi (sat. 95-101). + +### Reprodüksiyonla kanıtlanan kusurlar +- **#1**: `process_command_line('false')` → `None` döndü → `kishi -c` daima exit 0. +- **#2**: `kishi_exit` daima `sys.exit(0)`, `args` yok sayılır. +- **#3**: `pwd > f` → `/tmp` ekrana yazıldı, `f` oluşmadı (builtin redirect uygulanmıyor). +- **#4**: `echo viavar > $MYOUT` → literal `$MYOUT` dosyası oluştu (hedef expand edilmiyor). + +### İzin verilen API'ler (var olduğu doğrulanmış) +- `contextlib.redirect_stdout(fileobj)`, `contextlib.redirect_stderr(fileobj)` — Python stdlib, Linux+Windows aynı. (`tui_dashboard.py:432` zaten kullanıyor.) +- `open(path, 'w'|'a', encoding=...)` — dosya nesnesi; contextlib'e geçilir. +- `Expander.expand([target], globbing=False)` — mevcut imza. +- `os.open(...)` + `os.O_*` flag'leri — external/fork yolunda zaten kullanılıyor (`executor.py:162-170`). +- `sys.exit(int)` — SystemExit `main()` tarafından `except SystemExit: raise` ile yayılır (`main.py:285-286`). + +### Anti-pattern'ler (YAPMA) +- ❌ fd-level `os.dup2` ile builtin redirect (Windows'ta flush/restore riski — kullanıcı çapraz-platform sorunsuzluğu istedi). +- ❌ `process_command_line`'ın imzasını/parametrelerini değiştirmek (sadece `return` ekle). +- ❌ Var olmayan `Expander.expand_target()` gibi metot uydurmak — sadece mevcut `expand`'i kullan. +- ❌ Redirect yokken builtin çağrı yolunu değiştirmek (mevcut testleri bozar). + +--- + +## Phase 1 — Kırmızı Testler (TDD önce) + +**Ne yapılacak:** `tests/test_executor.py`'a yeni test sınıfları ekle. Önce ÇALIŞTIR, KIRMIZI olduklarını gör. + +`_make_pipeline` helper'ı redirect alanlarını set edecek şekilde genişlet (yeni bir yardımcı, mevcut imzayı bozmadan): +```python +def _make_redirect_pipeline(args, out_file=None, out_append=False, + err_file=None, in_file=None, err_to_out=False): + pipe = PipelineNode() + cmd = CommandNode() + cmd.args = list(args) + cmd.out_file = out_file; cmd.out_append = out_append + cmd.err_file = err_file; cmd.in_file = in_file; cmd.err_to_out = err_to_out + pipe.commands.append(cmd) + return pipe +``` + +Eklenecek testler: +- **#3** `TestBuiltinRedirection`: + - `pwd > f` (tmp_path) → dosya cwd içerir, **stdout boş** (`capsys`). + - `pwd >> f` → append modu (iki çağrı → iki satır). + - builtin `2>` ile stderr'e yazan mock builtin → err dosyasına yazar. + - function redirect: `state.FUNCTIONS['x']` body `pwd` → `x > f` dosyaya yazar. + - **Regresyon:** redirect'siz builtin (mevcut `_make_pipeline`) hâlâ doğrudan çağrılır, dönüş kodu korunur. +- **#4** `TestRedirectTargetExpansion`: + - `out_file="$MYVAR"` (MYVAR=/tmp/x) → gerçek `/tmp/x` yazılır, literal `$MYVAR` dosyası OLUŞMAZ. + - `out_file="~/..."` → `os.path.expanduser` hedefi. +- **#1** `TestProcessCommandLineExitCode`: + - `process_command_line("false")` → `1` döndürür (mock builtin `_false`→1 ile, ya da gerçek `false` external). + - `process_command_line("true")` → `0`. +- **#2** `TestExitBuiltin`: + - `kishi_exit(["exit","5"])` → `SystemExit` kodu `5` (pytest.raises(SystemExit) + `e.value.code == 5`). + - `kishi_exit(["exit"])` → kod `0`. + +**Doğrulama:** `python -m pytest tests/test_executor.py -k "Redirection or ExitCode or ExitBuiltin or Expansion" -v` → hepsi **FAIL/ERROR** (henüz fix yok). + +**Anti-pattern guard:** Testler gerçek davranışı (dosya içeriği, exit kodu, capsys çıktısı) doğrulasın; sadece "çağrıldı mı" değil. + +--- + +## Phase 2 — Fix #4: Redirect hedef expansion (en küçük, en düşük risk) + +**Ne yapılacak:** `execute_pipeline` içinde `valid_commands` kurulurken (mevcut `executor.py:28-35` döngüsü), `cmd.args` expand edildikten **sonra** redirect hedeflerini de expand et: +```python +for cmd in commands: + if cmd.args and cmd.args[0] in ALIASES: + ... + cmd.args = Expander.expand(cmd.args) + # YENİ: redirect hedeflerini de genişlet (tilde + $VAR; glob yok) + for attr in ("in_file", "out_file", "err_file"): + val = getattr(cmd, attr) + if val: + exp = Expander.expand([val], globbing=False) + if exp: + setattr(cmd, attr, exp[0]) + if cmd.args: + valid_commands.append(cmd) +``` + +**Neden burada:** Bu döngü hem builtin hem external hem Windows hem fork yolundan ÖNCE çalışır → tek değişiklik tüm yolları kapsar. + +**Doğrulama:** Phase 1'deki `TestRedirectTargetExpansion` testleri **GEÇER**. `python -m pytest tests/test_executor.py -k Expansion -v`. + +**Anti-pattern guard:** `globbing=True` KULLANMA (redirect hedefi glob'lanmamalı; çoklu eşleşme ambiguity'si). Tek elemanlı liste döner. + +--- + +## Phase 3 — Fix #3: Builtin/function redirect (contextlib) + +**Ne yapılacak:** `execute_pipeline` tek-komut yolunda (`executor.py:42-80`), builtin/function çağrısını redirect varsa bir yardımcıdan geçir. `tui_dashboard.py:430-453` desenini KOPYALA. + +Yeni modül-seviyesi yardımcı (`executor.py`): +```python +import contextlib + +def _run_redirected(callable_fn, call_args, cmd): + """Builtin/function'ı stdout/stderr redirect uygulayarak çalıştır. + contextlib kullanır — saf Python, Linux+Windows aynı.""" + has_redir = cmd.out_file or cmd.err_file or cmd.err_to_out + if not has_redir: + return callable_fn(call_args) + + out_ctx = contextlib.nullcontext() + err_ctx = contextlib.nullcontext() + out_fh = err_fh = None + try: + if cmd.out_file: + out_fh = open(cmd.out_file, "a" if cmd.out_append else "w", encoding="utf-8") + out_ctx = contextlib.redirect_stdout(out_fh) + if cmd.err_to_out: + # 2>&1: stderr'i mevcut (yönlendirilmiş) stdout'a bağla + err_ctx = contextlib.redirect_stderr(out_fh if out_fh else sys.stdout) + elif cmd.err_file: + err_fh = open(cmd.err_file, "a" if cmd.err_append else "w", encoding="utf-8") + err_ctx = contextlib.redirect_stderr(err_fh) + with out_ctx, err_ctx: + try: + return callable_fn(call_args) + except SystemExit: + raise + finally: + if out_fh: out_fh.close() + if err_fh: err_fh.close() +``` + +Tek-komut builtin dalı (`executor.py:65-66`): +```python +if cmd_name in BUILTINS: + return _run_redirected(BUILTINS[cmd_name], cmd.args[actual_cmd_idx:], cmd) +``` +Function dalı (`executor.py:67-80`): aynı şekilde, `execute_ast(FUNCTIONS[cmd_name])`'i `lambda _: execute_ast(...)` ile sarıp `_run_redirected`'tan geçir (positional param set/restore mantığı korunur). + +**Doğrulama:** Phase 1'deki `TestBuiltinRedirection` **GEÇER**; mevcut `TestBuiltinDispatch`/`TestFunctionDefinitionAndCall` testleri **HÂLÂ GEÇER** (redirect'siz yol değişmedi). `python -m pytest tests/test_executor.py -v`. + +**Anti-pattern guard:** ❌ `os.dup2` kullanma. ❌ `print()` çıktısını manuel string'e toplayıp tekrar yazma (contextlib zaten yakalar). ❌ `SystemExit`'i yutma (exit builtin'i çalışmalı) — `finally` ile dosya kapanır, exception yayılır. + +--- + +## Phase 4 — Fix #1: process_command_line exit kodu döndürsün + +**Ne yapılacak:** `executor.py:426-428` — `execute_ast` sonucunu döndür: +```python +ast = Parser.parse(tokens) +if ast: + return execute_ast(ast) +return 0 +``` +(Fonksiyon başındaki erken `return` yolları da `return 0` döndürsün — şu an boş `return` → `None`. `if not tokens: return 0` yap.) + +`main.py` `-c` dalı (sat. 207-213) zaten `sys.exit(result or 0)` yapıyor → otomatik düzelir. **Ek:** non-interactive dalda (sat. 219-230) son komutun statüsünü izle: +```python +last = 0 +for line in sys.stdin: + line = line.strip() + if line: + last = process_command_line(line) or 0 +sys.exit(last) +``` + +**Doğrulama:** `TestProcessCommandLineExitCode` **GEÇER**. Manuel: `python -m kishi -c "false"; echo $?` → `1`; `python -m kishi -c "true"; echo $?` → `0`. Mevcut `TestProcessCommandLine` testleri (dönüşü kontrol etmiyor) **HÂLÂ GEÇER**. + +**Anti-pattern guard:** ❌ `process_command_line`'a yeni parametre ekleme. Sadece `return` davranışı değişir. + +--- + +## Phase 5 — Fix #2: `exit N` çıkış kodunu onurlandırsın + +**Ne yapılacak:** `builtins.py:40-42` `kishi_exit`: +```python +def kishi_exit(args): + code = 0 + if len(args) > 1: + try: code = int(args[1]) + except ValueError: code = 1 + print(f"\n{COLOR_AMBER}Kishi:{COLOR_RESET} Exiting safely. Goodbye!", file=sys.stderr) + sys.exit(code) +``` +(Çıkış mesajını `stderr`'e al ki `exit 0` redirect'te stdout'u kirletmesin — opsiyonel ama temiz.) + +**Doğrulama:** `TestExitBuiltin` **GEÇER**. Manuel: `python -m kishi -c "exit 7"; echo $?` → `7`. + +**Anti-pattern guard:** ❌ Geçersiz arg'da çökme — `ValueError`'da kod `1` (bash davranışı). + +--- + +## Phase 6 — Doğrulama (Final) + +1. **Tüm test paketi:** `python -m pytest tests/ -v` → **228 + yeni testler** hepsi yeşil, regresyon yok. +2. **Manuel uçtan uca** (yeni terminalde): + - `python -m kishi -c "pwd > /tmp/v.txt"; cat /tmp/v.txt` → cwd dosyada. + - `python -m kishi -c "echo a > /tmp/v2.txt && pwd >> /tmp/v2.txt"` → iki satır. + - `python -m kishi -c "false"; echo $?` → `1`. + - `python -m kishi -c "exit 5"; echo $?` → `5`. + - `export D=/tmp; python -m kishi -c "pwd > \$D/v3.txt"` → `/tmp/v3.txt` yazılır. +3. **Anti-pattern grep guard'ları:** + - `grep -n "os.dup2" kishi/executor.py` → builtin redirect bölgesinde OLMAMALI. + - `grep -n "return execute_ast" kishi/executor.py` → `process_command_line` içinde VAR olmalı. + - `grep -n "contextlib" kishi/executor.py` → VAR olmalı. +4. **CI:** `pyproject.toml testpaths=["tests"]` → kök `test_cursor*.py` toplanmaz; etkilenmez. +5. **Cross-platform notu:** contextlib + `open()` saf Python; Windows fark gerektirmez. `kishi_exit`/`process_command_line` platform-bağımsız. + +--- + +## Etkilenen Dosyalar (özet) +- `kishi/executor.py` — Phase 2, 3, 4 (`_run_redirected` ekle, valid_commands döngüsü, process_command_line return). +- `kishi/builtins.py` — Phase 5 (`kishi_exit`). +- `kishi/main.py` — Phase 4 (non-interactive son statü). +- `tests/test_executor.py` — Phase 1 (yeni test sınıfları + helper). + +## Riskler +- 🟢 Düşük. Tüm değişiklikler "redirect varsa yeni yol, yoksa eski yol" prensibiyle additive; mevcut 228 test kontrol noktası. +- ⚠️ `2>&1` sıralama semantiği bash ile tam eşleşmeyebilir (Tier 1 kapsamı dışı; ayrı not). diff --git a/kishi/builtins.py b/kishi/builtins.py index 2e28428..5a01b14 100644 --- a/kishi/builtins.py +++ b/kishi/builtins.py @@ -38,8 +38,15 @@ def kishi_pwd(args): return 0 def kishi_exit(args): - print(f"\n{COLOR_AMBER}Kishi:{COLOR_RESET} Exiting safely. Goodbye!") - sys.exit(0) + code = 0 + if len(args) > 1: + try: + code = int(args[1]) + except ValueError: + code = 1 + # Goodbye message goes to stderr so 'exit > file' does not pollute stdout. + print(f"\n{COLOR_AMBER}Kishi:{COLOR_RESET} Exiting safely. Goodbye!", file=sys.stderr) + sys.exit(code) def kishi_clear(args): clear() diff --git a/kishi/executor.py b/kishi/executor.py index a475d06..16556b5 100644 --- a/kishi/executor.py +++ b/kishi/executor.py @@ -2,8 +2,9 @@ import sys import shlex import signal +import contextlib -from .state import COLOR_RED, COLOR_YELLOW, ALIASES, LOCAL_VARS, FUNCTIONS, BUILTINS +from .state import COLOR_RED, COLOR_RESET, COLOR_YELLOW, ALIASES, LOCAL_VARS, FUNCTIONS, BUILTINS from .parser import SequenceNode, IfNode, WhileNode, ForNode, FunctionDefNode, LogicNode, PipelineNode, CaseNode, UntilNode, SelectNode from .expander import Expander from .job_control import JobManager @@ -18,6 +19,49 @@ def get_close_match_suggestion(cmd_name): return f"\nDid you mean: {COLOR_CYAN}'{matches[0]}'{COLOR_RESET} ?" return "" +def _run_redirected(callable_fn, call_args, cmd): + """Run a builtin/function applying stdout/stderr redirection via contextlib. + + Pure-Python (no fd-level dup2) so behavior is identical on Linux and Windows. + Returns the callable's status. When no redirection is set, calls directly so + the existing non-redirected path is unchanged. + """ + if not (cmd.out_file or cmd.err_file or cmd.err_to_out): + return callable_fn(call_args) + + out_fh = None + err_fh = None + try: + out_ctx = contextlib.nullcontext() + err_ctx = contextlib.nullcontext() + + if cmd.out_file: + try: + out_fh = open(cmd.out_file, "a" if cmd.out_append else "w", encoding="utf-8") + except OSError: + print(f"{COLOR_RED}Error:{COLOR_RESET} Cannot write to {cmd.out_file}.", file=sys.stderr) + return 1 + out_ctx = contextlib.redirect_stdout(out_fh) + + if cmd.err_to_out: + # 2>&1 — bind stderr to the (possibly redirected) stdout target + err_ctx = contextlib.redirect_stderr(out_fh if out_fh is not None else sys.stdout) + elif cmd.err_file: + try: + err_fh = open(cmd.err_file, "a" if cmd.err_append else "w", encoding="utf-8") + except OSError: + print(f"{COLOR_RED}Error:{COLOR_RESET} Cannot write to {cmd.err_file}.", file=sys.stderr) + return 1 + err_ctx = contextlib.redirect_stderr(err_fh) + + with out_ctx, err_ctx: + return callable_fn(call_args) + finally: + if out_fh is not None: + out_fh.close() + if err_fh is not None: + err_fh.close() + def execute_pipeline(pipe_node): if not pipe_node.commands: return 0 @@ -31,6 +75,14 @@ def execute_pipeline(pipe_node): aliased_parts = shlex.split(ALIASES[cmd.args[0]]) cmd.args = aliased_parts + cmd.args[1:] cmd.args = Expander.expand(cmd.args) + # Expand redirection targets too (tilde + $VAR; no globbing to avoid + # ambiguous multi-match). Applies to builtin, external, fork and Windows paths. + for _attr in ("in_file", "out_file", "err_file"): + _val = getattr(cmd, _attr) + if _val: + _exp = Expander.expand([_val], globbing=False) + if _exp: + setattr(cmd, _attr, _exp[0]) if cmd.args: valid_commands.append(cmd) @@ -63,15 +115,15 @@ def execute_pipeline(pipe_node): if actual_cmd_idx < len(cmd.args): cmd_name = cmd.args[actual_cmd_idx] if cmd_name in BUILTINS: - return BUILTINS[cmd_name](cmd.args[actual_cmd_idx:]) + return _run_redirected(BUILTINS[cmd_name], cmd.args[actual_cmd_idx:], cmd) elif cmd_name in FUNCTIONS: args_passed = cmd.args[actual_cmd_idx:] old_args = {str(i): LOCAL_VARS.get(str(i), None) for i in range(1, len(args_passed))} for i in range(1, len(args_passed)): LOCAL_VARS[str(i)] = args_passed[i] - - status = execute_ast(FUNCTIONS[cmd_name]) - + + status = _run_redirected(lambda _a: execute_ast(FUNCTIONS[cmd_name]), args_passed, cmd) + for k, v in old_args.items(): if v is None: if k in LOCAL_VARS: del LOCAL_VARS[k] @@ -422,7 +474,8 @@ def process_command_line(cmd_line): from .parser import Parser tokens = Tokenizer.wrap_tokenize(cmd_line) if not tokens: - return + return 0 ast = Parser.parse(tokens) if ast: - execute_ast(ast) + return execute_ast(ast) + return 0 diff --git a/kishi/expander.py b/kishi/expander.py index 60bba1f..4a43ae4 100644 --- a/kishi/expander.py +++ b/kishi/expander.py @@ -43,7 +43,10 @@ def cmd_replacer(match): arg = re.sub(r'\$\(([^)]+)\)|`([^`]+)`', cmd_replacer, arg) # 2. Variable Expansion (unquoted and double-quoted) - if arg.startswith('$'): + # Only a token that is *exactly* a single variable ($VAR) takes the + # drop-if-empty path. "$VAR/suffix" / "$VAR.txt" fall through to the + # regex replacer so the non-variable suffix is preserved. + if re.fullmatch(r'\$[A-Za-z0-9_]+', arg): var_name = arg[1:] val = LOCAL_VARS.get(var_name, os.environ.get(var_name, ALIASES.get(var_name, ""))) if val: expanded_args.append(val) diff --git a/kishi/main.py b/kishi/main.py index 44abc57..fa7a40d 100644 --- a/kishi/main.py +++ b/kishi/main.py @@ -206,9 +206,10 @@ def _main_inner(): # --- 6. -c modu: komutu çalıştır ve çık --- if len(args) >= 2 and args[0] == "-c": + # Like 'bash -c', do NOT run .kishirc command lines here; only the + # aliases/exports parsed by load_rc_file apply. startup_cmds are + # interactive-only to avoid polluting scripted stdout. cmd = args[1] - for sc in startup_cmds: - process_command_line(sc) result = process_command_line(cmd) sys.exit(result or 0) @@ -218,16 +219,17 @@ def _main_inner(): # --- 8. Non-interactive mod: stdin'den satır satır oku --- if not is_interactive: - for sc in startup_cmds: - process_command_line(sc) + # Non-interactive (piped/scripted) input does not source .kishirc + # command lines, matching bash behavior. Aliases/exports already applied. + last_status = 0 try: for line in sys.stdin: line = line.strip() if line: - process_command_line(line) + last_status = process_command_line(line) or 0 except (EOFError, KeyboardInterrupt): pass - sys.exit(0) + sys.exit(last_status) # --- 9. Interactive mod --- state.IS_INTERACTIVE = True diff --git a/tests/test_executor.py b/tests/test_executor.py index 3e630bc..8ce5c83 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -59,6 +59,23 @@ def _make_sequence(*nodes): return seq +def _make_redirect_pipeline(args, out_file=None, out_append=False, + err_file=None, err_append=False, + in_file=None, err_to_out=False): + """Helper: single-command PipelineNode carrying redirection fields.""" + pipe = PipelineNode() + cmd = CommandNode() + cmd.args = list(args) + cmd.out_file = out_file + cmd.out_append = out_append + cmd.err_file = err_file + cmd.err_append = err_append + cmd.in_file = in_file + cmd.err_to_out = err_to_out + pipe.commands.append(cmd) + return pipe + + # --------------------------------------------------------------------------- # Variable Assignment (VAR=value) # --------------------------------------------------------------------------- @@ -671,3 +688,161 @@ def test_returns_empty_when_no_system_commands(self): state.SYSTEM_COMMANDS = [] result = get_close_match_suggestion("anything") assert result == "" + + +# --------------------------------------------------------------------------- +# Bug #3: Builtin / function output redirection (single-command path) +# --------------------------------------------------------------------------- + +class TestBuiltinRedirection: + def test_builtin_stdout_redirected_to_file(self, tmp_path): + """A builtin's stdout should be written to out_file, not the terminal.""" + f = tmp_path / "out.txt" + + def printer(args): + print("REDIRECTED_OUTPUT") + return 0 + + state.BUILTINS["_printer"] = printer + pipe = _make_redirect_pipeline(["_printer"], out_file=str(f)) + rc = execute_pipeline(pipe) + + assert rc == 0 + assert f.read_text() == "REDIRECTED_OUTPUT\n" + + def test_builtin_stdout_not_on_terminal(self, tmp_path, capsys): + """When redirected, nothing should reach the real stdout.""" + f = tmp_path / "out.txt" + + def printer(args): + print("SHOULD_BE_IN_FILE") + return 0 + + state.BUILTINS["_printer2"] = printer + execute_pipeline(_make_redirect_pipeline(["_printer2"], out_file=str(f))) + + captured = capsys.readouterr() + assert "SHOULD_BE_IN_FILE" not in captured.out + + def test_builtin_stdout_append(self, tmp_path): + """out_append should append, not truncate.""" + f = tmp_path / "a.txt" + f.write_text("EXISTING\n") + + def printer(args): + print("APPENDED") + return 0 + + state.BUILTINS["_ap"] = printer + execute_pipeline(_make_redirect_pipeline(["_ap"], out_file=str(f), out_append=True)) + + assert f.read_text() == "EXISTING\nAPPENDED\n" + + def test_builtin_stderr_redirected(self, tmp_path): + """A builtin's stderr should be written to err_file.""" + import sys as _sys + f = tmp_path / "e.txt" + + def errprinter(args): + print("ERRMSG", file=_sys.stderr) + return 0 + + state.BUILTINS["_err"] = errprinter + execute_pipeline(_make_redirect_pipeline(["_err"], err_file=str(f))) + + assert f.read_text() == "ERRMSG\n" + + def test_function_output_redirected(self, tmp_path): + """A user function's output should honor redirection.""" + f = tmp_path / "fn.txt" + + def body_builtin(args): + print("FROM_FUNC") + return 0 + + state.BUILTINS["_fnbody"] = body_builtin + state.FUNCTIONS["myfn"] = _make_sequence(_make_pipeline(["_fnbody"])) + execute_pipeline(_make_redirect_pipeline(["myfn"], out_file=str(f))) + + assert f.read_text() == "FROM_FUNC\n" + + def test_builtin_without_redirect_still_returns_status(self): + """Regression: redirect-less builtin still called directly with its status.""" + state.BUILTINS["_plain"] = lambda args: 7 + rc = execute_pipeline(_make_redirect_pipeline(["_plain"])) + assert rc == 7 + + +# --------------------------------------------------------------------------- +# Bug #4: Redirect target expansion ($VAR, ~) +# --------------------------------------------------------------------------- + +class TestRedirectTargetExpansion: + def test_redirect_target_expands_variable(self, tmp_path, monkeypatch): + """out_file '$VAR' should expand to the real path before opening.""" + target = tmp_path / "exp.txt" + monkeypatch.setenv("KISHI_RT", str(target)) + + def printer(args): + print("VAREXP") + return 0 + + state.BUILTINS["_rt"] = printer + execute_pipeline(_make_redirect_pipeline(["_rt"], out_file="$KISHI_RT")) + + assert target.read_text() == "VAREXP\n" + + def test_redirect_target_expands_tilde(self, tmp_path, monkeypatch): + """out_file '~/x' should expand via HOME.""" + monkeypatch.setenv("HOME", str(tmp_path)) + + def printer(args): + print("TILDE") + return 0 + + state.BUILTINS["_tl"] = printer + execute_pipeline(_make_redirect_pipeline(["_tl"], out_file="~/tl.txt")) + + assert (tmp_path / "tl.txt").read_text() == "TILDE\n" + + +# --------------------------------------------------------------------------- +# Bug #1: process_command_line should return the command's exit status +# --------------------------------------------------------------------------- + +class TestProcessCommandLineExitCode: + def test_returns_status_of_command(self): + state.BUILTINS["_rc3"] = lambda args: 3 + assert process_command_line("_rc3") == 3 + + def test_returns_zero_for_success(self): + state.BUILTINS["_rc0"] = lambda args: 0 + assert process_command_line("_rc0") == 0 + + def test_empty_input_returns_zero(self): + assert process_command_line("") == 0 + assert process_command_line(" ") == 0 + + +# --------------------------------------------------------------------------- +# Bug #2: exit builtin should honor a numeric exit code argument +# --------------------------------------------------------------------------- + +class TestExitBuiltin: + def test_exit_with_code(self): + from kishi.builtins import kishi_exit + with pytest.raises(SystemExit) as exc: + kishi_exit(["exit", "5"]) + assert exc.value.code == 5 + + def test_exit_no_arg_is_zero(self): + from kishi.builtins import kishi_exit + with pytest.raises(SystemExit) as exc: + kishi_exit(["exit"]) + assert exc.value.code == 0 + + def test_exit_invalid_arg_is_one(self): + from kishi.builtins import kishi_exit + with pytest.raises(SystemExit) as exc: + kishi_exit(["exit", "abc"]) + assert exc.value.code == 1 diff --git a/tests/test_expander.py b/tests/test_expander.py index eac7c1a..d6e60a4 100644 --- a/tests/test_expander.py +++ b/tests/test_expander.py @@ -41,6 +41,25 @@ def test_local_takes_precedence(self): del state.LOCAL_VARS["pvar"] del os.environ["pvar"] + def test_variable_with_path_suffix(self): + """$VAR/sub/file should expand the $VAR portion, keep the suffix.""" + os.environ["KISHI_DIR"] = "/tmp" + result = Expander.expand(["$KISHI_DIR/sub/file.txt"]) + assert result == ["/tmp/sub/file.txt"] + del os.environ["KISHI_DIR"] + + def test_variable_with_extension_suffix(self): + """$VAR.txt should expand $VAR and keep '.txt'.""" + os.environ["KISHI_BASE"] = "report" + result = Expander.expand(["$KISHI_BASE.txt"]) + assert result == ["report.txt"] + del os.environ["KISHI_BASE"] + + def test_bare_undefined_variable_still_dropped(self): + """Regression: a bare undefined $VAR is still dropped (word removal).""" + result = Expander.expand(["$NOPE_UNDEFINED_XYZ"]) + assert result == [] + class TestTildeExpansion: def test_tilde(self): diff --git a/tests/test_main.py b/tests/test_main.py index 5d0a443..fe4fc20 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -5,12 +5,15 @@ import os import re import sys +import subprocess import pytest from unittest.mock import patch from kishi import state from kishi.main import load_rc_file, load_plugins, _source_profile +PROJECT_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + # --------------------------------------------------------------------------- # Fixtures @@ -275,3 +278,35 @@ def test_single_quoted_values_stripped(self, tmp_path, clean_env): _source_profile(str(profile)) assert os.environ["KISHI_PROFILE_VAR"] == "single" + + +# --------------------------------------------------------------------------- +# Non-interactive modes must NOT run .kishirc command lines (bash -c parity) +# --------------------------------------------------------------------------- + +class TestNonInteractiveRcPollution: + def _run(self, argv, home, stdin=None): + env = dict(os.environ, HOME=str(home)) + return subprocess.run( + [sys.executable, "-m", "kishi"] + argv, + cwd=PROJECT_ROOT, env=env, input=stdin, + capture_output=True, text=True, timeout=30, + ) + + def test_dash_c_does_not_run_rc_startup_commands(self, tmp_path): + """kishi -c must not execute command lines from .kishirc on stdout.""" + (tmp_path / ".kishirc").write_text("echo RC_MARKER_SHOULD_NOT_APPEAR\n") + result = self._run(["-c", "true"], tmp_path) + assert "RC_MARKER_SHOULD_NOT_APPEAR" not in result.stdout + + def test_pipe_mode_does_not_run_rc_startup_commands(self, tmp_path): + """echo cmd | kishi must not execute .kishirc command lines.""" + (tmp_path / ".kishirc").write_text("echo RC_MARKER_PIPE\n") + result = self._run([], tmp_path, stdin="true\n") + assert "RC_MARKER_PIPE" not in result.stdout + + def test_dash_c_still_loads_aliases_from_rc(self, tmp_path): + """Aliases (not command lines) from .kishirc must still apply in -c mode.""" + (tmp_path / ".kishirc").write_text("alias greet='echo HELLO_FROM_ALIAS'\n") + result = self._run(["-c", "greet"], tmp_path) + assert "HELLO_FROM_ALIAS" in result.stdout