Add Claude Code GitHub Workflow

[soundsnw]

🤖 Installing Claude Code GitHub App

This PR adds a GitHub Actions workflow that enables Claude Code integration in our repository.

What is Claude Code?

Claude Code is an AI coding agent that can help with:

• Bug fixes and improvements
• Documentation updates
• Implementing new features
• Code reviews and suggestions
• Writing tests
• And more!

How it works

Once this PR is merged, we'll be able to interact with Claude by mentioning @claude in a pull request or issue comment.
Once the workflow is triggered, Claude will analyze the comment and surrounding context, and execute on the request in a GitHub action.

Important Notes

• This workflow won't take effect until this PR is merged
• @claude mentions won't work until after the merge is complete
• The workflow runs automatically whenever Claude is mentioned in PR or issue comments
• Claude gets access to the entire PR or issue context including files, diffs, and previous comments

Security

• Our Anthropic API key is securely stored as a GitHub Actions secret
• Only users with write access to the repository can trigger the workflow
• All Claude runs are stored in the GitHub Actions run history
• Claude's default tools are limited to reading/writing files and interacting with our repo by creating comments, branches, and commits.
• We can add more allowed tools by adding them to the workflow file like:

allowed_tools: Bash(npm install),Bash(npm run build),Bash(npm run lint),Bash(npm run test)

There's more information in the Claude Code action repo.

After merging this PR, let's try mentioning @claude in a comment on any PR to get started!

[soundsnw]

@claude review scripts/fix-onedrive-filenames-apfs.sh for any bugs or issues

[claude]

Claude finished @soundsnw's task in 1m 52s —— View job

---

Code Review: scripts/fix-onedrive-filenames-apfs.sh

• Gather context and read script
• Analyze for bugs and issues
• Post review feedback

---

Overall this is a well-structured and thoughtful script. It handles edge cases like nested directories, APFS-specific backup, collision avoidance, and process cleanup. That said, I found two real bugs and several other issues worth addressing.

---

Bug 1 — exit 0 in error paths bypasses finish(), leaking the caffeinate process

Lines 199–202 and 233–235 call exit 0 directly when the APFS check fails or the OneDrive folder is missing. The trap at line 75 only covers HUP INT QUIT TERM, not EXIT. Since the explicit finish call (line 299) is never reached, the caffeinate process started at line 79 is never killed, and OneDrive is never reopened.

# Line 75 — EXIT is not trapped
trap finish HUP INT QUIT TERM

# Line 202 — caffeinate leaks here
exit 0

# Line 235 — caffeinate leaks here too
exit 0

Fix: add EXIT to the trap so finish is always called:

trap finish HUP INT QUIT TERM EXIT

Fix this →

---

Bug 2 — fix_leading_spaces only removes one leading space

Line 121: the sed substitution s/^[ \t]// removes exactly one leading space or tab. A filename like " report.docx" (two spaces) would become " report.docx" — still invalid — and requires running the script multiple times to fully fix.

# Line 121 — removes only one leading character
fixedname="$(echo "$name" | sed -e 's/^[ \t]//')"

Compare with fix_trailing_chars (line 95), which correctly uses + to remove all trailing spaces:

fixedname="$(echo "$name" | awk '{sub(/[ \t]+$/, "")};1')"

Fix: add + (or use *) to remove all leading spaces:

fixedname="$(echo "$name" | sed -e 's/^[ \t]*//')"

Fix this →

---

Issue — loggedinuser is not validated

Line 167: if scutil returns nothing (e.g., no one is logged in at the console), loggedinuser is empty. The script continues and constructs onedrivefolder="/Users//OneDrive". The mkdir, rm, chown, and find calls all proceed against this path without error, potentially doing unexpected things.

local -r loggedinuser="$(scutil <<<"show State:/Users/ConsoleUser" | awk '/Name :/ && ! /loginwindow/ { print $3 }')"
# No check that loggedinuser is non-empty before proceeding

Fix: validate immediately after assignment:

[[ -n "$loggedinuser" ]] || { echo "Could not determine logged-in user, aborting."; exit 0; }

Fix this →

---

Issue — Collision resolution can silently overwrite files

Lines 99, 125, 151: when a renamed file conflicts with an existing file, a random 6-digit suffix is appended. But mv -vf is used unconditionally, so if the randomly-named target also happens to exist, that file is silently overwritten with no warning in the log.

mv -vf "$line" "$path"'/'"$fixedname"'-'"$(jot -nr 1 100000 999999)" >>"$fixlog"

This is unlikely, but worth noting given that this script runs on user data being backed up.

---

Minor — Typo in error message

Line 234: "st up OneDrive" should be "set up OneDrive".

/usr/local/jamf/bin/jamf displayMessage -message "Cannot find the OneDrive folder. Ask IT to help st up OneDrive..."
#                                                                                                   ^^

Fix this →

---

Minor — O(n²) file processing

Each fix function reads the temp file line-by-line using sed -n "${counter}"p, which re-reads the file from the beginning for every line. For a OneDrive folder with many affected files this gets slow. Using mapfile to load the file into an array once would be more efficient, though this only matters at scale.

---

Summary

Severity	Issue
🔴 Bug	exit 0 in error paths leaks caffeinate (add EXIT to trap)
🔴 Bug	fix_leading_spaces only strips one leading space (missing */+)
🟡 Issue	loggedinuser not validated before use
🟡 Issue	Collision suffix resolution with mv -f can overwrite files
🟢 Minor	Typo "st up" → "set up" (line 234)
🟢 Minor	O(n²) line-by-line file reading

[soundsnw]

@claude for bug 2, is it better to use :space: than * ?

[claude]

Claude finished @soundsnw's task in 42s —— View job

---

• Gather context and answer question

---

[[:space:]] vs [ \t] for Bug 2

Yes, [[:space:]] is better than [ \t] — and the full fix should be:

fixedname="$(echo "$name" | sed -e 's/^[[:space:]]*//')"

Why [[:space:]] is preferable:

• \t inside a bracket expression ([ \t]) is not POSIX standard. In strict POSIX sed, it may match a literal backslash+t or just t rather than a tab, depending on the implementation. Even on macOS's BSD sed, relying on \t in bracket expressions is fragile.
• [[:space:]] is the correct POSIX character class for whitespace and is guaranteed to match spaces, tabs, and other horizontal/vertical whitespace across all compliant sed implementations.

One nuance: [[:space:]] also matches \n, \r, \f, and \v — not just spaces and tabs. For filenames this is harmless in practice (OneDrive filenames won't contain newlines), so [[:space:]] is the right call.

For consistency, the same fix applies to the existing fix_trailing_chars on line 95 — its awk pattern [ \t]+$ has the same portability caveat and could be written as [[:space:]]+$.

Fix this →

---