feat: improve install.sh with robust merging and parameterization#98
Closed
visahak wants to merge 1 commit into
Closed
feat: improve install.sh with robust merging and parameterization#98visahak wants to merge 1 commit into
visahak wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR introduces documentation and an automated installation script for Kaizen Lite Skills integration with Bob. The README provides setup instructions, usage examples, and development guidelines, while the install.sh script handles skill copying, configuration merging, and verification tasks. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platform-integrations/bob/kaizen-lite/install.sh`:
- Around line 117-129: The current awk that appends "everything after old
kaizen-lite" (the command referencing TARGET_MODES_FILE) fails to print trailing
content when kaizen-lite is the last mode; update that awk so it prints any
lines after the kaizen-lite match even if no subsequent "- slug:" is found.
Replace the awk '/^ - slug: kaizen-lite/ {f=1; next} f && /^ - slug:/ {p=1;
f=0} p' "$TARGET_MODES_FILE" >> "$temp_output" with an expression that sets a
flag on the kaizen-lite match and prints lines while the flag is set (and
continues to print after the next "- slug:" as before) — e.g. use logic like /^
- slug: kaizen-lite/{f=1; next} f{ if (/^ - slug:/) {p=1; f=0} } p||f{print} to
ensure trailing comments/content after kaizen-lite are preserved when appending
temp_new_mode; keep references to TARGET_MODES_FILE, temp_output and
temp_new_mode intact.
- Around line 107-132: temp_output is created but never removed and the awk
patterns only match exactly two-space indentation; add a cleanup trap at script
start to rm -f "$temp_new_mode" "$temp_output" on EXIT (so temp files are
removed even after mv or on errors) and update the awk/grep patterns that
currently use '^ - slug:' to a whitespace-flexible form (e.g., match leading
spaces/tabs before '- slug:' and specifically '- slug: kaizen-lite' when
selecting SOURCE_MODES_FILE and TARGET_MODES_FILE) so the merge works with
different YAML indentation styles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65a982d5-ef33-4154-ac0f-e4072de1d5b5
📒 Files selected for processing (2)
platform-integrations/bob/kaizen-lite/README.mdplatform-integrations/bob/kaizen-lite/install.sh
Comment on lines
+107
to
+132
| temp_new_mode=$(mktemp) | ||
| awk '/^ - slug: kaizen-lite/ {f=1} /^ - slug:/ && !/kaizen-lite/ {f=0} f' "$SOURCE_MODES_FILE" > "$temp_new_mode" | ||
|
|
||
| # Check if kaizen-lite already exists in custom_modes.yaml | ||
| if grep -q "slug: kaizen-lite" "$TARGET_MODES_FILE"; then | ||
| echo -e "${BLUE} Updating existing kaizen-lite mode...${NC}" | ||
| # Remove old kaizen-lite mode and add new one | ||
| temp_output=$(mktemp) | ||
|
|
||
| # Copy everything before kaizen-lite | ||
| awk '/^ - slug: kaizen-lite/{exit} {print}' "$TARGET_MODES_FILE" > "$temp_output" | ||
|
|
||
| # Add new kaizen-lite mode | ||
| cat "$temp_new_mode" >> "$temp_output" | ||
|
|
||
| # Add everything after old kaizen-lite (skip to next mode or end) | ||
| awk '/^ - slug: kaizen-lite/ {f=1; next} f && /^ - slug:/ {p=1; f=0} p' "$TARGET_MODES_FILE" >> "$temp_output" | ||
|
|
||
| mv "$temp_output" "$TARGET_MODES_FILE" | ||
| else | ||
| echo -e "${BLUE} Adding new kaizen-lite mode...${NC}" | ||
| # Just append the new mode | ||
| cat "$temp_new_mode" >> "$TARGET_MODES_FILE" | ||
| fi | ||
|
|
||
| rm "$temp_new_mode" |
There was a problem hiding this comment.
Temp file temp_output is never cleaned up.
The temp_new_mode file is cleaned up at line 132, but temp_output (created at line 114) is never removed. After the mv operation, the temp file handle persists in /tmp.
Additionally, the awk patterns assume exactly 2-space YAML indentation (^ - slug:). Different indentation styles will silently break the merge.
🛠️ Proposed fix for temp file cleanup
mv "$temp_output" "$TARGET_MODES_FILE"
+ # temp_output was moved, no cleanup needed
else
echo -e "${BLUE} Adding new kaizen-lite mode...${NC}"
# Just append the new mode
cat "$temp_new_mode" >> "$TARGET_MODES_FILE"
fi
rm "$temp_new_mode"Note: Since mv moves the file, explicit rm isn't needed for temp_output. However, consider adding a trap to clean up temp files on script exit/error:
# Near the top of the script, after set -e
cleanup() {
rm -f "$temp_new_mode" "$temp_output" 2>/dev/null || true
}
trap cleanup EXIT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platform-integrations/bob/kaizen-lite/install.sh` around lines 107 - 132,
temp_output is created but never removed and the awk patterns only match exactly
two-space indentation; add a cleanup trap at script start to rm -f
"$temp_new_mode" "$temp_output" on EXIT (so temp files are removed even after mv
or on errors) and update the awk/grep patterns that currently use '^ - slug:'
to a whitespace-flexible form (e.g., match leading spaces/tabs before '- slug:'
and specifically '- slug: kaizen-lite' when selecting SOURCE_MODES_FILE and
TARGET_MODES_FILE) so the merge works with different YAML indentation styles.
Comment on lines
+117
to
+129
| awk '/^ - slug: kaizen-lite/{exit} {print}' "$TARGET_MODES_FILE" > "$temp_output" | ||
|
|
||
| # Add new kaizen-lite mode | ||
| cat "$temp_new_mode" >> "$temp_output" | ||
|
|
||
| # Add everything after old kaizen-lite (skip to next mode or end) | ||
| awk '/^ - slug: kaizen-lite/ {f=1; next} f && /^ - slug:/ {p=1; f=0} p' "$TARGET_MODES_FILE" >> "$temp_output" | ||
|
|
||
| mv "$temp_output" "$TARGET_MODES_FILE" | ||
| else | ||
| echo -e "${BLUE} Adding new kaizen-lite mode...${NC}" | ||
| # Just append the new mode | ||
| cat "$temp_new_mode" >> "$TARGET_MODES_FILE" |
There was a problem hiding this comment.
YAML merge may lose trailing content when kaizen-lite is the last mode.
The awk logic at line 123 only starts printing (p=1) when it encounters another - slug: after kaizen-lite. If kaizen-lite is the last mode in the target file, any trailing comments or content after it will be silently dropped.
♻️ Proposed fix to preserve trailing content
- # Add everything after old kaizen-lite (skip to next mode or end)
- awk '/^ - slug: kaizen-lite/ {f=1; next} f && /^ - slug:/ {p=1; f=0} p' "$TARGET_MODES_FILE" >> "$temp_output"
+ # Add everything after old kaizen-lite (skip to next mode or end, preserve trailing content)
+ awk '
+ /^ - slug: kaizen-lite/ {f=1; next}
+ f && /^ - slug:/ {p=1; f=0}
+ f && /^[^ ]/ && !/^ - slug:/ {p=1; f=0} # Also catch non-indented lines (top-level keys, comments)
+ p
+ ' "$TARGET_MODES_FILE" >> "$temp_output"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platform-integrations/bob/kaizen-lite/install.sh` around lines 117 - 129, The
current awk that appends "everything after old kaizen-lite" (the command
referencing TARGET_MODES_FILE) fails to print trailing content when kaizen-lite
is the last mode; update that awk so it prints any lines after the kaizen-lite
match even if no subsequent "- slug:" is found. Replace the awk '/^ - slug:
kaizen-lite/ {f=1; next} f && /^ - slug:/ {p=1; f=0} p' "$TARGET_MODES_FILE" >>
"$temp_output" with an expression that sets a flag on the kaizen-lite match and
prints lines while the flag is set (and continues to print after the next "-
slug:" as before) — e.g. use logic like /^ - slug: kaizen-lite/{f=1; next} f{
if (/^ - slug:/) {p=1; f=0} } p||f{print} to ensure trailing comments/content
after kaizen-lite are preserved when appending temp_new_mode; keep references to
TARGET_MODES_FILE, temp_output and temp_new_mode intact.
Contributor
|
@visahak Can you address the coderabbit comments? |
Collaborator
Author
|
I don't think we need to merge this anymore. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding install and readme for the bob skills
Summary by CodeRabbit
New Features
Documentation