Repository navigation
fix(ci): stop labelling detached scans as main; upload the PR head sha (#98) - #112
Merged
Merged
Conversation
#98) - detectRepoUrl no longer falls back to "main": a detached HEAD (how actions/checkout leaves a pull_request run) or a missing git yields branch "", and a missing remote no longer discards the real branch. - fileLink takes a ref; the HTML report links to meta.git.sha when the report carries it (permalinks), else the branch, else plain text — never /blob//. - --upload attributes the report to pull_request.head.sha on PR events, else GITHUB_SHA, else the local HEAD, so the quality-gate status lands on the PR head instead of GitHub's merge commit. - GitHub event payload fixtures (push, new-branch push, pull_request, pull_request_target, merge_group, workflow_dispatch) for these tests and the CI-context work that follows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88
VibeCode QAPR #112 ( No workflow found. Add this to enable automatic PR scanning: name: VibeCode QA
on:
pull_request:
workflow_dispatch:
permissions:
contents: read
pull-requests: write
security-events: write
jobs:
scan:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: vibecodeqa/action@v1
with:
fail-under: "70" |
This was referenced Oct 5, 2026
serge-ivo
commented
Oct 5, 2026
serge-ivo
left a comment
Contributor
Author
There was a problem hiding this comment.
Adversarial review, round 1: #112 (cli#98 steps 1–2)
Verdict: READY. Every finding below is minor or a nit, and #113 resolves each of them. This holds as long as #113 merges right after this PR.
Verified at 1b3b633: tsc clean, biome check src/ clean, vitest run 752/752 passed (dist/ built first, since cli.test.ts spawns dist/cli.js). Also merged in stack order with #113/#114/#116/#115; see the #113 review for the result.
Findings
- minor, CONFIRMED.
src/report/html.ts:58/components.ts:19-21: on this PR alone nothing writesmeta.git, solinkRefalways falls back tometa.branch. A detached-HEAD scan (everypull_requestcheckout, and local detached checkouts) now shows no file links at all. Acceptance criterion 3 ("HTML file links use the sha") is met only once #113 populatesmeta.git.sha. That is fine for a stacked merge, but don't release from this commit alone. - minor, CONFIRMED.
src/upload.tscurrentGitSha: the PR head from the payload wins for any event that haspull_request, includingpull_request_target. On that event the default checkout is the base branch, so the scan of base code gets attributed to the PR head, and the quality-gate status lands on the PR for code it didn't scan.GITHUB_SHAis also read even whenGITHUB_ACTIONSis unset, so an env leaked from a parent CI into a local run wins over the localHEAD. Both are fixed by #113'sdetectCiContext. Merge them together. - nit. The
merge_group.jsonfixture is only exercised by #113's tests. Fine as is.
Not a problem:
meta.branch = ""passes schema 0.5.0 (branch: z.string()).- github-app's
persistReportnever readsmeta.branch. - The upload
shachange matches whatpostQualityGateStatusexpects (github-app/src/index.ts).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #98: steps 1–2, no schema change. Step 3 (
detectCiContext→meta.git/meta.ci) is stacked on this branch in a follow-up PR.What changed (1b3b633)
detect.ts: a detached HEAD or a missing git repo now givesbranch: "". It used to give"main". A missing remote no longer discards the real local branch.report/components.ts:fileLinktakes a ref (sha or branch). An empty ref renders plain text, so there is no more/blob//. NewlinkRef(meta)prefersmeta.git.sha, then the branch, then"".report/html.ts: useslinkRef.upload.ts: thecurrentGitShasent by--uploadis the PR head sha on PR events, elseGITHUB_SHA, elseHEAD. The quality-gate status therefore lands on the PR head, not GitHub's merge commit.fixtures/github-events/holdspush,push-new-branch,pull_request,pull_request_target,merge_groupandworkflow_dispatch.Verification
pnpm build: clean.pnpm lint: 0 errors (62 warnings, same as the baseline on main).pnpm test: 64 files, 752/752 passing (main baseline 739, plus 13 new).detect.test.ts› "never guesses main on a detached HEAD".pages.test.ts› "report file links" block.upload.test.ts› "uses the PR head sha on pull_request, not the merge sha".envor saves and restoresGITHUB_*, so the suite is safe under Actions.Note: with only this PR merged, a detached build outside CI has no sha in its meta, so its file links render as plain text. The stacked step-3 PR adds
meta.git.sha, which turns them into permalinks.🤖 Generated with Claude Code
https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88