Skip to content

fix(ci): build check-affected's uncommitted snapshot outside .git - #8553

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

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

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #8425

Summary

Issue-lane run 39382060403 (#8530), the first admission after #8520, stopped on check-affected again: could not create directory of .git/worktrees/tree: Read-only file system, with or without the agent sandbox. --uncommitted built its snapshot with git worktree add, which writes .git/worktrees/, read-only in the lane. Admissions are held until this lands.

Fix

scripts/check-affected.sh --uncommitted (:103-148) now writes nothing under .git:

  • new objects go to $snap/objects, with the repository's object directory as an alternate; the temporary index works as before;
  • the commit is checked out in a scratch repository $snap/tree whose alternates list both object directories (a shallow clone's boundary file is copied in);
  • the outer run resolves --base to a commit SHA (fetching first only for a remote/branch base without --no-fetch) and passes --base <sha> --no-fetch to the inner run; a failed fetch still skips stale-base-overlap there.

scripts/lib/gate-entry.test.sh gains one allowlist row for the new SHA resolution, in the same shape as check-stale-base-overlap.sh's.

Verification

  • New bwrap case mounts the whole .git read-only: failed before the fix (Read-only file system), passes after; scripts/check-affected.test.sh 38/38, including the device-node case and "leaves HEAD, the index and the worktree list as they were".
  • shellcheck clean. check-affected.sh --tests: 9 passed; check-script-contract.test.sh fails 2 cases only because htmlhint is not installed in this fresh worktree (no npm ci), unrelated.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT

In the issue lane .git/worktrees is read-only, so --uncommitted died at
`git worktree add`. The snapshot now writes its objects to a temporary
object directory that borrows the repository's objects as an alternate,
and checks the commit out in a scratch repository under the same temp
directory. The base is fetched and resolved to a commit in the outer run,
and the inner run gets --no-fetch; a failed outer fetch still skips the
stale-base check. A shallow clone's boundary is copied across.

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 07:03
@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-20T07:05:34.090148Z aa6639f 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 aa6639fb382d62c2040196ade4e0e69814ab399e: 0 finding(s), 0 as inline comments.

Security review: PR #8553 (head aa6639f)

No security issues found. I posted no inline comments.

The PR changes scripts/check-affected.sh, its test, and the BASE_REF_ALLOW allowlist in scripts/lib/gate-entry.test.sh. It moves the --uncommitted snapshot out of .git and into a scratch repository. I read the diff only and did not run the new read-only .git test.

  • Command injection: The new git fetch "${base%%/*}" "${base#*/}" in the --uncommitted path repeats an existing pattern at the later fetch in the same script. base is a --base argument from whoever runs the script, not untrusted PR or issue text, and all expansions are quoted. A ---prefixed --base value is not a new exposure because the same script already accepted it.
  • Environment handling: GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES are exported only for the staging steps and unset before the scratch repository is created. The inner run therefore inherits no redirected object store. CHECK_AFFECTED_FETCH_FAILED only turns off the stale-base check, so it grants no access.
  • Scratch repository:
    • It is created with --template= and checked out with core.hooksPath=/dev/null, so no hooks run during checkout.
    • The alternates file lists only the snapshot's object directory and the checkout's own, and the shallow file is copied from the checkout.
    • The inner script comes from $tools, not from the snapshot, so the snapshot's contents cannot replace the checker.
  • Temporary files: mktemp -d creates the directory with owner-only permissions, and the cleanup trap removes only $snap.
  • Instruction surfaces: The diff deletes no standing instruction, hook block list or allowlist. The only allowlist change adds one entry to BASE_REF_ALLOW in gate-entry.test.sh, for a git rev-parse that captures a resolved sha. That test file is a lint-style test, not a control that blocks anything at runtime. I did not consult the instruction exception register, since no instruction text was removed or softened.
  • Actions hardening: No workflow, permission or action-pin changes are in the diff.

@github-actions

Copy link
Copy Markdown
Contributor

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

Code review: PR #8553 (head aa6639f)

No findings. I reviewed all three files changed in this PR.

Scope. scripts/check-affected.sh --uncommitted now builds its snapshot without writing under .git. New objects go to $snap/objects. The commit is checked out in a scratch repository whose alternates list both object directories. The outer run resolves --base to a SHA and passes --no-fetch to the inner run.

What I read and traced:

  • Fetch handling: The outer fetch failure is carried to the inner run through CHECK_AFFECTED_FETCH_FAILED. That keeps stale-base-overlap skipping as it did before. Only a remote/branch base is fetched, and --no-fetch is still respected.
  • SHA base in the inner checks: check-changelog-fragments.sh (--check-required) and check-stale-base-overlap.sh resolve the base ref with rev-parse and merge-base. A commit SHA works for both, with objects reachable through the alternates. Only the messages and repro commands show a SHA instead of origin/main.
  • Snapshot staging: GIT_INDEX_FILE is now exported before the git ls-files listing. The copied index has the same content as the real one, so the listing is unchanged.
  • Test and allowlist: The new bwrap case runs against a read-only .git. The added allowlist row in gate-entry.test.sh matches the new rev-parse line.

I did not run the test suite or bwrap here. The PR's results (38/38 passing, the case failing before the fix) are author-claimed and unverified by me.

Closing issue. The PR uses Refs: #8425, so it closes nothing and has no acceptance criteria to check. No .claude/rules/ file matches the changed paths beyond the shell-script ones, and none of them is broken.

Edge cases I judged too minor to report:

  • A : in the object-directory path would break GIT_ALTERNATE_OBJECT_DIRECTORIES.
  • The caller's own GIT_OBJECT_DIRECTORY or GIT_ALTERNATE_OBJECT_DIRECTORIES are cleared by the unset after the commit.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 730f858 Oct 20, 2026
57 checks passed
@kyle-sexton
kyle-sexton deleted the fix/check-affected-readonly-git branch October 20, 2026 07:23
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