Post clang-tidy's Fix as Review Suggestions Instead of Pushing It

A failed clang-tidy check now runs once more with --fix and uploads the patch as the clang-tidy-fix artifact; the check still fails. /bot fix-tidy downloads that patch for the PR's latest commit, checks it adds only includes to C++ files the PR changes, and posts one review: a one-click suggestion for each include on a line the PR's diff shows, and the whole patch with a git apply command. Nothing is pushed and no PR code runs in the bot, so the PR's author can run it too and no token is needed.
Temporary clang-tidy Probe: Use std::string_view Without Its Include
2026-10-12 10:21:17 +00:00 · 2026-10-12 17:43:45 +08:00 · 2026-10-12 17:30:48 +08:00 · 2026-10-12 17:29:38 +08:00
3 changed files with 373 additions and 1 deletions
@@ -3,7 +3,8 @@ name: clang-tidy
 # Runs clang-tidy, with the checks in .clang-tidy, over the C++ lines a pull
 # request changes (scripts/clang_tidy_diff.py). The compile database is
 # configured without the precompiled header, so code that only builds because
-# the PCH supplied an include fails here.
+# the PCH supplied an include fails here. A failed check also uploads the
+# missing-include fixes, which /bot fix-tidy (pr-tidy-bot.yml) posts on the PR.
 #
 # No paths filter: the job is a required check, and a workflow skipped by a
 # paths filter leaves a required check pending forever. A PR with no C++
@@ -83,7 +84,34 @@ jobs:
        run: cmake --build build-tidy --target git_commit_hash_header

      - name: Run clang-tidy on the changed lines
+        id: tidy
        if: steps.changes.outputs.cpp == 'true'
        run: >
          python3 scripts/clang_tidy_diff.py -p build-tidy --base HEAD^1
          --clang-tidy "$RUNNER_TEMP/clang-tidy/bin/clang-tidy" -j "$(nproc)"
+
+      # The fixes for a failed check, for /bot fix-tidy (pr-tidy-bot.yml) to post on
+      # the PR. They go into the patch only, so the check still fails. The status
+      # says whether findings remain that --fix cannot fix.
+      - name: Make the fix patch
+        id: fix
+        if: failure() && steps.tidy.outcome == 'failure'
+        run: |
+          mkdir -p "$RUNNER_TEMP/clang-tidy-fix"
+          status=0
+          python3 scripts/clang_tidy_diff.py -p build-tidy --base HEAD^1 \
+            --clang-tidy "$RUNNER_TEMP/clang-tidy/bin/clang-tidy" -j "$(nproc)" -- --fix > /dev/null || status=$?
+          echo "$status" > "$RUNNER_TEMP/clang-tidy-fix/status"
+          git -c core.quotePath=false diff --no-color --no-ext-diff --src-prefix=a/ --dst-prefix=b/ \
+            > "$RUNNER_TEMP/clang-tidy-fix/fix.patch"
+          if [ -s "$RUNNER_TEMP/clang-tidy-fix/fix.patch" ]; then
+            echo "patch=true" >> "$GITHUB_OUTPUT"
+          fi
+
+      - name: Upload the fix patch
+        if: failure() && steps.fix.outputs.patch == 'true'
+        uses: actions/upload-artifact@v7
+        with:
+          name: clang-tidy-fix
+          path: ${{ runner.temp }}/clang-tidy-fix/
+          retention-days: 30
@@ -0,0 +1,340 @@
+name: PR clang-tidy Bot
+
+# Posts the includes the clang-tidy check reports missing, on request, as a
+# review the PR's author can apply.
+#
+# Command, posted as a comment on the PR by its author or an account with write
+# access:
+#   /bot fix-tidy
+#
+# The fix comes from the clang-tidy check's own run on the PR's latest commit:
+# when the check fails, clang_tidy.yml runs it again with --fix and uploads the
+# result as the clang-tidy-fix artifact. Findings --fix cannot fix, such as a
+# using in a header, are left for the check to report.
+#
+# The reply is one review. Each include whose place falls on a line the PR's
+# diff shows is a suggestion GitHub can commit in one click, and the review's
+# body holds the whole patch with a git apply command for the rest. Nothing is
+# pushed, and no code from the PR runs here: the patch is only accepted when
+# every line it adds is an #include in a C++ file the PR already changes.
+
+on:
+  issue_comment:
+    types:
+      - created
+
+permissions: {}
+
+jobs:
+  suggest:
+    # Skips the job unless a PR comment mentions the command.
+    if: >-
+      github.repository == 'OrcaSlicer/OrcaSlicer'
+      && github.event.issue.pull_request != null
+      && contains(github.event.comment.body, '/bot fix-tidy')
+    concurrency:
+      group: ${{ github.workflow }}-${{ github.event.issue.number }}
+      cancel-in-progress: false
+    permissions:
+      actions: read          # find the check's run and download its patch
+      contents: read         # check out the PR head
+      pull-requests: write   # the review
+      issues: write          # feedback comment + reactions
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - name: Find the check's fix
+        id: find
+        uses: actions/github-script@v9
+        with:
+          script: |
+            const MARKER = '<!-- pr-tidy-bot -->';
+            const CHECK_WORKFLOW = 'clang_tidy.yml';
+            const ARTIFACT = 'clang-tidy-fix';
+            const CPP = /^(src|tests)\/.+\.(cpp|cc|cxx|hpp|h|hxx)$/;
+
+            const { owner, repo } = context.repo;
+            const issue = context.payload.issue;
+            const comment = context.payload.comment;
+
+            // Ignores a comment whose sender is not its author.
+            if (context.payload.sender.login !== comment.user.login || comment.user.type !== 'User') {
+              core.info('Ignoring a comment not written by its sender, or by a bot.');
+              return;
+            }
+
+            const commandLine = (comment.body || '')
+              .split('\n')
+              .map((line) => line.trim())
+              .find((line) => /^\/bot\s+fix-tidy\b/i.test(line));
+            if (!commandLine) {
+              core.info('No /bot fix-tidy command found.');
+              return;
+            }
+
+            const commenter = comment.user.login;
+            // Answers only the PR's author and maintainers, so the command cannot be used to spam.
+            if (commenter !== issue.user.login) {
+              const { data: access } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo, username: commenter });
+              if (!['admin', 'write'].includes(access.permission)) {
+                core.info(`Ignoring /bot fix-tidy from @${commenter}: not the PR's author and no write access.`);
+                return;
+              }
+            }
+
+            const react = (content) =>
+              github.rest.reactions.createForIssueComment({ owner, repo, comment_id: comment.id, content });
+            async function reply(body, content) {
+              await react(content);
+              await github.rest.issues.createComment({ owner, repo, issue_number: issue.number, body: `${MARKER}\n@${commenter} ${body}` });
+            }
+            const refuse = (reason) => reply(reason, '-1');
+
+            await react('eyes');
+
+            if (!/^\/bot\s+fix-tidy$/i.test(commandLine)) {
+              return refuse('`/bot fix-tidy` takes no arguments.');
+            }
+
+            const { data: pr } = await github.rest.pulls.get({ owner, repo, pull_number: issue.number });
+            if (pr.state !== 'open') {
+              return refuse('the PR is not open.');
+            }
+            if (!pr.head.repo) {
+              return refuse('the PR\'s branch is in a repository that no longer exists.');
+            }
+
+            const sha = pr.head.sha.slice(0, 10);
+            const { data: { workflow_runs: runs } } = await github.rest.actions.listWorkflowRuns({
+              owner, repo, workflow_id: CHECK_WORKFLOW, event: 'pull_request', head_sha: pr.head.sha, per_page: 1
+            });
+            const run = runs[0];
+            if (!run) {
+              return refuse(`the clang-tidy check has not run on ${sha}, the PR's latest commit.`);
+            }
+            if (run.status !== 'completed') {
+              return refuse(`the clang-tidy check is still running on ${sha}. Comment \`/bot fix-tidy\` again once it finishes.`);
+            }
+            if (run.conclusion === 'success') {
+              return reply(`the clang-tidy check passes on ${sha}. There is nothing to fix.`, '+1');
+            }
+            const { data: { artifacts } } = await github.rest.actions.listWorkflowRunArtifacts({
+              owner, repo, run_id: run.id, name: ARTIFACT
+            });
+            if (!artifacts.some((a) => !a.expired)) {
+              return refuse(
+                run.conclusion === 'failure'
+                  ? `clang-tidy has no automatic fix for what the check reports on ${sha}. The findings, in the [check's log](${run.html_url}), need a manual edit.`
+                  : `the clang-tidy check on ${sha} did not finish (${run.conclusion}). Re-run it, then comment \`/bot fix-tidy\` again.`
+              );
+            }
+
+            // The right-side lines of each changed C++ file's diff: the lines a
+            // review comment can be placed on. A file too large for GitHub to
+            // show its diff has none, and gets no suggestions.
+            const files = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr.number, per_page: 100 });
+            const ranges = {};
+            for (const file of files) {
+              if (file.status === 'removed' || !CPP.test(file.filename)) {
+                continue;
+              }
+              ranges[file.filename] = [...(file.patch || '').matchAll(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/gm)]
+                .map((m) => [Number(m[1]), Number(m[1]) + Number(m[2] ?? 1) - 1])
+                .filter(([first, last]) => last >= first);
+            }
+
+            core.setOutput('run-id', run.id);
+            core.setOutput('run-url', run.html_url);
+            core.setOutput('head-sha', pr.head.sha);
+            core.setOutput('head-repo', pr.head.repo.full_name);
+            core.setOutput('ranges', JSON.stringify(ranges));
+
+      - name: Download the fix
+        if: steps.find.outputs.run-id != ''
+        uses: actions/download-artifact@v8
+        with:
+          name: clang-tidy-fix
+          run-id: ${{ steps.find.outputs.run-id }}
+          github-token: ${{ github.token }}
+          path: ${{ runner.temp }}/clang-tidy-fix
+
+      # The PR's own files, only to apply the patch to. Nothing in them is run.
+      - name: Checkout the PR head
+        if: steps.find.outputs.run-id != ''
+        uses: actions/checkout@v7
+        with:
+          repository: ${{ steps.find.outputs.head-repo }}
+          ref: ${{ steps.find.outputs.head-sha }}
+          path: head
+          fetch-depth: 1
+          lfs: 'false'
+          persist-credentials: false
+          sparse-checkout: |
+            src
+            tests
+
+      # The check made the patch on the PR's merge commit, so it is applied to the
+      # head and diffed again, giving line numbers in the PR's own files.
+      - name: Check the fix and place its suggestions
+        id: place
+        if: steps.find.outputs.run-id != ''
+        env:
+          RANGES: ${{ steps.find.outputs.ranges }}
+        run: |
+          python3 -I - "$RUNNER_TEMP/clang-tidy-fix/fix.patch" head "$RUNNER_TEMP" <<'EOF'
+          import json, os, re, subprocess, sys
+
+          patch_path, head, out = sys.argv[1:]
+          ranges = json.loads(os.environ["RANGES"])
+
+          def output(**values):
+              with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as f:
+                  for key, value in values.items():
+                      f.write(f"{key}={value}\n")
+
+          # Every line the patch adds must be an #include or blank, in a C++ file the
+          # PR changes, and it may not remove, create, rename or change the mode of
+          # anything.
+          include = re.compile(r'\s*#\s*include\s*(<[\w./+-]+>|"[\w./+-]+")\s*')
+          header = re.compile(r"(index [0-9a-f]+\.\.[0-9a-f]+( \d+)?|@@ -\d+(,\d+)? \+\d+(,\d+)? @@.*)")
+          problems = []
+          path = None
+          for line in open(patch_path, encoding="utf-8").read().splitlines():
+              if line.startswith("diff --git "):
+                  match = re.fullmatch(r"diff --git a/(.+) b/\1", line)
+                  path = match.group(1) if match else None
+                  if path not in ranges:
+                      problems.append(f"{line}: not a C++ file the PR changes")
+              # git appends a tab to the --- and +++ lines of a path that contains a space.
+              elif line.removesuffix("\t") in (f"--- a/{path}", f"+++ b/{path}") or header.fullmatch(line):
+                  pass
+              elif line.startswith(" ") or line == "\\ No newline at end of file":
+                  pass
+              elif line.startswith("+") and (include.fullmatch(line[1:]) or not line[1:].strip()):
+                  pass
+              else:
+                  problems.append(f"{path}: {line}")
+          if problems:
+              for problem in problems:
+                  print(f"::error::Refused patch line {problem}")
+              output(result="rejected")
+              sys.exit()
+
+          if subprocess.run(["git", "-C", head, "apply", "--index", os.path.abspath(patch_path)]).returncode != 0:
+              output(result="apply-failed")
+              sys.exit()
+
+          git = ["git", "-C", head, "-c", "core.quotePath=false", "diff", "--cached", "--no-color", "--no-ext-diff",
+                 "--src-prefix=a/", "--dst-prefix=b/"]
+          with open(os.path.join(out, "head.patch"), "w", encoding="utf-8") as f:
+              f.write(subprocess.run(git, check=True, capture_output=True, encoding="utf-8").stdout)
+
+          # Each run of added lines becomes one suggestion on the line it goes after
+          # (before, at the top of a file): that line, replaced by itself and the
+          # includes. It can only sit on a line the PR's diff shows.
+          hunks = []
+          path = None
+          for line in subprocess.run(git + ["-U0"], check=True, capture_output=True, encoding="utf-8").stdout.splitlines():
+              if line.startswith("diff --git "):
+                  path = re.fullmatch(r"diff --git a/(.+) b/\1", line).group(1)
+              elif line.startswith("@@ "):
+                  hunks.append((path, int(re.match(r"@@ -(\d+)", line).group(1)), []))
+              elif hunks and line.startswith("+") and not line.startswith("+++ "):
+                  hunks[-1][2].append(line[1:])
+
+          suggestions = []
+          total = suggested = 0
+          for path, after, added in hunks:
+              count = sum(bool(include.fullmatch(line)) for line in added)
+              total += count
+              original = subprocess.run(["git", "-C", head, "show", f"HEAD:{path}"], check=True, capture_output=True,
+                                        encoding="utf-8").stdout.splitlines()
+              line = max(after, 1)
+              if line > len(original) or not any(first <= line <= last for first, last in ranges[path]):
+                  continue
+              body = "\n".join([original[line - 1], *added] if after else [*added, original[0]])
+              # A fence longer than any run of backticks in the line it repeats.
+              fence = "`" * max([3] + [len(run) + 1 for run in re.findall(r"`+", body)])
+              suggestions.append({"path": path, "line": line, "side": "RIGHT",
+                                  "body": f"{fence}suggestion\n{body}\n{fence}"})
+              suggested += count
+          with open(os.path.join(out, "suggestions.json"), "w", encoding="utf-8") as f:
+              json.dump(suggestions, f)
+          output(result="ok", includes=total, suggested=suggested)
+          EOF
+
+      - name: Post the fix
+        if: always() && steps.find.outputs.run-id != ''
+        uses: actions/github-script@v9
+        env:
+          RESULT: ${{ steps.place.outputs.result }}
+          INCLUDES: ${{ steps.place.outputs.includes }}
+          SUGGESTED: ${{ steps.place.outputs.suggested }}
+          HEAD_SHA: ${{ steps.find.outputs.head-sha }}
+          CHECK_URL: ${{ steps.find.outputs.run-url }}
+        with:
+          script: |
+            const fs = require('fs');
+            const path = require('path');
+            const MARKER = '<!-- pr-tidy-bot -->';
+            // GitHub's limit on a review body is 65536 characters.
+            const MAX_PATCH = 60000;
+
+            const { owner, repo } = context.repo;
+            const issue = context.payload.issue;
+            const comment = context.payload.comment;
+            const commenter = comment.user.login;
+            const { RESULT, INCLUDES, SUGGESTED, HEAD_SHA, CHECK_URL, RUNNER_TEMP } = process.env;
+            const sha = HEAD_SHA.slice(0, 10);
+            const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`;
+
+            const react = (content) =>
+              github.rest.reactions.createForIssueComment({ owner, repo, comment_id: comment.id, content });
+            async function refuse(reason) {
+              await react('-1');
+              await github.rest.issues.createComment({ owner, repo, issue_number: issue.number, body: `${MARKER}\n@${commenter} ${reason}` });
+            }
+
+            if (RESULT === 'rejected') {
+              return refuse(`the check's fix changes more than includes, so I didn't post it. See the [run log](${runUrl}).`);
+            }
+            if (RESULT === 'apply-failed') {
+              return refuse(`the check's fix doesn't apply to ${sha}. Merge the base branch into the PR, then comment \`/bot fix-tidy\` again once the check has run.`);
+            }
+            if (RESULT !== 'ok') {
+              return refuse(`something went wrong. See the [run log](${runUrl}).`);
+            }
+
+            const read = (...parts) => fs.readFileSync(path.join(RUNNER_TEMP, ...parts), 'utf8');
+            const patch = read('head.patch');
+            const suggestions = JSON.parse(read('suggestions.json'));
+            const passes = read('clang-tidy-fix', 'status').trim() === '0';
+
+            const one = INCLUDES === '1';
+            const includes = one ? '1 missing include' : `${INCLUDES} missing includes`;
+            const inline = SUGGESTED === '0'
+              ? (one ? 'It goes on a line this PR does not change, so I can\'t suggest it inline.'
+                     : 'They go on lines this PR does not change, so I can\'t suggest them inline.')
+              : SUGGESTED === INCLUDES
+                ? (one ? 'It is suggested inline below.' : 'Each is suggested inline below.')
+                : `${SUGGESTED} of them are suggested inline below; the rest go on lines this PR does not change.`;
+            const remaining = passes
+              ? `With ${one ? 'it' : 'them'}, the check passes.`
+              : `Some findings remain that need a manual edit; see the [check's log](${CHECK_URL}).`;
+            // A fence longer than any run of backticks in the patch's context lines.
+            const fence = '`'.repeat(Math.max(3, ...[...patch.matchAll(/`+/g)].map((m) => m[0].length + 1)));
+            const apply = patch.length > MAX_PATCH
+              ? `The full patch is too large to show here. Download \`fix.patch\` from the [check's run](${CHECK_URL}) and run \`git apply fix.patch\`.`
+              : `To apply ${one ? 'it' : 'all of them'}, run this in your checkout of the PR branch:\n\n${fence}sh\ngit apply <<'EOF'\n${patch}EOF\n${fence}`;
+            const body = (withSuggestions) => `${MARKER}\n@${commenter} clang-tidy's fix for ${sha} adds ${includes}. ` +
+              `${withSuggestions ? `${inline} ` : ''}${remaining}\n\n${apply}`;
+
+            const review = { owner, repo, pull_number: issue.number, commit_id: HEAD_SHA, event: 'COMMENT' };
+            try {
+              await github.rest.pulls.createReview({ ...review, body: body(true), comments: suggestions });
+            } catch (error) {
+              // A suggestion GitHub will not place still leaves the patch.
+              core.warning(`Posting the suggestions failed (${error.message}); posting the patch alone.`);
+              await github.rest.pulls.createReview({ ...review, body: body(false) });
+            }
+            await react('+1');
@@ -434,3 +434,7 @@ unsigned char picking_checksum_alpha_channel(unsigned char red, unsigned char gr

 } // namespace Slic3r

+
+namespace Slic3r {
+std::string_view tidy_bot_probe() { return std::string_view("x"); }
+}