Fix: copy .claude/scripts/ root files during install#155
Fix: copy .claude/scripts/ root files during install#155udhaya10 wants to merge 1 commit intoparcadei:mainfrom
Conversation
_copy_scripts() copies from opc/scripts/ but .claude/scripts/ root files (status.py, status.sh, tldr_stats.py, etc.) live in a different source tree and were never deployed. This breaks statusLine which references ~/.claude/scripts/status.py. Add _copy_dotfile_scripts() to copy root-level .py/.sh files from .claude/scripts/ during both copy and symlink installs. Also add the missing "Installed N scripts" output to the wizard. Fixes parcadei#154
|
PR author is not in the allowed authors list. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new helper function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| console.print(f" [green]OK[/green] Installed {result['installed_rules']} rules") | ||
| console.print(f" [green]OK[/green] Installed {result['installed_agents']} agents") | ||
| console.print(f" [green]OK[/green] Installed {result['installed_servers']} MCP servers") | ||
| console.print(f" [green]OK[/green] Installed {result['installed_scripts']} scripts") |
There was a problem hiding this comment.
Bug: The setup wizard will crash with a KeyError during symlink installation because the install_opc_integration_symlink function fails to return the installed_scripts count.
Severity: CRITICAL
Suggested Fix
In the install_opc_integration_symlink function, initialize the result dictionary with "installed_scripts": 0. Then, capture the integer return values from the _copy_scripts and _copy_dotfile_scripts calls and sum them into the result["installed_scripts"] key before returning.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: opc/scripts/setup/wizard.py#L806
Potential issue: The `install_opc_integration_symlink` function does not initialize or
populate the `installed_scripts` key in the dictionary it returns. However, the setup
wizard in `wizard.py` unconditionally accesses `result['installed_scripts']` when a user
chooses the symlink installation option. This discrepancy will cause a `KeyError` and
crash the setup wizard during the installation process, preventing users from completing
the setup with this option. The function calls `_copy_scripts` and
`_copy_dotfile_scripts` but fails to capture and store their return values, which are
the counts of the installed scripts.
Did we get this right? 👍 / 👎 to inform future reviews.
udhaya10
left a comment
udhaya10
left a comment
There was a problem hiding this comment.
Re: Sentry comment on line 806 — false positive.
result['installed_scripts'] is only accessed after install_opc_integration() (copy path), which initializes it at line 487. The install_opc_integration_symlink() result is consumed at lines 762-780 and 821-829 where only symlinked_dirs and backed_up_dirs are read — never installed_scripts. No KeyError will occur.
PR looks good to merge.
1 participant
Summary
.claude/scripts/root files (e.g.status.py,status.sh,tldr_stats.py) are never copied to~/.claude/scripts/during fresh install, breakingstatusLinewhich referencesstatus.py_copy_scripts()only handlesopc/scripts/(different source tree) —.claude/scripts/root files were missedChanges
_copy_dotfile_scripts()— copies root-level.py/.shfiles from.claude/scripts/to targetinstall_opc_integration()andinstall_opc_integration_symlink()"Installed N scripts"print line in wizard (both code paths)Why this approach
ROOT_SCRIPTSROOT_SCRIPTSmapsopc/scripts/, not.claude/scripts/.claude/scripts/_copy_scripts()subdirs_copy_dotfile_scripts()_copy_scripts(), mirrors existing patternZero overlap between the two functions —
_copy_scripts()handles subdirs + named root files fromopc/scripts/, while_copy_dotfile_scripts()handles root-level files from.claude/scripts/.Test plan
~/.claude/scripts/status.pyexistsstatus.pyis copiedOK Installed N scriptsecho '{}' | python3 ~/.claude/scripts/status.pyFixes #154
Summary by CodeRabbit
Release Notes
New Features
Improvements