feat: add zsh shell detection to install script

[elibosley]

Summary by CodeRabbit

• New Features

  • Installer now detects when the environment is using Zsh and halts with clear error messages and guidance so users can move Zsh configuration to interactive-only files.

• Bug Fixes

  • Prevents running the installer under unsupported shell setups, improving installation reliability and avoiding misconfigured runs.

[coderabbitai]

Caution

Review failed

The head commit changed during the review from 3c17545 to 0cbb068.

Walkthrough

A new check_zsh function was added to the installation verification script; it detects if the script is running under zsh, writes multiple error messages to stderr instructing zsh be limited to interactive use, and exits with status 1 before other checks.

Changes

Cohort / File(s)
Verify script plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh — Added check_zsh() to detect zsh, emit error messages to stderr, and exit 1 immediately to prevent running the script under zsh.

Sequence Diagram(s)

sequenceDiagram
    participant UserShell as User shell (process $$)
    participant Script as verify_install.sh
    UserShell->>Script: invoke verify_install.sh
    Script->>Script: check_zsh() (inspect current process command)
    alt shell is zsh
        Script->>UserShell: write error messages to stderr
        Script->>UserShell: exit 1
    else shell is not zsh
        Script->>Script: continue with remaining verification steps
    end

Loading

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• refactor(install): simplify installation scripts and improve error vi… #1513 — Modifies the same verify_install.sh script and touches script execution/verification behavior; likely related to pre-checks and error handling.

Suggested reviewers

• pujitm

Poem

I nibble lines of code at dawn, 🐇
A tiny guard so scripts run right,
If zsh peeks in, I sound the bell,
"Keep zsh for chats, not for this flight."
Hop on, bash — the install's bright!

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/zsh-check

---

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

[elibosley]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (2)

8-10: Comment/code drift
The comment mentions querying both $$ and $PPID, but only the first call was implemented. Update the comment after fixing the PID logic to keep documentation trustworthy.

---

22-23: Consider scope-limiting the check
Running check_zsh unconditionally will also abort when the script is invoked from bash that was itself launched by zsh (zsh -c "bash …"). If the intent is to block execution directly under zsh, add an interactive/login test (e.g., [[ $- == *i* ]]).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 86b6c4f and dc5b51a.

📒 Files selected for processing (1)

• plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (5)

Learnt from: elibosley
PR: #1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:19-24
Timestamp: 2025-05-08T19:28:54.365Z
Learning: The directory /usr/local/emhttp/plugins/dynamix.my.servers is a valid directory that exists as part of the Unraid API plugin installation and should be included in verification checks.

Learnt from: elibosley
PR: #1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/setup_api.sh:107-113
Timestamp: 2025-05-07T16:07:47.236Z
Learning: The Unraid API is designed to handle missing configuration files gracefully with smart internal fallbacks rather than requiring installation scripts to create default configurations.

Learnt from: pujitm
PR: #1415
File: plugin/plugins/dynamix.unraid.net.plg:234-236
Timestamp: 2025-06-11T14:14:30.348Z
Learning: For the Unraid Connect plugin, the script /etc/rc.d/rc.unraid-api is bundled with the plugin package itself, so its presence on the target system is guaranteed during installation.

Learnt from: pujitm
PR: #1252
File: api/src/environment.ts:56-56
Timestamp: 2025-03-27T13:34:53.438Z
Learning: For critical components in the Unraid API, such as retrieving version information from package.json, failing fast (allowing crashes) is preferred over graceful degradation with fallback values.

Learnt from: elibosley
PR: #969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the setup commands in web/justfile and api/justfile are intentional and correct behavior.

🔇 Additional comments (1)

plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh (1)

18-19: Behavioural change: script now exits non-zero early
Up to now the verify script has always exited 0, even on failures, to avoid breaking higher-level installers. The new unconditional exit 1 alters that contract and may abort automated deployments. Double-check that this is intentional.

[coderabbitai]

⚠️ Potential issue

Detection always reports “not-zsh” – wrong PID used
ps -o comm= -p $$ inspects the current script’s process (bash, due to the she-bang), so the comparison will never hit “zsh”. Use the parent PID instead:

-  parent_shell=$(ps -o comm= -p $$)
+  parent_shell=$(ps -o comm= -p "$PPID")

Without this change the new safeguard is ineffective.

🤖 Prompt for AI Agents

In
plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh
around lines 11 to 15, the script incorrectly checks the current shell process
ID ($$) instead of the parent process ID, causing it to never detect zsh. Change
the command to use the parent process ID ($PPID) with ps to correctly identify
if the parent shell is zsh and trigger the warning as intended.

[elibosley]

Yes — great point. If the user’s login shell is bash, and their .bash_profile or .bashrc contains a line like:

bash
Copy
Edit
exec zsh
Then their shell process hierarchy looks like:

scss
Copy
Edit
bash (login shell)
└── zsh (interactive shell from .bash_profile)
└── your_script.sh (via plugin install)
In that case:

$$ → the PID of the script itself (usually bash, from the shebang)

$PPID → the PID of zsh

ps -o comm= -p "$PPID" → "zsh" ✅

So your fixed version correctly detects zsh in this case.

However, if the user launches the script from bash directly (e.g., not using zsh, or via other methods like cron, or via the Unraid GUI), then the parent shell could be bash, sh, or something else entirely.

[coderabbitai]

@elibosley Exactly! You've captured the key scenario this detection is designed for. The exec zsh pattern in bash profile files is a common way users configure zsh as their interactive shell while keeping bash as the login shell, and that's precisely when we want the script to detect and warn about potential issues.

Your explanation of the process hierarchy is spot-on - the script needs to check the parent process to catch the zsh shell that's actually executing it, not the script's own bash process from the shebang.

The fact that other launch contexts (cron, Unraid GUI, direct bash execution) won't have zsh as the parent is actually the desired behavior - those contexts shouldn't trigger the warning since they're not affected by the user's interactive shell configuration.

---

✏️ Learnings added

Learnt from: elibosley
PR: unraid/api#1539
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:11-15
Timestamp: 2025-07-25T17:46:57.584Z
Learning: In the Unraid API installation script, the zsh detection function is specifically designed to catch cases where users have `exec zsh` in their .bash_profile or .bashrc, creating a process hierarchy like: bash (login) → zsh (interactive) → script. The detection should use `$PPID` to check the parent process, not `$$` which only checks the script's own process.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

[claude]

Claude finished @elibosley's task —— View job

---

Review Findings

🚨 CRITICAL BUG in line 12: The shell detection logic is broken and will never work.

current_shell=$(ps -o comm= -p $$)

This checks the current process ($$) which will always be bash due to the #!/bin/bash shebang. The function will never detect zsh and the security check is completely ineffective.

Root Cause: Should use $PPID (parent process ID) instead of $$ to detect if the parent shell is zsh.

Impact: Users running the script under zsh (the exact scenario this PR aims to prevent) will not be detected, making the entire safety mechanism useless.

Fix Required: Change line 12 to:

current_shell=$(ps -o comm= -p "$PPID")

This critical logic error makes the zsh detection completely non-functional and defeats the purpose of the entire PR.

Todo List

• Read the changed file and analyze the diff
• Review only the added/modified lines for critical issues
• Post review findings

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.47%. Comparing base (b518131) to head (0cbb068).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1539   +/-   ##
=======================================
  Coverage   58.47%   58.47%           
=======================================
  Files         636      636           
  Lines       31156    31156           
  Branches     3649     3649           
=======================================
  Hits        18220    18220           
  Misses      12899    12899           
  Partials       37       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1539/dynamix.unraid.net.plg