Repository navigation
fix(ci): honour config failUnder under --ci; print the gate's source (#108) - #109
Conversation
…108) `--ci` resolved the exit threshold as `flag ?? (ci ? 60 : config ?? 0)`, so a `failUnder` in .vcqa.json / package.json#vcqa was ignored in CI: a project asking for 80 passed at 61, one asking for 40 failed at 59. Precedence is now `--fail-under` -> config `failUnder` -> `--ci` default 60 -> 0, resolved in a pure `resolveFailUnder` (src/fail-under.ts). The CI output names the effective threshold and where it came from, e.g. "Passing: score 72 >= 80 (config)" / "Failing: score 70 < 80 (config)". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88
VibeCode QAPR #109 ( 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" |
serge-ivo
left a comment
There was a problem hiding this comment.
Adversarial review, round 1 (COMMENT only)
What I ran. I checked out head b4503a7 in a throwaway worktree and ran pnpm build, pnpm lint and pnpm test: build clean, lint 0 errors (62 warnings), 748/748 tests pass. I grepped every caller of the threshold, read src/commands/init.ts and vibecodeqa/action action.yml, and test-merged this branch with #110 and #111.
The core logic in resolveFailUnder is correct, and the precedence matches #108. --fail-under 0 and config failUnder: 0 both turn the gate off, and the output says so.
Findings
-
minor, CONFIRMED:
src/cli.test.ts:275. The "config below 60" end-to-end test passes on main's behaviour too. The test above it asserts the fixture scores above 60. On main the--cigate is 60, so this run also exits 0 there. The only thing that fails on main is the(config)text in the output. So "configfailUnder: 40, score 50, exit 0" from the acceptance criteria is covered only by the unit test.
Fix: add a fixture that scores below 60 (for example, seed it with lint or complexity findings) and assert that it exits 0 with configfailUnderset under its score. -
minor, CONFIRMED:
src/commands/init.ts:35and:88. The scaffold contradicts the new precedence.vcqa initwrites a workflow that runs--ci --fail-under 70and a.vcqa.jsonwithfailUnder: 60. Because the flag now always wins, the config value is dead in every scaffolded repo. Editing.vcqa.jsonchanges nothing in CI, and the two files disagree (70 vs 60).
Fix: drop--fail-under 70from the workflow template and put the intended number in.vcqa.json. That makes the config the single source. -
nit, CONFIRMED:
README.md:257-258still says--cimeans "exit 1 if score < 60".
Fix: use the same wording as the new--helptext. -
nit, CONFIRMED: this PR and #110 conflict in
src/cli.ts(theprintHelpflag block).git merge-tree pr109 pr110reports a content conflict. Whichever merges second needs a rebase. -
nit, pre-existing:
src/config.ts:61does not range-check configfailUnder."failUnder": "80"(a string) is silently dropped, and under--ciit falls back to 60. The new(ci default)source label makes this visible, which helps.150or-5is accepted as is.
Fix (optional): warn on a non-number or out-of-range value. -
follow-up, outside this repo, CONFIRMED from
vibecodeqa/actionaction.yml:76-79and:104-107. The composite action runs--jsonwithout--ci. It turns the CLI's exit code into a failure only when its ownfail-underinput is non-zero, and that input defaults to0. A configfailUndertherefore still does not gate CI for anyone using the action. It belongs in an action issue, not this PR.
Verdict: no blockers. Items 1 and 2 are worth fixing before or soon after merge.
The previous low-threshold test scanned a fixture that scores above 60, so it passed under the old CI default too. The fixture now scores below 60 and the test asserts it, so only the new precedence exits 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88
`vcqa init` wrote a workflow running `--ci --fail-under 70` next to a .vcqa.json with `failUnder: 60`. With the flag winning, the config value was dead and the two disagreed. The workflow now runs `--ci` and the config carries `failUnder: 70`, the gate the workflow used to enforce. README's --ci / --fail-under rows match the new --help text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88
VibeCode QAPR #109 ( 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" |
|
Round-1 review (5411684330) addressed in new commits; no force-push. Head is now 3016218.
Caveat: the Verified at 3016218: |
serge-ivo
left a comment
There was a problem hiding this comment.
Adversarial review, round 2 (final, COMMENT only)
What I ran, at head 3016218. I used a fresh worktree. Build is clean and lint shows 0 errors (62 warnings). pnpm test passes 65 files, 748/748.
I copied this branch's cli.test.ts into a main checkout and ran it against main's build. All 4 new --ci honours config failUnder tests fail there, as do the 2 updated init tests, so they test the change.
I then ran the built CLI over a full precedence matrix on a fixture that scores 69 (--skip-tests). That is 10 configs × 9 flag sets = 90 runs.
- Configs: none;
.vcqa.jsonat 50 / 80;package.json#vcqaat 50 / 80; both files; malformed JSON;"80"as a string;-5;0. - Flag sets: none;
--ci;--fail-under 50|80;--ci --fail-under 50|80|0|abc;--json --ci.
Round-1 findings
| # | Status | Evidence |
|---|---|---|
| 1 low-threshold test passed on main | FIXED | The new test exits 1 on main's build, because the fixture scores below 60. |
2 init scaffold contradicted precedence |
FIXED | vcqa init on a fresh dir writes run: npx @vibecodeqa/cli --ci --sarif --badge (parses as valid YAML) and .vcqa.json with failUnder: 70. Repos scaffolded earlier keep --fail-under 70 in their workflow, and the flag still wins, so their gate does not move. |
| 3 README | FIXED | — |
| 4 conflict with #110 | still open | git merge-tree pr109 pr110 now conflicts in README.md as well as src/cli.ts. |
| 5 config range check | NOT FIXED (deferred, pre-existing) | "failUnder": "80" falls back to Passing: score 69 ≥ 60 (ci default). The source label at least makes this visible. |
6 vibecodeqa/action |
out of scope | — |
Precedence matrix: correct in all 90 runs. The order is flag, then config, then --ci 60, then none.
--fail-under 0turns the gate off..vcqa.jsonandpackage.json#vcqabehave the same.--json --ciprints no gate line but keeps the exit code.
New findings (all nits)
-
nit, CONFIRMED:
src/cli.ts:709. A configfailUnderof 0 or below now turns off the--cigate, and the message misstates a negative value.- On main,
--ciwith configfailUnder: 0still gated at 60. Here it printsNo score gate: failUnder is 0 (config)and exits 0. That is consistent with the new precedence, but nothing documents that 0 means off. - Config
-5prints the same line,failUnder is 0, although the value is -5.
Fix: print
gate.thresholdas given, and document 0 as "no gate" (vibecodeqa/vibecodeqa#57). - On main,
-
nit, CONFIRMED, pre-existing:
src/cli.ts:52-63. A typo in--fail-undersilently falls back.--ci --fail-under abcwith config 50 givesPassing: score 69 ≥ 50 (config). #110 makes--test-timeout abcexit 2. Doing the same here would make the two flags consistent. The(config)label does at least show what happened. -
nit, CONFIRMED, pre-existing:
src/config.ts:20. A.vcqa.jsonwithoutfailUnderhidespackage.json#vcqa.failUnder. Matrix row "both":.vcqa.json={"ignore":[]}andpackage.json#vcqa.failUnder= 80 givePassing: score 69 ≥ 60 (ci default). The loader is all-or-nothing. That is not new, but the docs now say ".vcqa.json/package.json#vcqa" as if the two were merged.
Verdict: READY TO MERGE. Resolve the #110 conflict (src/cli.ts help block and README.md flags table) and keep both sides' rows.
# Conflicts: # src/cli.ts
|
Merged main (cec32ca) in; conflicts resolved in Verified at 2c63c30: |
VibeCode QAPR #109 ( 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" |
Closes #108.
What changed
--ciusedflag ?? (ci ? 60 : config ?? 0), so.vcqa.json/package.json#vcqafailUnderwas ignored in CI.resolveFailUnder(src/fail-under.ts):--fail-under→ configfailUnder→--cidefault 60 → 0. A configfailUnder: 0counts as an explicit choice (no gate).--ci(non-JSON/markdown) the output now names the threshold and its source:Passing: score 82 ≥ 80 (config)/Failing: score 70 < 80 (config)/(flag)/(ci default). I found no consumer inaction,github-app,appormcpthat parses theFailing:line.--helptext for--ci/--fail-underupdated, and so are the README flag rows.vcqa init: the scaffolded workflow runs--ciwithout--fail-under, and.vcqa.jsoncarries the gate (failUnder: 70), so editing the config actually changes CI.Verification
pnpm build: clean.pnpm lint: 0 errors (62 warnings, same as the baseline on main).pnpm test: 65 files, 748/748 passing (main baseline 739, plus 9 new).src/fail-under.test.tscovers the precedence table. Insrc/cli.test.ts, new end-to-end tests run the built CLI:failUnder: 100under--ciexits 1, and the test asserts the score was above 60, so the old code would have passed it.failUnder: 1under--ciexits 0.--fail-under 1overrides config 100.The acceptance criteria are written as config 80 / score 70 and config 40 / score 50. The fixture's score is not pinned to those numbers, so the tests check the same thresholds-around-60 property with 100 and 1.
Follow-up
A docs PR on vibecodeqa/vibecodeqa updates
docs/docs/ci.md:15,configuration.md:27andscoring.md:42, which describe the old behaviour.🤖 Generated with Claude Code
https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88