🐛 QD-14800 Patch workflow job's check-run instead of creating an orphan - #599
🐛 QD-14800 Patch workflow job's check-run instead of creating an orphan#599Anna Zhukova (bindreams) wants to merge 5 commits into
Conversation
…input (default to job.check_run_id expression)
…parser rejects job context outside default)
|
I was waiting for this one ! :) thanks a lot! |
|
Ready for review! |
| // PATCH can fail in fork-PR contexts with a restricted token, on a | ||
| // stale ID, or with mismatched app identity. Fall through to the | ||
| // legacy create/update-by-name path so the user still gets a check-run. | ||
| core.warning( |
There was a problem hiding this comment.
- do you have a run of the action with
@azhukova/QD-14800version? - can we move those comments into the function comments / collapse them a bit?
There was a problem hiding this comment.
Rewrote the comments and created a demonstration run over at JetBrains/qodana-kotlin-cli#18. You can see how it was before the fix in a screenshot attached to the YT ticket.
| required: false | ||
| default: "" | ||
| deprecationMessage: 'This option is for development purposes only. Do not use it in production.' | ||
| _job-check-run-id: |
There was a problem hiding this comment.
are we sure there's no way without internal parameter to avoid this? we could test / explore existing APIs more, but if there's no other way, I would accept this solution and everything else looks good to me
There was a problem hiding this comment.
There are other solutions but they are all subpar. You could use the API to iterate over the list of current runs and try and find the one that's yours according to some heuristics based on name and running status - I thought this is too fragile. The solution currently in place is borrowed from https://github.com/qoomon/actions--context/blob/main/action.yaml
There are two more solutions:
- Restructure qodana-action into a composite action and evaluate the expression completely inside;
- Wait for Add JOB_ env vars actions/runner#4053 which has no downsides.
2 participants
Summary
Fixes QD-14800.
On a PR in a repo with two or more workflows triggered by
pull_request,qodana-actionposted its summary check-run under the wrong workflow's name in the PR Checks rollup (e.g. "Semantic PR Title / Qodana for JVM" when Qodana actually ran in the CI workflow).Root cause.
checks.createcannot specify a check-suite; GitHub auto-groups every check-run posted by thegithub-actionsapp on a given SHA into the first existing check-suite for that app, which on multi-workflow repos is whichever workflow finished its first job earliest.Fix. PATCH the workflow job's already-existing check-run instead of creating a parallel one.
actions.listJobsForWorkflowRunreturns jobs whoseidfield IS the check-run ID; patching that keeps the result inside the workflow's own check-suite. Do not passstatus/conclusion(GitHub Actions owns those for job check-runs). Falls back to the legacychecks.createpath if running outside Actions or if lookup/PATCH fails.Changes
scan/src/utils.ts:resolveJobCheckRunId(client)— paginateslistJobsForWorkflowRun, matches the running job byGITHUB_JOBexact →${GITHUB_JOB} (prefix (matrix) →RUNNER_NAME, with RUNNER_NAME sub-filter +status === 'in_progress'tie-break to disambiguate concurrently-running matrix legs. Memoized at module scope so chunked annotation publishing (≥50 annotations) doesn't re-list jobs each chunk.updateJobCheck(client, id, output)— sends only{owner, repo, check_run_id, output}(nostatus/conclusion).publishGitHubCheckreworked: tries the new path; on PATCH failure, invalidates the cache and falls through to the existinglistForRef+create/updatelegacy path.scan/src/annotations.ts— fallback error message updated to mentionactions: read.README.md— threepermissions:examples now includeactions: read; branch-protection section gets a note explaining the new check name comes from the workflow job (setjobs.<id>.name:for stability).New required permission
Breaking change
Users whose branch-protection rule requires a check named exactly
Qodana for <Linter>must update the rule to require the workflow job's check name (the job's YAML key, or whatever they set viajobs.<id>.name:). README documents this.Test plan
npm run build— cleannpm run -w scan lint— 0 errorsnpm run -w scan test— 37 passed, 1 skipped (was 26 baseline; +11 new tests)scan/dist/index.jsre-bundled by the local pre-commit hookpull_requestworkflows — confirm Qodana summary appears only under the workflow that invoked the action (no orphan check); cross-checkcheck_suite_idviagh apiactions: readpermission) still produces a check-run with the legacy orphan-grouping bug, plus a clear warning telling the user to add the permissionNew tests
GITHUB_JOBmatch — PATCH payload carries nostatus/conclusionin_progresstie-breakin_progresslegs —RUNNER_NAMEdisambiguatesname:override →RUNNER_NAMEfallbackGITHUB_ACTIONSunset → legacy fallbackGITHUB_RUN_ID→ legacy fallback (no API call)paginaterejects → warning + legacy fallbackpublishGitHubCheckreuses cached job ID, no second API call