ci(tessl-review): distinguish tooling failures from low scores - #308
Conversation
The whole premise here was wrong. I was diffing against a month-old local That version has been discarded and this PR rebased. |
When the Tessl CLI fails to run at all — auth failure, network error, crash — the job counted it as a review failure and reported it as "N skill(s) scored below 80%". That is misleading: the skill was never scored. On #189 this surfaced as a red "below 80%" check on a skill that actually scores 85%, which reads to a contributor as a quality rejection. Track those separately as errors and report them as "review could not be run", with a⚠️ row and an explanatory note in the step summary. They still fail the job — they are just no longer confused with low scores. Also: - Skip cleanly with an explanatory summary when TESSL_TOKEN is not configured, instead of running an unauthenticated CLI and failing. - Pin actions/checkout and tesslio/setup-tessl to commit SHAs, with `# vN` comments so Renovate keeps tracking them. - Add a concurrency group so superseded runs are cancelled. - Use a single redirect block in the skip summary, silencing the one actionlint/shellcheck warning (SC2129) in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Lars Trieloff <lars@trieloff.net>
ae0ae6e to
3a04a70
Compare
trieloff
changed the title
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.2.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.2.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Problem
When the Tessl CLI fails to run at all — auth failure, network error, crash — the review job counts it as a review failure and reports:
That is misleading. The skill was never scored. On #189 this surfaced as a red "below 80%" check on a skill that actually scores 85%, which reads to a contributor as a quality rejection rather than a broken CI run. It cost a fair amount of time to work out that the real message buried in the log was:
Changes
Tooling failures are no longer reported as low scores. They get their own counter and are reported as "review could not be run", with a⚠️ row in the summary table and an explanatory note beneath it. They still fail the job — they're just no longer conflated with a below-threshold score.
Skip cleanly when Tessl isn't configured. If
TESSL_TOKENis absent, the job now skips with an explanatory step summary instead of installing an unauthenticated CLI and failing on the first review call.Pin actions to commit SHAs.
actions/checkoutandtesslio/setup-tesslare pinned with# vNcomments, so Renovate keeps tracking them.Add a concurrency group so superseded runs are cancelled.
Silence the one lint warning in this file (SC2129) by using a single redirect block in the skip summary.
actionlintis now clean on it.Deliberately unchanged: the
pull_request_targettrigger, the fork-onlyevalenvironment gating, the merge-ref checkout, and thetessl review run --workspace --json --thresholdinvocation. Those are all already correct onmain.Verification
actionlintis clean. The exit-code behaviour the error/score split relies on was checked against the real CLI, on a skill in this repo:089)189)1I also confirmed
main's existing score regex does correctly match the CLI's"reviewScore": 89field (the-iflag plus the optional underscore makereview[_]?scorematchreviewScore), so that is left alone.Note on the first version of this PR
I originally believed the workflow still needed the fork-secrets fix and the migration off the deprecated
tessl skill review. Both had already landed onmainin e56e29f, 756e8d7 and 16184c2 on 2026-07-06/07 — I was diffing against a month-old localmainand did not notice. The failing check on #189 is simply a stale run from2026-07-07T08:49Z, about 72 minutes before the fork fix merged.That version has been discarded. What remains is the subset that is genuinely still missing, and it no longer touches the fork-gating logic — which is good, because my earlier version would have regressed 756e8d7 by requiring
evalapproval for same-repo PRs too.🤖 Generated with Claude Code