Ignore scripts/, CLAUDE.md, .gitignore in auto-tag filter - #373
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new GitHub Actions workflow for automated daily version bumping and introduces Claude Code configuration. The workflow monitors the dev branch for meaningful changes (excluding documentation, tests, CI configs, scripts, and metadata files) and automatically increments the build number in the kernel driver version when changes are detected.
Changes:
- Creates an auto-tag workflow that runs daily to check for meaningful driver changes and bump versions accordingly
- Adds Claude Code permissions configuration to control allowed/denied commands in the development environment
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
.github/workflows/auto-tag.yml |
New workflow for automated version bumping on dev branch with filtering for non-driver changes |
.claude/settings.json |
New Claude Code permissions configuration allowing development commands while restricting dangerous operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Filter out ignored paths | ||
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/)' || true) | ||
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/|scripts/|CLAUDE\.md$|\.gitignore$)' || true) |
There was a problem hiding this comment.
The pattern \.gitignore$ only matches the root-level .gitignore file. If a .gitignore file is added or modified in a non-ignored subdirectory (e.g., kernel/.gitignore, utilities/.gitignore), it will be considered a "meaningful change" and trigger a version bump.
While .gitignore files in already-ignored directories like test/ and scripts/ are correctly excluded (since those directory patterns match first), .gitignore files in other directories would trigger bumps. If the intent is to ignore all .gitignore files regardless of location, consider using a pattern like (^|/)\.gitignore$ or simply \.gitignore (without anchors) to match any path ending with .gitignore.
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/|scripts/|CLAUDE\.md$|\.gitignore$)' || true) | |
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/|scripts/|CLAUDE\.md$|.*\.gitignore$)' || true) |
| id: bump | ||
| run: | | ||
| current="${{ steps.version.outputs.current }}" | ||
| IFS='.' read -r major minor patch build <<< "$current" |
There was a problem hiding this comment.
The version parsing assumes a 4-part version number (major.minor.patch.build), but there's no validation that the version string actually has 4 parts. If the MODULE_VERSION in d4xx.c has fewer than 4 parts (e.g., "1.0.2"), the read command will leave build empty, and new_build=$((build + 1)) will result in new_build=1 instead of failing. This could silently introduce incorrect version numbers.
Consider adding validation after the IFS read to ensure all four variables are populated, for example:
if [ -z "$major" ] || [ -z "$minor" ] || [ -z "$patch" ] || [ -z "$build" ]; then
echo "::error::Version '$current' must have exactly 4 parts (major.minor.patch.build)"
exit 1
fi| IFS='.' read -r major minor patch build <<< "$current" | |
| if [ -z "$major" ] || [ -z "$minor" ] || [ -z "$patch" ] || [ -z "$build" ]; then | |
| echo "::error::Version '$current' must have exactly 4 parts (major.minor.patch.build)" | |
| exit 1 | |
| fi |
| run: | | ||
| current="${{ steps.version.outputs.current }}" | ||
| new_version="${{ steps.bump.outputs.new_version }}" | ||
| sed -i "s/MODULE_VERSION(\"${current}\")/MODULE_VERSION(\"${new_version}\")/" kernel/realsense/d4xx.c |
There was a problem hiding this comment.
The sed command uses unescaped special regex characters in the replacement pattern. If the version number contains characters that have special meaning in sed's replacement context (like &, /, or \), the sed command could behave unexpectedly. While version numbers typically don't contain such characters, it's a good practice to escape the variables or use a different delimiter.
Consider using a delimiter that won't appear in version strings, or escape the variables properly. For example:
sed -i "s|MODULE_VERSION(\"${current}\")|MODULE_VERSION(\"${new_version}\")|" kernel/realsense/d4xx.cUsing | as the delimiter avoids conflicts with / if it ever appears in version strings.
| escaped_new_version=${new_version//&/\\&} | |
| sed -i "s|MODULE_VERSION(\"${current}\")|MODULE_VERSION(\"${escaped_new_version}\")|" kernel/realsense/d4xx.c |
|
|
||
| # Filter out ignored paths | ||
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/)' || true) | ||
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/|scripts/|CLAUDE\.md$|\.gitignore$)' || true) |
There was a problem hiding this comment.
The PR description states "Adds scripts/, CLAUDE.md, and .gitignore to the ignored paths in the auto-tag version bump workflow", which implies these are being added to an existing workflow. However, this appears to be a new file (based on the diff showing all lines as additions starting from line 1).
If this is indeed a new file, the PR description should clarify that this is creating a new auto-tag workflow that includes these paths in the ignore list, rather than adding them to an existing workflow. If this is modifying an existing file, please verify that the diff is correctly showing the changes.
| - name: Push commit | ||
| if: steps.changes.outputs.has_changes == 'true' | ||
| run: git push origin dev | ||
|
|
||
| - name: Push tag | ||
| if: steps.changes.outputs.has_changes == 'true' | ||
| run: git push origin "${{ steps.bump.outputs.new_tag }}" | ||
|
|
There was a problem hiding this comment.
The workflow pushes the commit and tag in separate steps (lines 118-124). If the commit push succeeds but the tag push fails, the repository will be in an inconsistent state: the version bump commit exists on the remote, but the corresponding tag doesn't.
Consider combining the push operations into a single step, or add error handling to ensure both succeed or both fail. For example:
git push origin dev && git push origin "$new_tag" || {
echo "::error::Failed to push. Repository may be in inconsistent state."
exit 1
}Alternatively, push both in a single command: git push origin dev "$new_tag"
| - name: Push commit and tag | |
| if: steps.changes.outputs.has_changes == 'true' | |
| run: | | |
| new_tag="${{ steps.bump.outputs.new_tag }}" | |
| git push origin dev && git push origin "$new_tag" || { | |
| echo "::error::Failed to push commit and tag. Repository may be in inconsistent state." | |
| exit 1 | |
| } |
There was a problem hiding this comment.
The .claude/settings.json file is included in this PR, but the PR description only mentions changes to the auto-tag workflow filter. This file doesn't appear to be related to the stated purpose of the PR (adding ignored paths to the auto-tag workflow).
If this file is being added as part of setting up the Claude Code environment and is unrelated to the auto-tag changes, it should either be in a separate PR or the PR description should be updated to mention it. Including unrelated changes in a single PR makes it harder to review and understand the purpose of each change.
| meaningful=$(echo "$changed" | grep -v -E '^(\.claude/|\.github/|test/|docs/|scripts/|CLAUDE\.md$|\.gitignore$)' || true) | ||
|
|
||
| if [ -z "$meaningful" ]; then | ||
| echo "Only ignored-folder changes since $tag:" |
There was a problem hiding this comment.
The comment "Only ignored-folder changes" is misleading because the ignored paths include specific files (CLAUDE.md, .gitignore) in addition to folders. Consider updating to "Only ignored-path changes" or "Only non-meaningful changes" for accuracy.
| echo "Only ignored-folder changes since $tag:" | |
| echo "Only ignored-path changes since $tag:" |
| new_version="${{ steps.bump.outputs.new_version }}" | ||
| new_tag="${{ steps.bump.outputs.new_tag }}" | ||
|
|
||
| git config user.email "builder@example.com" |
There was a problem hiding this comment.
The git configuration uses "builder@example.com" as the commit author email. Using "example.com" might cause issues with some git hosting services or organizational policies that validate email domains. Consider using a noreply email address specific to your GitHub organization (e.g., "builder@users.noreply.github.com") or a valid domain email address.
| git config user.email "builder@users.noreply.github.com" |
|
|
||
| permissions: | ||
| contents: write | ||
|
|
There was a problem hiding this comment.
The workflow can run concurrently via scheduled runs and manual workflow_dispatch triggers. If two instances run simultaneously, they might:
- Both detect the same "latest tag"
- Both calculate the same "new version"
- Both try to commit and push, leading to conflicts
Consider adding a concurrency group to prevent multiple instances from running simultaneously:
concurrency:
group: auto-tag-version-bump
cancel-in-progress: falseThis ensures only one instance runs at a time, with subsequent triggers queued.
| concurrency: | |
| group: auto-tag-version-bump | |
| cancel-in-progress: false |
There was a problem hiding this comment.
The permissions allow unrestricted ssh * and scp * commands. These wildcards grant very broad access and could potentially be used to access unintended systems or transfer sensitive data. Consider restricting these to specific hosts or patterns that are necessary for the development workflow, for example:
Bash(ssh user@specific-host *)Bash(scp * user@specific-host:*)
This follows the principle of least privilege and reduces the attack surface if the Claude Code environment were to be compromised.
| "Bash(ssh user@specific-host *)", | |
| "Bash(scp * user@specific-host:*)", |
Summary
scripts/,CLAUDE.md, and.gitignoreto the ignored paths in the auto-tag version bump workflow🤖 Generated with Claude Code