Skip to content

ci(ui-preview-smoke): checkout base branch before agent runs#2240

Merged
teeohhem merged 1 commit into
mainfrom
teeohhem/ui-smoke-checkout-fix
May 7, 2026
Merged

ci(ui-preview-smoke): checkout base branch before agent runs#2240
teeohhem merged 1 commit into
mainfrom
teeohhem/ui-smoke-checkout-fix

Conversation

@teeohhem
Copy link
Copy Markdown
Contributor

@teeohhem teeohhem commented May 7, 2026

Summary

Fixes the brand-new ui-preview-smoke workflow (#2238). On its first run it failed with:

fatal: not in a git directory
Failed to configure git authentication
  at configureGitAuth (.../claude-code-action/v1/src/github/operations/git-config.ts:41:9)

claude-code-action configures git config user.name as one of its first steps and requires a valid working tree to run. The original workflow skipped actions/checkout because the agent only drives Playwright at the deployed preview URL and never needs the repo source — but the action itself does.

Adds actions/checkout@v4 pinned to the repo's default branch (github.event.repository.default_branch) before the agent step. We deliberately don't check out the PR head ref: this workflow runs under pull_request_target, and we'd rather not execute fork-supplied code in the runner. The base branch is sufficient to satisfy the action.

persist-credentials: false since the agent has its own github_token already.

How to test on Vercel preview

N/A — CI workflow change.

References

The new ui-preview-smoke workflow exits early with `fatal: not in a
git directory` because claude-code-action configures `git config
user.name` as one of its first steps and needs a valid working tree.

Add `actions/checkout@v4` pinned to the repo's default branch (not the
PR head ref) so we satisfy the action's requirement without executing
fork-supplied code under pull_request_target. The smoke test only
drives Playwright at the deployed Vercel preview, so PR code is never
needed in the runner.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hyperdx-oss Ignored Ignored May 7, 2026 7:27pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 7, 2026

⚠️ No Changeset found

Latest commit: 6e8cfb0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the review/tier-1 Trivial — auto-merge candidate once CI passes label May 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

🟢 Tier 1 — Trivial

Docs, images, lock files, or a dependency bump. No functional code changes detected.

Why this tier:

  • All files are docs / images / lock files

Review process: Auto-merge once CI passes. No human review required.
SLA: Resolves automatically.

Stats
  • Production files changed: 0
  • Production lines changed: 0
  • Branch: teeohhem/ui-smoke-checkout-fix
  • Author: teeohhem

To override this classification, remove the review/tier-1 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@teeohhem teeohhem merged commit 7743d22 into main May 7, 2026
18 checks passed
@teeohhem teeohhem deleted the teeohhem/ui-smoke-checkout-fix branch May 7, 2026 19:28
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

PR Review

✅ No critical issues found.

Small, well-scoped CI fix. A few notes:

  • 👍 Correct security posture — checking out default_branch instead of the PR head under pull_request_target avoids executing fork-supplied code with elevated permissions. persist-credentials: false is the right choice here.
  • 👍 Comment in the workflow clearly explains the why, which is useful for future maintainers since the checkout looks vestigial otherwise.
  • ℹ️ Minor: github.event.repository.default_branch is populated for both pull_request_target and workflow_dispatch, so the ref expression is safe across both triggers this workflow supports.
  • ℹ️ Optional follow-up (non-blocking): adding fetch-depth: 1 would shave a few seconds since the agent never inspects history — but the default is fine.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Deep Review

Scope: 1 file, +11 lines in .github/workflows/ui-preview-smoke.yml. Adds an actions/checkout@v4 step pinned to the default branch (with persist-credentials: false) before claude-code-action so the action has a valid git working tree under pull_request_target.

✅ No critical issues found. Conservative posture is preserved (no fork-head checkout, no credential persistence, no fork code execution path). Two compositional concerns worth considering before merge.

🟡 P2 -- recommended

  • .github/workflows/ui-preview-smoke.yml:39 -- The new base-branch checkout puts CLAUDE.md, AGENTS.md, and .claude/agents/* into $GITHUB_WORKSPACE; if claude-code-action runs claude from the workspace, the CLI auto-loads CLAUDE.md independently of --setting-sources user (which only governs settings.json discovery), so any future commit to main's CLAUDE.md silently rewrites the smoke agent's system prompt on every fork PR — a privilege-amplification path from "commit to main" to "fork-PR comment content posted under the bot identity."
    • Fix: Confirm whether claude-code-action invokes the CLI with cwd=$GITHUB_WORKSPACE; if it does, add rm -f CLAUDE.md AGENTS.md && rm -rf .claude after checkout, or check out into a dedicated subdirectory the action does not enter.
🔵 P3 nitpicks (1)
  • .github/workflows/ui-preview-smoke.yml:42 -- ref: ${{ github.event.repository.default_branch }} is a moving target — two runs of the same event id can see different trees if main advances mid-run, and PRs filed against release branches see a tree that does not match their actual base.
    • Fix: Switch to ref: ${{ github.event.pull_request.base.sha }} (resolving a sha for the workflow_dispatch path) so the working tree is reproducible from the event payload alone — also closes the racy interaction with the CLAUDE.md finding above.

Reviewers (7): correctness, security, adversarial, reliability, testing, maintainability, project-standards.

Testing gaps:

  • Pure CI YAML change — the workflow definition only takes effect on a future PR that touches packages/app/** or packages/common-utils/**, or via a manual workflow_dispatch. Confirm the fix end-to-end via dispatch (or a representative target PR) before relying on it.
  • No assertion that the workflow checks out a base-pinned ref; a future edit could silently switch ref: to pull_request.head.sha and reintroduce fork-code execution under pull_request_target. A lint pass (e.g., zizmor, or an actionlint rule) would catch this regression class.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

E2E Test Results

All tests passed • 165 passed • 3 skipped • 1203s

Status Count
✅ Passed 165
❌ Failed 0
⚠️ Flaky 4
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

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

Labels

review/tier-1 Trivial — auto-merge candidate once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant