#72: Add project copy mode ownership and gitignore management#77
#72: Add project copy mode ownership and gitignore management#77
Conversation
📝 WalkthroughWalkthroughThis PR adds copy-mode installation for Beislið project skills, complementing the existing symlink-only approach. The feature introduces ownership markers, content fingerprinting, and managed .gitignore support to enable safe, repeatable copying of skill directories into projects. ChangesProject Copy-Mode Installation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🤖 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/install_lib.sh`:
- Around line 690-691: The local declaration in _write_project_gitignore uses
"local project=\"$1\" gitignore=\"$project/.gitignore\"" which triggers
ShellCheck SC2318 because gitignore references $project in the same local
statement; change the function to first declare and assign local project="$1"
and then declare local gitignore="$project/.gitignore" (i.e., split into two
local statements) so gitignore is initialized after project is set.
- Around line 531-532: In _project_copy_marker_owns(), avoid declaring marker in
the same local statement that defines dst because marker expands dst at
declaration time and causes portability issues; change the function to declare
local dst and local skill first (e.g., local dst="$1" local skill="$2"), then on
a separate line assign marker using marker="$dst/.beislid-owner.json" (keep
proper quoting) so marker is evaluated after dst is set.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 344c0a13-f0d3-4e70-a6e5-5f3a4a44584f
📒 Files selected for processing (7)
README.mdbin/beisliddocs/configuration.mddocs/how-to-use.mdinstall.shscripts/install_lib.shscripts/test_install.sh
| _project_copy_marker_owns() { | ||
| local dst="$1" skill="$2" marker="$dst/.beislid-owner.json" |
There was a problem hiding this comment.
Split local declarations to avoid shell portability concern.
ShellCheck SC2318: marker references $dst from the same local statement. While bash processes left-to-right, this is a portability and clarity concern.
🛠️ Proposed fix
_project_copy_marker_owns() {
- local dst="$1" skill="$2" marker="$dst/.beislid-owner.json"
+ local dst="$1" skill="$2"
+ local marker="$dst/.beislid-owner.json"
[[ -f "$marker" ]] || return 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _project_copy_marker_owns() { | |
| local dst="$1" skill="$2" marker="$dst/.beislid-owner.json" | |
| _project_copy_marker_owns() { | |
| local dst="$1" skill="$2" | |
| local marker="$dst/.beislid-owner.json" | |
| [[ -f "$marker" ]] || return 1 |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 532-532: This assignment is used again in this 'local', but won't have taken effect. Use two 'local's.
(SC2318)
🤖 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/install_lib.sh` around lines 531 - 532, In
_project_copy_marker_owns(), avoid declaring marker in the same local statement
that defines dst because marker expands dst at declaration time and causes
portability issues; change the function to declare local dst and local skill
first (e.g., local dst="$1" local skill="$2"), then on a separate line assign
marker using marker="$dst/.beislid-owner.json" (keep proper quoting) so marker
is evaluated after dst is set.
| _write_project_gitignore() { | ||
| local project="$1" gitignore="$project/.gitignore" |
There was a problem hiding this comment.
Split local declarations to avoid shell portability concern.
ShellCheck SC2318: gitignore references $project from the same local statement.
🛠️ Proposed fix
_write_project_gitignore() {
- local project="$1" gitignore="$project/.gitignore"
+ local project="$1"
+ local gitignore="$project/.gitignore"
if [[ -L "$gitignore" ]]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _write_project_gitignore() { | |
| local project="$1" gitignore="$project/.gitignore" | |
| _write_project_gitignore() { | |
| local project="$1" | |
| local gitignore="$project/.gitignore" | |
| if [[ -L "$gitignore" ]]; then |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 691-691: This assignment is used again in this 'local', but won't have taken effect. Use two 'local's.
(SC2318)
🤖 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/install_lib.sh` around lines 690 - 691, The local declaration in
_write_project_gitignore uses "local project=\"$1\"
gitignore=\"$project/.gitignore\"" which triggers ShellCheck SC2318 because
gitignore references $project in the same local statement; change the function
to first declare and assign local project="$1" and then declare local
gitignore="$project/.gitignore" (i.e., split into two local statements) so
gitignore is initialized after project is set.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 2 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
fix: CodeRabbit auto-fixes for PR #77
1 participant
Summary
--copyproject installs with per-skill.beislid-owner.jsonmarkers.beislid/project-install.json.gitignoreguidance/--write-gitignoreVerification
bash -n install.sh bin/beislid scripts/install_lib.sh scripts/test_install.shpython3 scripts/validate_skills.pybash scripts/test_install.sh— 59 passedpython3 tests/agent-smoke/run.py gate ship-it --hosts claude,codex --timeout 900 --changed-only— passed on claude, codexNotes
.codexandRELEASE_NOTES.mdare excluded from this PR.Summary by CodeRabbit
New Features
--copymode for creating local portable skill copies, in addition to the default symlink mode.--write-gitignoreflag to manage.gitignoreconfiguration during project installations.--forceflag semantics for safer symlink replacement without affecting unrelated files.Documentation