fix(desktop-prod): clean backend data dir for actual fresh-install emulation - #103
Conversation
…ll emulation The script was cleaning Tauri's APP_ID dir (~/Library/Application Support/com.debpalash.omnivoice-studio) but the Python backend writes to ~/Library/Application Support/OmniVoice — a separate hardcoded path in backend/core/config.py::get_app_data_dir(). Result: "🧹 Cleaning all OmniVoice data for fresh prod emulation" was deleting an empty directory while the real user data (SQLite db, voice profiles, dub jobs, outputs, logs) sat untouched. Developers running desktop-prod thought they were testing a clean install path, but were actually testing on accumulated state. Fix: add a BACKEND_DATA variable and a 1b cleanup step targeting the backend's actual data dir. Per platform: - macOS: ~/Library/Application Support/OmniVoice - Linux: ~/.omnivoice - Windows: %APPDATA%/OmniVoice (not in this script; Windows uses .bat) Surfaced while running `bun desktop-prod` for the first time today on a clean tree — the .app launched fine but Settings showed a pre-existing voice profile from a prior session, contradicting the "fresh" claim. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe script now explicitly separates backend state (SQLite, voice profiles, outputs, logs) from Tauri app data. Platform-specific initialization defines ChangesBackend Data Cleanup Separation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/desktop-prod.sh`:
- Line 92: The BD_SIZE and HF_SIZE assignments use command substitutions that
will abort the script under set -euo pipefail if du fails; change each to a
best-effort check: run du -sh "${BACKEND_DATA}" (and du -sh "${HF_DATA}") inside
an if statement, capture its output and pipe to cut only on success, and
otherwise set BD_SIZE/HF_SIZE to a safe default like "N/A" (or empty) so
failures don't exit the script; update the assignments for BD_SIZE and HF_SIZE
accordingly, referencing those exact variable names in the modified blocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 544cb1f6-16d1-43a2-b7d0-9d5b2b27fbed
📒 Files selected for processing (1)
scripts/desktop-prod.sh
| # 1b. Backend data (SQLite db, voice profiles, outputs, logs) | ||
| # — separate dir hardcoded in backend/core/config.py, NOT under APP_ID. | ||
| if [ -d "${BACKEND_DATA}" ]; then | ||
| BD_SIZE=$(du -sh "${BACKEND_DATA}" 2>/dev/null | cut -f1) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Bash, with set -euo pipefail, does a failing command inside VAR=$(cmd1 | cmd2)cause script exit, and isif VAR=$(...)exempt from-e termination semantics?
💡 Result:
In Bash, with set -e (errexit) enabled, the behavior of failing commands inside command substitution and if statements is governed by specific exceptions defined in the Bash Reference Manual [1][2]. 1. Does a failing command inside VAR=$(cmd1 | cmd2) cause script exit? Yes, it typically does, provided the pipeline itself returns a non-zero exit status. When set -e is enabled, Bash exits immediately if a pipeline returns a non-zero status [1][2]. With set -o pipefail also enabled, the pipeline's exit status is the status of the last command to exit with a non-zero status (or zero if all succeed) [3][4]. Therefore, if any command in the pipeline fails, the entire pipeline returns a non-zero status, triggering the set -e exit [3][4]. Note that the subshell created for command substitution inherits the -e setting from the parent shell [5]. 2. Is if VAR=$(...) exempt from -e termination semantics? Yes, the command substitution itself is exempt from -e termination when it appears as the test condition of an if statement [6][1]. According to the Bash Reference Manual, the shell does not exit if the failing command is part of the test following the if reserved word [1][2]. Because the if statement explicitly handles the exit status of the command list (the command substitution), Bash considers the error "caught" and does not trigger an immediate exit [6][7]. Summary of behavior: - VAR=$(cmd): If the command (or pipeline) fails, the script exits [6][5]. - if VAR=$(cmd); then...; fi: If the command fails, the script does not exit; instead, the if condition evaluates to false, and the script continues to the else block or the next statement [6][7].
Citations:
- 1: https://www.gnu.org/s/bash/manual/html_node/The-Set-Builtin.html
- 2: https://durak.org/sean/pubs/software/bash-5.2/bashref_17.html
- 3: https://unixy.io/blog/set-euo-pipefail-not-enough/
- 4: https://medium.com/@betashorts1998/understanding-bash-pipelines-and-set-o-pipefail-ba7e06ffb684
- 5: https://stackoverflow.com/questions/19054868/using-command-substitution-or-similar-but-still-having-script-exit-using-set
- 6: https://brandonwie.dev/posts/bash-set-e-command-substitution
- 7: https://sumguy.com/bash-set-e-gotcha/
🏁 Script executed:
# First, check if the file exists and read the relevant section around line 92
head -100 scripts/desktop-prod.sh | tail -20Repository: debpalash/OmniVoice-Studio
Length of output: 783
🏁 Script executed:
# Check the beginning of the file for set options
head -20 scripts/desktop-prod.shRepository: debpalash/OmniVoice-Studio
Length of output: 904
🏁 Script executed:
# Get the exact line and surrounding context
sed -n '85,100p' scripts/desktop-prod.shRepository: debpalash/OmniVoice-Studio
Length of output: 614
🏁 Script executed:
# Check if there are other similar logging patterns in scripts directory
rg "BD_SIZE|du -sh" scripts/ --type shRepository: debpalash/OmniVoice-Studio
Length of output: 321
🏁 Script executed:
# Look for error handling patterns with command substitution in scripts/
rg "if.*=\$\(" scripts/ --type sh -A 2 | head -40Repository: debpalash/OmniVoice-Studio
Length of output: 52
🏁 Script executed:
# Check if there are other size/logging operations that handle errors
rg "SIZE\|size" scripts/ --type sh -B 2 -A 2 | head -50Repository: debpalash/OmniVoice-Studio
Length of output: 52
🏁 Script executed:
# Find the exact line numbers for both BD_SIZE and HF_SIZE
grep -n "BD_SIZE\|HF_SIZE" scripts/desktop-prod.shRepository: debpalash/OmniVoice-Studio
Length of output: 318
Fix both size-logging assignments to handle du failures gracefully.
The script has set -euo pipefail at the top, which means any non-zero exit from a pipeline in a command substitution like BD_SIZE=$(du -sh ... | cut -f1) will immediately abort the script. This prevents the cleanup (rm -rf) from running if du fails due to transient filesystem or permission issues.
The issue affects both line 92 (BD_SIZE) and line 101 (HF_SIZE). Wrap each in an if statement to make the logging best-effort:
Suggested fix
- BD_SIZE=$(du -sh "${BACKEND_DATA}" 2>/dev/null | cut -f1)
+ if BD_SIZE="$(du -sh "${BACKEND_DATA}" 2>/dev/null | cut -f1)"; then
+ :
+ else
+ BD_SIZE="unknown"
+ fi
echo " ✗ Backend data: ${BACKEND_DATA} (${BD_SIZE})"
...
- HF_SIZE=$(du -sh "${HF_CACHE}" 2>/dev/null | cut -f1)
+ if HF_SIZE="$(du -sh "${HF_CACHE}" 2>/dev/null | cut -f1)"; then
+ :
+ else
+ HF_SIZE="unknown"
+ fi
echo " ✗ HF cache: ${HF_CACHE} (${HF_SIZE})"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/desktop-prod.sh` at line 92, The BD_SIZE and HF_SIZE assignments use
command substitutions that will abort the script under set -euo pipefail if du
fails; change each to a best-effort check: run du -sh "${BACKEND_DATA}" (and du
-sh "${HF_DATA}") inside an if statement, capture its output and pipe to cut
only on success, and otherwise set BD_SIZE/HF_SIZE to a safe default like "N/A"
(or empty) so failures don't exit the script; update the assignments for BD_SIZE
and HF_SIZE accordingly, referencing those exact variable names in the modified
blocks.
1 participant
Problem
`scripts/desktop-prod.sh` claims "🧹 Cleaning all OmniVoice data for fresh prod emulation" but cleans the wrong directory.
These are separate hardcoded paths. The script's cleanup never touches the backend dir, so developers running `bun desktop-prod` think they're testing a clean install path but are actually testing on accumulated state.
Surfaced
Running `bun desktop-prod` for the first time today on a clean tree: the .app launched, Settings showed a pre-existing voice profile ("palish") from a prior session despite the script's "fresh emulation" claim.
Fix
Add a `BACKEND_DATA` variable and a `1b` cleanup step targeting the backend's actual data dir. Per platform (matches `backend/core/config.py`):
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit