fix(claude-review): don't re-fetch a base commit the checkout already has

[crowecawcaw]

What was the problem/requirement? (What/Why)

Enabling the Claude PR-review workflows on a private repo.

Run git fetch --depth=1 origin "$BASE_SHA"
  BASE_SHA: ae90810027af2dfca00cfd14456432128aba9c35
fatal: could not read Username for 'https://github.com': No such device or address
##[error]Process completed with exit code 128.

The step order shows how far it got — everything security-relevant worked, then it tripped on step 6:

Step	Result
Resolve and validate PR from trusted head SHA	✅
Collect prior review state	✅
Checkout PR head (read-only)	✅
Strip symlinks from PR head	✅
Fetch base commit for diff	❌
Install token generator / OIDC / mint token / agent	⏭️ skipped

Cause: the PR-head checkout sets persist-credentials: false deliberately, so no token is left on disk for the agent to read. That also leaves no credential for a later git fetch. Public repos fetch anonymously and never notice; a private repo has no anonymous access, so the step fails.

The fetch was redundant anyway. That same checkout uses fetch-depth: 0, so the base commit is normally already in the clone. In the failing run the log shows * [new branch] main -> origin/main, and ae90810 is main's tip — the job died on an unauthenticated fetch of an object it already had.

What was the solution? (How)

Only fetch when the base commit genuinely is not present:

git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null \
  || git fetch --depth=1 origin "$BASE_SHA"

This keeps persist-credentials: false intact and introduces no token handling into the step. I deliberately preferred this over passing a one-shot http.extraheader credential: it is the smaller change to a security-sensitive shared workflow, and it needs no secret in a step that runs before the agent.

^{commit} rather than a bare SHA so a tree/blob SHA cannot satisfy the check.

What is the impact of this change?

• Public repos (deadline-cloud, deadline-cloud-worker-agent, deadline-cloud-samples, deadline-cloud-for-blender): no behavior change. The base commit is already present, so where they previously ran a successful redundant fetch they now skip it. Marginally faster.
• Private repos: the review stage can now proceed past this step.

How was this change tested?

zizmor against this repo's own zizmor.yml: no findings.

Verified the shell logic in a scratch repo across all three paths, checking exit codes rather than just output:

Case	Expected	Actual
Base commit present	skip fetch, exit 0	exit 0, no fetch
Base commit absent	fall through to fetch	fetch attempted
Absent and fetch fails	step fails loudly	exit 128
Tree SHA instead of commit SHA	rejected by ^{commit}	exit 128

The third case is the one that matters for not masking real problems: set -euo pipefail plus || only swallows the probe's failure, not the fetch's, so a genuinely missing object still fails the step.

Was this change documented?

Yes — a comment on the step explains why the fetch is conditional, so it is not "simplified" back into an unconditional fetch later.

Does this PR introduce new dependencies?

No.

Is this a breaking change?

No.

Does this change impact security?

No weakening. The change avoids introducing a credential rather than adding one:

• persist-credentials: false is unchanged, so still no token on disk for the agent.
• No secret is added to this step's environment.
• The alternative fix (an authenticated fetch) would have put a token into a pre-agent step; this avoids that entirely.
• Failure modes are unchanged: a genuinely unavailable base commit still fails the step rather than silently reviewing against the wrong base.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.