Onboard simplic-oxs-agent-skill - #45
Merged
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThis PR adds cross-platform synchronization for shared AI-agent guidelines and configures Claude Code, GitHub Copilot, and generic agents to use them. It also adds public cleaning-order client models, enums, fields, and a package version update. ChangesAgent Guidelines Synchronization
Cleaning Client Model Expansion
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.gitignore:
- Line 365: The `.gitignore` contains a lone entry "pat" that looks accidental
or unclear; either remove this entry if it's not needed or replace it with a
precise ignore pattern and a short comment explaining what "pat" refers to
(e.g., specific file or artifact), so future reviewers understand its
purpose—update the `.gitignore` entry named "pat" accordingly.
In `@scripts/sync-guidelines.ps1`:
- Around line 59-66: The current removal calls Remove-Item -Recurse -Force $Dir
without verifying $Dir is a safe target; before calling Remove-Item, add
validation on $Dir (e.g., ensure it's not null/empty, not a root drive like "\"
or "C:\", has a minimum length, and resolves under an expected base directory or
repo-specific path) by using Resolve-Path and checking the resolved path is a
child of the known base (or matches an allowed pattern); only proceed with
Remove-Item when these checks pass, otherwise Write-Host an error and abort the
clone step. Ensure you update the branch where Test-Path $Dir and Remove-Item
are used, referencing $Dir, Remove-Item, Test-Path and the clone/Invoke-Git
block so the validation runs immediately before the destructive Remove-Item
call.
In `@scripts/sync-guidelines.sh`:
- Around line 46-53: The script currently runs rm -rf "$DIR" blindly; add a
safety validation before that removal: check that DIR is not empty, not "/", not
the filesystem root or user home, and ideally resolve realpath and confirm it is
under an expected base directory or matches an expected repo-specific pattern;
if the check fails, abort the sync and log a clear warning instead of removing
anything. Apply this validation immediately before the rm -rf line that
references DIR in the scripts/sync-guidelines.sh git-clone block, and only
proceed to delete and clone when the validation passes (otherwise exit non-zero
after logging).
🪄 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: 5949e5fa-a6ca-49df-9745-68412da79f74
📒 Files selected for processing (7)
.claude/settings.json.github/copilot-instructions.md.gitignoreAGENTS.mdCLAUDE.mdscripts/sync-guidelines.ps1scripts/sync-guidelines.sh
| FodyWeavers.xsd | ||
|
|
||
| pat No newline at end of file | ||
| pat |
There was a problem hiding this comment.
Clarify or remove the unrelated pat entry.
This line appears unrelated to the PR's objective of onboarding agent guidelines. Is pat a project-specific artifact that needs ignoring, or was it added accidentally?
🤖 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 @.gitignore at line 365, The `.gitignore` contains a lone entry "pat" that
looks accidental or unclear; either remove this entry if it's not needed or
replace it with a precise ignore pattern and a short comment explaining what
"pat" refers to (e.g., specific file or artifact), so future reviewers
understand its purpose—update the `.gitignore` entry named "pat" accordingly.
Comment on lines
+59
to
+66
| # Clone the repo for the first time | ||
| if (Test-Path $Dir) { Remove-Item -Recurse -Force $Dir } | ||
| if (Invoke-Git @('clone', '--quiet', '--depth', '1', '--branch', $Branch, $Repo, $Dir)) { | ||
| Set-Content -Path $stamp -Value $today -Encoding utf8 -NoNewline | ||
| Write-Host "[agent-guidelines] synced from $Repo ($Branch) -> $Dir @ $today" | ||
| } else { | ||
| Write-Host "[agent-guidelines] warning: git clone failed" | ||
| } |
There was a problem hiding this comment.
Add safety validation before destructive directory removal.
Line 60 performs Remove-Item -Recurse -Force $Dir without validating that $Dir is a safe, expected path. If $Dir is misconfigured (empty string, unintended path, etc.), this could delete unintended directories.
🛡️ Proposed fix to add path validation
} else {
# Clone the repo for the first time
- if (Test-Path $Dir) { Remove-Item -Recurse -Force $Dir }
+ if (Test-Path $Dir) {
+ if ([string]::IsNullOrWhiteSpace($Dir) -or $Dir -eq '/' -or $Dir -eq '\') {
+ Write-Host "[agent-guidelines] error: invalid directory path '$Dir'"
+ exit 1
+ }
+ Remove-Item -Recurse -Force $Dir
+ }
if (Invoke-Git @('clone', '--quiet', '--depth', '1', '--branch', $Branch, $Repo, $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
| # Clone the repo for the first time | |
| if (Test-Path $Dir) { Remove-Item -Recurse -Force $Dir } | |
| if (Invoke-Git @('clone', '--quiet', '--depth', '1', '--branch', $Branch, $Repo, $Dir)) { | |
| Set-Content -Path $stamp -Value $today -Encoding utf8 -NoNewline | |
| Write-Host "[agent-guidelines] synced from $Repo ($Branch) -> $Dir @ $today" | |
| } else { | |
| Write-Host "[agent-guidelines] warning: git clone failed" | |
| } | |
| # Clone the repo for the first time | |
| if (Test-Path $Dir) { | |
| if ([string]::IsNullOrWhiteSpace($Dir) -or $Dir -eq '/' -or $Dir -eq '\') { | |
| Write-Host "[agent-guidelines] error: invalid directory path '$Dir'" | |
| exit 1 | |
| } | |
| Remove-Item -Recurse -Force $Dir | |
| } | |
| if (Invoke-Git @('clone', '--quiet', '--depth', '1', '--branch', $Branch, $Repo, $Dir)) { | |
| Set-Content -Path $stamp -Value $today -Encoding utf8 -NoNewline | |
| Write-Host "[agent-guidelines] synced from $Repo ($Branch) -> $Dir @ $today" | |
| } else { | |
| Write-Host "[agent-guidelines] warning: git clone failed" | |
| } |
🤖 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 59 - 66, The current removal calls
Remove-Item -Recurse -Force $Dir without verifying $Dir is a safe target; before
calling Remove-Item, add validation on $Dir (e.g., ensure it's not null/empty,
not a root drive like "\" or "C:\", has a minimum length, and resolves under an
expected base directory or repo-specific path) by using Resolve-Path and
checking the resolved path is a child of the known base (or matches an allowed
pattern); only proceed with Remove-Item when these checks pass, otherwise
Write-Host an error and abort the clone step. Ensure you update the branch where
Test-Path $Dir and Remove-Item are used, referencing $Dir, Remove-Item,
Test-Path and the clone/Invoke-Git block so the validation runs immediately
before the destructive Remove-Item call.
Comment on lines
+46
to
+53
| # Clone the repo for the first time | ||
| rm -rf "$DIR" | ||
| if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; then | ||
| printf '%s' "$TODAY" > "$STAMP" | ||
| echo "[agent-guidelines] synced from $REPO ($BRANCH) -> $DIR @ $TODAY" | ||
| else | ||
| echo "[agent-guidelines] warning: git clone failed" | ||
| fi |
There was a problem hiding this comment.
Add safety validation before destructive directory removal.
Line 47 performs rm -rf "$DIR" without validating that $DIR is a safe, expected path. If $DIR is misconfigured (empty string, "/" etc.), this could delete unintended directories.
🛡️ Proposed fix to add path validation
else
# Clone the repo for the first time
- rm -rf "$DIR"
+ if [[ -z "$DIR" ]] || [[ "$DIR" == "/" ]] || [[ "$DIR" == "." ]]; then
+ echo "[agent-guidelines] error: invalid directory path '$DIR'"
+ exit 1
+ fi
+ rm -rf "$DIR"
if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; 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.
Suggested change
| # Clone the repo for the first time | |
| rm -rf "$DIR" | |
| if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; then | |
| printf '%s' "$TODAY" > "$STAMP" | |
| echo "[agent-guidelines] synced from $REPO ($BRANCH) -> $DIR @ $TODAY" | |
| else | |
| echo "[agent-guidelines] warning: git clone failed" | |
| fi | |
| # Clone the repo for the first time | |
| if [[ -z "$DIR" ]] || [[ "$DIR" == "/" ]] || [[ "$DIR" == "." ]]; then | |
| echo "[agent-guidelines] error: invalid directory path '$DIR'" | |
| exit 1 | |
| fi | |
| rm -rf "$DIR" | |
| if git clone --quiet --depth 1 --branch "$BRANCH" "$REPO" "$DIR" 2>/dev/null; then | |
| printf '%s' "$TODAY" > "$STAMP" | |
| echo "[agent-guidelines] synced from $REPO ($BRANCH) -> $DIR @ $TODAY" | |
| else | |
| echo "[agent-guidelines] warning: git clone failed" | |
| 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 46 - 53, The script currently runs
rm -rf "$DIR" blindly; add a safety validation before that removal: check that
DIR is not empty, not "/", not the filesystem root or user home, and ideally
resolve realpath and confirm it is under an expected base directory or matches
an expected repo-specific pattern; if the check fails, abort the sync and log a
clear warning instead of removing anything. Apply this validation immediately
before the rm -rf line that references DIR in the scripts/sync-guidelines.sh
git-clone block, and only proceed to delete and clone when the validation passes
(otherwise exit non-zero after logging).
Added new internal models/enums for cleaning orders: resources, codes, chambers, contacts, procedures, status, and ownership. Extended InternalCleaningModel with detailed properties and XML docs. Bumped project version to 1.1.526.820.
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