Skip to content

fix(ci): skip device nodes when check-affected stages uncommitted work - #8520

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/check-affected-nonregular
Oct 20, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/check-affected-nonregular

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #8425

Summary

Issue-lane runs fail check-affected before any check runs: the lane's sandbox bind-mounts /dev/null over dotfiles in the checkout (.bash_profile, .bashrc, .gitconfig, .vscode and others), and check-affected --uncommitted staged the work tree with git add -A, which refuses a device node (can only add regular files, symbolic links or git-directories). This blocks every issue-lane admission.

Fix

scripts/check-affected.sh --uncommitted now stages only regular files, symlinks, repositories and deletions from git ls-files --modified --others --exclude-standard. A masked tracked file keeps its committed content; when nothing qualifies, nothing is added (an empty pathspec would make add -A stage everything). No workflow or sandbox change.

Verification

  • bash scripts/check-affected.test.sh: PASS=37 FAIL=0. The new case masks one tracked and one untracked file with bwrap; before the fix it fails with rc=2 and the git error, after it passes. It skips when bwrap cannot start.
  • Reproduced the failure locally with bwrap --ro-bind /dev/null <file>; the all-masked case exits 0 after the fix.
  • shellcheck clean on both files.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT

The issue lane's sandbox bind-mounts /dev/null over dotfiles in the checkout
(.bash_profile, .bashrc, .gitconfig, .vscode and others). check-affected
--uncommitted staged the work tree with `git add -A`, which refuses a device
node, so every issue-lane run's check-affected failed before any check ran.

Stage only regular files, symlinks, repositories and deletions; a masked
tracked file keeps its committed content. The test masks a tracked and an
untracked file with bwrap and skips when bwrap cannot start.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 20, 2026 04:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T04:44:47.224287Z 4c00dfa Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 4c00dfac2ceaf21ccaaea6cfe223e3b1abcc1187: 0 finding(s), 0 as inline comments.

No security issues found in PR #8520 (scripts/check-affected.sh and its test).

  • Injection: Paths flow from git ls-files -z through a NUL-delimited pathspec file to git add --pathspec-from-file. No shell or eval interpolation of filenames, so there is no command injection.
  • Glob or pathspec-magic names: These could stage more files than intended. That only touches the same working-tree files the previous git add -A already staged, into a temporary snapshot index and worktree. No privilege or trust boundary is crossed.
  • Empty list: The guard against an empty pathspec is correct, since add -A with no pathspec would stage everything.
  • Symlinks and devices: Symlinks are staged as links, not their targets, and device nodes are skipped. That is the intended behavior.
  • Out of scope: The diff changes no workflow triggers, permissions, tokens, action pins or standing instructions, so the Actions and instruction-surface lenses do not apply. I did not consult the instruction exception register, because no instruction was removed.
  • Test: The bwrap case is read-only and skips itself when bwrap cannot start.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 4c00dfac2ceaf21ccaaea6cfe223e3b1abcc1187: 0 finding(s), 0 as inline comments.

Code review: no findings

I read the full diff, the PR context and the surrounding code in scripts/check-affected.sh and scripts/check-affected.test.sh. I did not run the test suite. The PR's claimed PASS=37 FAIL=0 and shellcheck results are author-claimed and unverified by me.

What I checked

  • Staging filter (scripts/check-affected.sh:110-120): the filter keeps regular files, symlinks (including broken ones), directories (nested repos and gitlinks) and missing paths (deletions). It drops device nodes, fifos and sockets.
  • Deletions and index state: tracked deletions still reach git add -A, so they are staged. Files whose worktree matches the copied index are not listed, and their index entries carry over unchanged.
  • Empty list: the empty-array guard is correct. An empty pathspec list would make add -A stage everything.
  • Masked tracked file: it stays at its committed content in the snapshot, as the PR says.
  • Test (scripts/check-affected.test.sh:252-267): it masks one tracked file (notes.txt) and one untracked file (.bash_profile). It then asserts that new-tool is still checked and that git's "can only add regular files" error is absent. notes.txt is committed with content data. The bwrap probe skips the case cleanly when bwrap is missing or cannot start. Cleanup restores the state the final state assertion compares against.
  • Scope: I looked for a .claude/rules rule matching the changed paths and found none that applies. The PR closes no issue (Refs: #8425), so there are no acceptance criteria to check.

Minor observations (not blocking, no inline comment)

  • git add --pathspec-from-file parses each entry as a pathspec, so a file named with glob characters (such as * or [) could also match other paths. That includes a masked device node, which would bring the original error back. Git tries an exact match first, and this is very unlikely in practice. Setting GIT_LITERAL_PATHSPECS=1 on that git add would remove the case.
  • If the sandbox ever masked a directory that holds tracked files, the files under it would look deleted, and the snapshot would stage their deletion. No .vscode is tracked in this repo (git ls-files .vscode returned nothing), so this is hypothetical today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant