Onboard simplic-oxs-agent-skill#178
Open
MEichhoff wants to merge 1 commit into
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
WalkthroughThis PR establishes an agent guidelines synchronization system with platform-specific scripts that fetch shared coding rules and practices from a central repository, automated session-level triggering, and comprehensive instructions for AI agents and developers on how to use the synced guidelines. ChangesAgent Guidelines Sync Infrastructure
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/sync-guidelines.ps1`:
- Line 60: Validate $Dir before calling Test-Path/Remove-Item to avoid
accidental recursive deletion: ensure $Dir is non-empty, not a drive root (e.g.
"C:\"), not a system or commonly dangerous path, and resolves inside an expected
base directory (use Resolve-Path or Join-Path to canonicalize) and only then
call Remove-Item -Recurse -Force; if validation fails, log an error and abort.
Reference the $Dir variable and the Test-Path / Remove-Item usage when adding
this guard.
- Around line 65-66: The current Write-Host "[agent-guidelines] warning: git
clone failed" silently continues on clone error; update the failure handling for
the first-time clone (when the local guidelines directory is missing) to return
a non-zero exit so the script fails fast. Locate the git clone error branch by
searching for the exact string "[agent-guidelines] warning: git clone failed"
and replace the silent log with a failing action (e.g., Write-Error or throw
plus exit 1) so the script exits non-zero on initial clone failure instead of
succeeding.
- Around line 52-54: The script currently calls Invoke-Git '-C', $Dir, 'reset',
'--quiet', '--hard', "origin/$Branch" and then unconditionally writes the stamp
with Set-Content $stamp and logs success; change it to check the Invoke-Git
result/exit status (or catch exceptions) and only call Set-Content -Path $stamp
and Write-Host when the reset succeeded; on failure, emit an error/message and
exit with non-zero instead of stamping so .last-sync isn't updated for failed
resets.
In `@scripts/sync-guidelines.sh`:
- Around line 47-48: The script currently runs rm -rf "$DIR" (where DIR is set
from AGENT_GUIDELINES_DIR) without safety checks; add a guard that verifies
DIR/AGENT_GUIDELINES_DIR is non-empty, not equal to "/" or ".", and ideally
resolves to an absolute path (use realpath) and/or matches an expected base
prefix before performing removal; if the checks fail, abort with an error
message instead of deleting. Ensure the checks reference the
AGENT_GUIDELINES_DIR/DIR variables used in the script so the removal only runs
when the safety conditions pass.
- Around line 52-53: The current git clone failure branch in the
scripts/sync-guidelines.sh script only echoes a warning and exits successfully;
change that branch to fail fast by returning a non-zero exit code: update the
failure handling after the git clone command (the conditional that currently
runs echo "[agent-guidelines] warning: git clone failed") to log an error and
call exit 1 so the script stops with a failure when the initial clone fails.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10c2bf9b-2237-4157-918f-bba240a627b9
📒 Files selected for processing (7)
.claude/settings.json.github/copilot-instructions.md.gitignoreAGENTS.mdCLAUDE.mdscripts/sync-guidelines.ps1scripts/sync-guidelines.sh
Comment on lines
+52
to
+54
| Invoke-Git @('-C', $Dir, 'reset', '--quiet', '--hard', "origin/$Branch") | Out-Null | ||
| Set-Content -Path $stamp -Value $today -Encoding utf8 -NoNewline | ||
| Write-Host "[agent-guidelines] synced from $Repo ($Branch) -> $Dir @ $today" |
Contributor
There was a problem hiding this comment.
Do not stamp success if git reset fails
Line 52 ignores the Invoke-Git reset boolean and still writes .last-sync on Line 53. This can mark stale content as fresh.
Suggested fix
- if (Invoke-Git @('-C', $Dir, 'fetch', '--quiet', 'origin', $Branch)) {
- Invoke-Git @('-C', $Dir, 'reset', '--quiet', '--hard', "origin/$Branch") | Out-Null
+ if (Invoke-Git @('-C', $Dir, 'fetch', '--quiet', 'origin', $Branch) -and
+ (Invoke-Git @('-C', $Dir, 'reset', '--quiet', '--hard', "origin/$Branch"))) {
Set-Content -Path $stamp -Value $today -Encoding utf8 -NoNewline
Write-Host "[agent-guidelines] synced from $Repo ($Branch) -> $Dir @ $today"
} else {🤖 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/sync-guidelines.ps1` around lines 52 - 54, The script currently calls
Invoke-Git '-C', $Dir, 'reset', '--quiet', '--hard', "origin/$Branch" and then
unconditionally writes the stamp with Set-Content $stamp and logs success;
change it to check the Invoke-Git result/exit status (or catch exceptions) and
only call Set-Content -Path $stamp and Write-Host when the reset succeeded; on
failure, emit an error/message and exit with non-zero instead of stamping so
.last-sync isn't updated for failed resets.
| } | ||
| } else { | ||
| # Clone the repo for the first time | ||
| if (Test-Path $Dir) { Remove-Item -Recurse -Force $Dir } |
Contributor
There was a problem hiding this comment.
Add a safety guard before recursive delete
Line 60 can recursively remove an unsafe path if $Dir is misconfigured. Validate $Dir before Remove-Item.
🤖 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/sync-guidelines.ps1` at line 60, Validate $Dir before calling
Test-Path/Remove-Item to avoid accidental recursive deletion: ensure $Dir is
non-empty, not a drive root (e.g. "C:\"), not a system or commonly dangerous
path, and resolves inside an expected base directory (use Resolve-Path or
Join-Path to canonicalize) and only then call Remove-Item -Recurse -Force; if
validation fails, log an error and abort. Reference the $Dir variable and the
Test-Path / Remove-Item usage when adding this guard.
Comment on lines
+65
to
+66
| Write-Host "[agent-guidelines] warning: git clone failed" | ||
| } |
Contributor
There was a problem hiding this comment.
Return non-zero on first-time clone failure
When clone fails here, the script exits success. For a missing local guidelines directory, this should fail to avoid silent stale/missing guidance.
🤖 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/sync-guidelines.ps1` around lines 65 - 66, The current Write-Host
"[agent-guidelines] warning: git clone failed" silently continues on clone
error; update the failure handling for the first-time clone (when the local
guidelines directory is missing) to return a non-zero exit so the script fails
fast. Locate the git clone error branch by searching for the exact string
"[agent-guidelines] warning: git clone failed" and replace the silent log with a
failing action (e.g., Write-Error or throw plus exit 1) so the script exits
non-zero on initial clone failure instead of succeeding.
Comment on lines
+47
to
+48
| rm -rf "$DIR" | ||
| if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; then |
Contributor
There was a problem hiding this comment.
Guard rm -rf against unsafe target paths
Line 47 deletes whatever AGENT_GUIDELINES_DIR resolves to. Add a hard safety check (non-empty, not /, not .) before removal to prevent destructive misconfiguration.
Suggested fix
- rm -rf "$DIR"
+ if [[ -z "$DIR" || "$DIR" == "/" || "$DIR" == "." ]]; then
+ echo "[agent-guidelines] error: unsafe AGENT_GUIDELINES_DIR='$DIR'"
+ exit 1
+ fi
+ rm -rf "$DIR"📝 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.
Suggested change
| rm -rf "$DIR" | |
| if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; then | |
| if [[ -z "$DIR" || "$DIR" == "/" || "$DIR" == "." ]]; then | |
| echo "[agent-guidelines] error: unsafe AGENT_GUIDELINES_DIR='$DIR'" | |
| exit 1 | |
| fi | |
| rm -rf "$DIR" | |
| if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; then |
🤖 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/sync-guidelines.sh` around lines 47 - 48, The script currently runs
rm -rf "$DIR" (where DIR is set from AGENT_GUIDELINES_DIR) without safety
checks; add a guard that verifies DIR/AGENT_GUIDELINES_DIR is non-empty, not
equal to "/" or ".", and ideally resolves to an absolute path (use realpath)
and/or matches an expected base prefix before performing removal; if the checks
fail, abort with an error message instead of deleting. Ensure the checks
reference the AGENT_GUIDELINES_DIR/DIR variables used in the script so the
removal only runs when the safety conditions pass.
Comment on lines
+52
to
+53
| echo "[agent-guidelines] warning: git clone failed" | ||
| fi |
Contributor
There was a problem hiding this comment.
Fail fast when initial clone fails
On Line 52-53, clone failure only logs a warning and exits success. That can silently continue with missing/stale guidelines. Return non-zero in this path.
Suggested fix
else
echo "[agent-guidelines] warning: git clone failed"
+ exit 1
fi📝 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.
Suggested change
| echo "[agent-guidelines] warning: git clone failed" | |
| fi | |
| echo "[agent-guidelines] warning: git clone failed" | |
| exit 1 | |
| fi |
🤖 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/sync-guidelines.sh` around lines 52 - 53, The current git clone
failure branch in the scripts/sync-guidelines.sh script only echoes a warning
and exits successfully; change that branch to fail fast by returning a non-zero
exit code: update the failure handling after the git clone command (the
conditional that currently runs echo "[agent-guidelines] warning: git clone
failed") to log an error and call exit 1 so the script stops with a failure when
the initial clone fails.
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
1 participant
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.
Introduces shared AI agent guidelines from the central simplic-oxs-agent-skill repository.
This branch includes AGENTS.md, CLAUDE.md, sync scripts, and the .agent-guidelines folder for daily guideline updates.
Summary by CodeRabbit
New Features
Documentation
Chores