Shell and terminal improved

[KenVanHoeylandt]

• Refactored shell parsing so that the callstack doesn't increase when script complexity increases.
• Improved terminal input handling: input delay from +- 100 ms to at most about 16 ms.

Summary by CodeRabbit

• New Features
  • Shell commands can now run script files, with expanded support for arithmetic, control flow, functions, substitutions, and redirections.
  • Command completion distinguishes directories from runnable commands and completes file paths in commands and arguments.
• Bug Fixes
  • Shell output preserves embedded NUL bytes and routes child standard output and error to their corresponding streams.
  • Terminal output updates more promptly, and shell input continues to be processed until the shell exits.
  • Deeply nested shell operations are less dependent on native stack space.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: df94b18f-27b8-4fcd-9fb6-7cf75d021c82

📥 Commits

Reviewing files that changed from the base of the PR and between ca8125d and 2c84a1c.

📒 Files selected for processing (1)

• Modules/app-module/source/stdio_wrap.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• Modules/app-module/source/stdio_wrap.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The shell parser, expansion, arithmetic, and execution implementations are replaced or added in C++. Several nested operations use iterative or frame-based processing. Shell integration updates standard-stream handling, command completion, script launching, and resource allocation. Terminal input callbacks now signal the shell pump, which notifies the render task. New tests cover shell behavior, nesting, stack use, and completion.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 2c84a

No actionable issue remains on the supplied evidence; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to ca812

The new execution path does not appear to create a remote shell entrypoint, but a failed subshell directory restoration could leave later commands using an unexpected directory. The prior behavior and some affected input and terminal paths remain unverified.

Retained concerns

• Medium · security · inferred: If restoring a subshell's application working directory fails, cleanup still restores its logical cwd and continues without reporting the failure. Subsequent relative-path operations could therefore use an unexpected directory; whether this differs from the replaced executor is unresolved.

Security review details

Security Blast Radius

• inferred — The evidenced exposure is commands executed within the shell application, including script input and subsequent relative-path operations. Repository evidence does not establish remote reachability or an increase in privilege.

Security Findings and Attack Paths

• inferred — A script that reaches a subshell directory-restoration failure could leave the interpreter's logical cwd inconsistent with the application's cwd before later commands run. Neither a successful exploit nor a regression against the old executor has been established.

Trust Boundaries and Controls

• observed — The observed command path enters through the existing shell application rather than directly through the added execution-machine header.

Resilience and Maintainability Implications

• observed — ExecGroup owns a subshell snapshot and restores it during frame cleanup, but its directory-restoration call does not propagate failure to that cleanup path.

Hardening Proposals

• proposed — Define an explicit failure outcome for subshell directory restoration so later relative-path operations cannot silently proceed with inconsistent directory state.

🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 31.28% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 227 functions across 29 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title identifies the shell and terminal areas, but it does not describe the main changes, such as the stack-growth reduction or terminal input latency improvement.	Use a specific title, such as "Refactor shell execution to reduce stack usage and terminal input latency".

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 8

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2f624a98-74ea-492d-a1e2-8f33541b4dea

📥 Commits

Reviewing files that changed from the base of the PR and between 33898dd and d229bce.

📒 Files selected for processing (34)

• Documentation/ideas.md
• Modules/app-module/source/stdio_wrap.cpp
• Tactility/Private/Tactility/app/shell/Shell.h
• Tactility/Private/Tactility/app/shell/shell/sh.h
• Tactility/Private/Tactility/app/shell/shell/sh_arith.h
• Tactility/Private/Tactility/app/shell/shell/sh_builtins.h
• Tactility/Private/Tactility/app/shell/shell/sh_glob.h
• Tactility/Private/Tactility/app/shell/shell/sh_lex.h
• Tactility/Private/Tactility/app/shell/shell/sh_machine.h
• Tactility/Private/Tactility/app/shell/shell/sh_parse.h
• Tactility/Private/Tactility/app/shell/shell/sh_port.h
• Tactility/Private/Tactility/app/terminal/Shell.h
• Tactility/Private/Tactility/app/terminal/vterm/vterm.h
• Tactility/Source/app/shell/Run.cpp
• Tactility/Source/app/shell/Shell.cpp
• Tactility/Source/app/shell/main.cpp
• Tactility/Source/app/shell/shell/sh_arith.c
• Tactility/Source/app/shell/shell/sh_arith.cpp
• Tactility/Source/app/shell/shell/sh_builtins.c
• Tactility/Source/app/shell/shell/sh_exec.c
• Tactility/Source/app/shell/shell/sh_exec.cpp
• Tactility/Source/app/shell/shell/sh_expand.c
• Tactility/Source/app/shell/shell/sh_expand.cpp
• Tactility/Source/app/shell/shell/sh_glob.c
• Tactility/Source/app/shell/shell/sh_lex.c
• Tactility/Source/app/shell/shell/sh_parse.c
• Tactility/Source/app/shell/shell/sh_parse.cpp
• Tactility/Source/app/terminal/Shell.cpp
• Tactility/Source/app/terminal/Terminal.cpp
• Tactility/Source/app/terminal/vterm/vterm.c
• Tactility/Tests/CMakeLists.txt
• Tactility/Tests/Scripts/shell_test.sh
• Tactility/Tests/Source/ShellCompletionTest.cpp
• Tactility/Tests/Source/ShellInterpreterTest.cpp

💤 Files with no reviewable changes (5)

• Tactility/Source/app/shell/shell/sh_expand.c
• Tactility/Source/app/shell/shell/sh_parse.c
• Documentation/ideas.md
• Tactility/Source/app/shell/shell/sh_exec.c
• Tactility/Source/app/shell/shell/sh_arith.c

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 79c88f76-ce20-4628-8a33-2b94c76034c5

📥 Commits

Reviewing files that changed from the base of the PR and between d229bce and ca8125d.

📒 Files selected for processing (12)

• Modules/app-module/source/stdio_wrap.cpp
• Tactility/Private/Tactility/app/shell/shell/sh_machine.h
• Tactility/Private/Tactility/app/terminal/vterm/vterm.h
• Tactility/Source/app/shell/Run.cpp
• Tactility/Source/app/shell/Shell.cpp
• Tactility/Source/app/shell/shell/sh_exec.cpp
• Tactility/Source/app/shell/shell/sh_expand.cpp
• Tactility/Source/app/terminal/Terminal.cpp
• Tactility/Source/app/terminal/vterm/vterm.c
• Tactility/Tests/Source/Main.cpp
• Tactility/Tests/Source/ShellCompletionTest.cpp
• Tactility/Tests/Source/ShellInterpreterTest.cpp

🚧 Files skipped from review as they are similar to previous changes (8)

• Tactility/Private/Tactility/app/terminal/vterm/vterm.h
• Tactility/Source/app/terminal/Terminal.cpp
• Tactility/Source/app/terminal/vterm/vterm.c
• Tactility/Source/app/shell/Run.cpp
• Tactility/Source/app/shell/Shell.cpp
• Tactility/Tests/Source/ShellInterpreterTest.cpp
• Tactility/Source/app/shell/shell/sh_expand.cpp
• Tactility/Source/app/shell/shell/sh_exec.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.