[Madrigal]

Issue #, if available:

Description of changes:

Draft: opened to verify the new check on a real GitHub runner (configure time, the runner's clang-tidy version) before review.

clang-tidy-check currently fails on almost every PR (about 28 of the last 34 runs), mostly for reasons that have nothing to do with the PR. Because the check is always red, real issues get merged unnoticed. This PR makes it report only problems on lines the PR adds or modifies, and only on files it can actually compile.

Why it fails today

• It runs clang-tidy on every changed file in full, so older performance-* warnings anywhere in a touched file fail the job.
• It configures with -DBUILD_ONLY="s3" -DENABLE_TESTING=OFF. Test files and services other than s3 then have no compile command, so clang-tidy guesses flags and fails on missing includes (aws/testing/AwsCppSdkGTestSuite.h, gtest/gtest.h not found). No line filter can suppress those compiler errors.
• Unity builds put most core sources behind ub_core.cpp, so those files have no compile command of their own.
• It diffs against origin/<head branch>, which doesn't exist for PRs from forks.

Real issues that were merged
Since the check was added in #3753, 11 merged PRs introduced 29 performance-* errors on lines they added. The check was red on those PRs too, but that was indistinguishable from the usual noise. Some examples:

PR	Errors	Check	Location
#3866	12	performance-move-const-arg, performance-unnecessary-value-param	s3-transfer: DownloadRequest.cpp, UploadRequest.cpp, S3TransferManager.cpp
#3910	6	performance-unnecessary-copy-initialization	ClientProtocolTest.cpp:40, CodecTest.cpp (5)
#3947	2	performance-move-const-arg (std::move on a trivially-copyable DateTime)	smithy/client/schema/Document.cpp:154, :253
#3945	1	performance-unnecessary-value-param	BucketAndObjectOperationTest.cpp:1575 (s3-crt integration tests)
#3836	1	performance-unnecessary-value-param	core/source/client/RetryStrategy.cpp:113

The others are #3844, #3919, #3916, #3895, #3884 and #3849. The new check flags all 29 errors. Fixing the existing ones is out of scope here; the point is that new ones will now be visible.

What changes
The logic moves out of the workflow into tools/scripts/run_clang_tidy.py, which also runs locally. The workflow now just checks out, installs dependencies, and calls the script. The script:

1. Picks BUILD_ONLY: s3, plus any service whose tests or high-level library the PR touches, plus that service's test dependencies. It reads these from the existing lists in cmake/sdksCommon.cmake (SDK_TEST_PROJECT_LIST, TEST_DEPENDENCY_LIST). For example, Fix S3 CRT async request lifetime #3945 configures s3;s3-crt, so its s3-crt test is analyzed instead of skipped.
2. Configures CMake with tests on and unity builds off, so every source file gets its own compile command.
3. Checks only changed files with a compile command. Headers, and tests of services that weren't configured, are listed as skipped instead of failing. A header problem is still reported when clang-tidy's report points at a line the PR changed in a .cpp file (as in TM Header Update #3844).
4. Runs clang-tidy with -line-filter set to the PR's added or modified lines, so only diagnostics on those lines are reported. The script builds the line filter from the diff itself instead of using LLVM's clang-tidy-diff.py, so the clang-tidy binary is the only dependency. Whether and where clang-tidy-diff.py is installed differs between apt, Homebrew and pip packages. .clang-tidy is unchanged and still decides what's an error (WarningsAsErrors: 'performance-*'). Making more checks blocking later is a one-line .clang-tidy change.
5. Reports errors clearly: a summary at the end of the log, plus inline annotations on the PR's "Files changed" tab.

CONTRIBUTING.md gets a "Checking Your Changes Locally" section with the steps below, plus a PR checklist item.

Security

• The trigger stays pull_request, with contents: read and no secrets, same as before. A separate security review found no new risk above Low.
• Hardening: output from PR-controlled tools (CMake, clang-tidy) is printed with workflow commands paused (::stop-commands::), so it can't inject workflow commands. File lists are prefixed so a crafted file name can't start a command line. Only real diagnostics inside the repo become annotations.

How this was tested
All runs were local (macOS, clang-tidy 21.1.6), on full source trees extracted from git, using the same CMake configure the workflow runs.

• History sweep: every merged PR since Add Clang tidy to CI  #3753 that touches non-generated C++ sources, 55 PRs, each run at its squash commit (parent vs. merge commit). It found the 11 PRs and 29 errors above. The other 44 pass, including PRs where the old check only failed on pre-existing issues or missing includes.
• Independent cross-check: for every run, I also ran plain clang-tidy on each whole checked file with the same compile flags, and kept only errors on lines the PR added. The script's results matched exactly in 54 of 55 runs, with nothing missed and nothing extra. In TM Header Update #3844 the script reported two more errors than the cross-check. They're in new headers, and clang-tidy attaches them to destructor definitions on .cpp lines the PR added. That's correct behavior; the cross-check only looked at errors located in the .cpp itself.
• Before and after mid-PR fixes:
  • Make EventBridge unit tests protocol-agnostic (JSON and RPC v2 CBOR) #3959: commits before and after its bugprone-narrowing-conversions fix. The warnings show up on the changed lines and don't block, as .clang-tidy specifies.
  • Add schema-serde streaming JSON/XML deserializers, ec2Query, Codec, and ClientProtocol #3910: before and after its "fixing clang-tidy findings" commit. Six performance-* errors remain in both, matching the merged state.
• Injected issue: adding a by-value Aws::String parameter to DateTimeCommon.cpp produced performance-unnecessary-value-param and a non-zero exit.
• Hardening: fake clang-tidy output with forged ::add-mask::/::warning lines, spoofed error: lines in code snippets, and an out-of-repo path. Only the real diagnostic was annotated, and everything else stayed inside the paused region.
• Built-in line filter: the script first used clang-tidy-diff.py. After switching to building -line-filter itself, Fix S3 CRT async request lifetime #3945, TM Header Update #3844, Single File Upload/Download #3866, Make CognitoCachingCredentialsProviderTest work with both JSON and RPC v2 CBOR #3958, JSON number parsing bug fix #3921 and Validate event stream header name and value lengths when encoding #3962 report exactly the same errors as before.
• Fresh local setup: a new venv with pip install clang-tidy (23.1.4) and the script run with no flags on Fix S3 CRT async request lifetime #3945. It found the same error, and no system-header errors on macOS.
• Real checkout, as documented in CONTRIBUTING.md: in a git checkout of this branch on macOS, where the only thing installed beyond the existing git/cmake/python3 was pip install clang-tidy, python3 tools/scripts/run_clang_tidy.py ran end to end in about a minute. With a temporary by-value Aws::String parameter added to DateTimeCommon.cpp, it reported performance-unnecessary-value-param on that line and exited 1.
• Not yet run on a GitHub runner. This draft is for that.

Running locally
From the repo root, with submodules checked out:

pip install clang-tidy          # or a system clang-tidy: apt install clang-tidy, brew install llvm
git fetch origin
python3 tools/scripts/run_clang_tidy.py       # compares against origin/main by default

Needs git, cmake and clang-tidy. On macOS the script passes the SDK path (xcrun --show-sdk-path) to clang-tidy itself, so no extra flags are needed. CI installs whatever clang-tidy the ubuntu-latest image provides, so a local version can differ slightly in which diagnostics it reports.

Known limitations

• Changed headers are not analyzed directly.
• Tests for Windows-only sources have no compile command on the Linux runner and are skipped.
• workflow_dispatch runs are now a no-op.

Check all that applies:

• Did a review by yourself.
• Added proper tests to cover this PR. (If tests are not applicable, explain.)
• Checked if this PR is a breaking (APIs have been changed) change.
• Checked if this PR will not introduce cross-platform inconsistent behavior.
• Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

• Linux
• Windows
• Android
• MacOS
• IOS
• Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.