diff --git a/.github/workflows/dependabot-merge.yml b/.github/workflows/dependabot-merge.yml index 6e383a5..f6c36d2 100644 --- a/.github/workflows/dependabot-merge.yml +++ b/.github/workflows/dependabot-merge.yml @@ -1,31 +1,108 @@ name: Dependabot auto-merge -on: pull_request + +# Merges a Dependabot PR only once CI has actually reported green. +# +# This used to run `on: pull_request` and call `gh pr merge --auto`. That flag reads like +# "merge when green", but it means "merge when branch protection is satisfied" — it delegates +# the waiting entirely to required status checks. `main` is unprotected and declares none, so +# the condition was vacuously true the instant the PR opened and the merge landed immediately. +# PR #233 was opened at 04:18:08 and merged at 04:18:20; its test jobs did not start until +# 04:18:34 and went red at 04:21. That is how a failing dependency bump reached main. +# +# Rather than turn on branch protection, the list of checks that must be green lives here. +# Firing on workflow_run instead of pull_request is what makes that possible: the merge is +# decided after CI reports, so there is no check to wait on that is waiting on us, and no +# runner sits idle for the duration of the build. + +on: + workflow_run: + workflows: + - ".NET Workflow" + - "Verify Generated Files" + types: [completed] permissions: contents: write pull-requests: write - packages: read jobs: - dependabot: - name: Process Dependabot PR + merge: + name: Merge green Dependabot PR runs-on: ubuntu-latest - if: github.actor == 'dependabot[bot]' timeout-minutes: 5 + # Only what the event itself is, never who it claims to be. A successful CI run on a pull + # request is the cheap pre-filter; whose pull request it is gets decided in the step below, + # from the API. A failed run is not merely "not yet" — it must never reach the merge step. + if: > + github.event.workflow_run.event == 'pull_request' && + github.event.workflow_run.conclusion == 'success' + steps: - - name: Fetch Dependabot metadata - id: metadata - uses: dependabot/fetch-metadata@25dd0e34f4fe68f24cc83900b1fe3fe149efef98 # v3.1.0 - with: - github-token: "${{ secrets.GITHUB_TOKEN }}" - - - name: Enable auto-merge for Dependabot PRs - if: steps.metadata.outputs.update-type == 'version-update:semver-major' || steps.metadata.outputs.update-type == 'version-update:semver-minor' || steps.metadata.outputs.update-type == 'version-update:semver-patch' + - name: Merge if Dependabot opened it and every check on the commit is green env: - PR_URL: ${{github.event.pull_request.html_url}} - GH_TOKEN: ${{secrets.GITHUB_TOKEN}} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SHA: ${{ github.event.workflow_run.head_sha }} + SELF: Merge green Dependabot PR run: | - set -e - echo "Processing PR: $PR_URL" - gh pr merge --auto --merge "$PR_URL" + set -euo pipefail + + # The workflow_run payload says who triggered the run, and that is not an + # authenticated statement about who opened the pull request: a re-run re-attributes + # it to whoever pressed the button, and nothing in the payload is signed. Trusting + # `actor.login` or the `dependabot/` branch prefix to decide what may be merged is + # githubactions:S8232. So the payload is used only to locate the pull request, and + # GitHub is asked who actually opened it. + # + # Matching on the head SHA rather than the branch name also buys the property this + # workflow exists for: the only pull request that can match is one whose head is the + # exact commit CI just reported on. A commit pushed after CI finished moves the head, + # nothing matches, and nothing is merged — instead of merging code CI never saw. + case "$SHA" in + "" | *[!0-9a-f]*) echo "Head SHA is not a hex digest; refusing to continue." >&2; exit 1 ;; + esac + + FOUND=$(gh api --paginate "repos/$GITHUB_REPOSITORY/pulls?state=open&per_page=100" \ + --jq "[.[] | select(.head.sha == \"$SHA\")][0] + | select(. != null) + | [.number, .user.login] | @tsv") + + if [ -z "$FOUND" ]; then + echo "No open pull request whose head is $SHA; nothing to merge." + exit 0 + fi + + PR=$(printf '%s' "$FOUND" | cut -f1) + AUTHOR=$(printf '%s' "$FOUND" | cut -f2) + + if [ "$AUTHOR" != "dependabot[bot]" ]; then + echo "PR #$PR was opened by $AUTHOR, not Dependabot. Leaving it alone." + exit 0 + fi + + # The workflow that triggered us is green, but it is not the only one. Ask the + # commit what every check says, and treat anything still running as a reason to + # wait rather than a reason to merge. A check that is absent entirely is not + # counted: Verify Generated Files skips a markdown-only bump by design. + # + # skipped and neutral count as green. dotnet.yml skips Analyze & Release and + # Security Scanning on a pull request, and CodeQL reports neutral there. + gh api --paginate "repos/$GITHUB_REPOSITORY/commits/$SHA/check-runs" \ + --jq ".check_runs[] + | select(.name != \"$SELF\") + | select(.status != \"completed\" + or (.conclusion != \"success\" + and .conclusion != \"skipped\" + and .conclusion != \"neutral\")) + | \" \(.name): \(.status)/\(.conclusion // \"pending\")\"" > "$RUNNER_TEMP/not-green.txt" + + if [ -s "$RUNNER_TEMP/not-green.txt" ]; then + echo "Not merging PR #$PR. These checks are not green:" + cat "$RUNNER_TEMP/not-green.txt" + # Not a failure of this workflow: whichever run finishes last fires it again, + # and the merge happens on that pass. + exit 0 + fi + + echo "PR #$PR is Dependabot's and every check on $SHA is green. Merging." + gh pr merge --merge "$PR" diff --git a/CLAUDE.md b/CLAUDE.md index 7a12dd7..0d53df7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -200,7 +200,7 @@ forms as well, and matching it is a change to every form at once rather than par ### Can the quantities be generated for languages other than C# and C++? -`SevenTargetProjectionTests` is the probe that answers it, and the answer so far is **five of +`SevenTargetProjectionTests` is the probe that answers it, and the answer so far is **six of seven**. It builds one quantity — the magnitude form of `Length`, read from the real `dimensions.json` — as a language-agnostic `ktsu.Coder` AST and writes it in all seven of that library's targets. The neutral shape is the whole of what a generated magnitude is: a record struct @@ -210,16 +210,21 @@ C# comes out as exactly what `QuantitiesGenerator` writes today, which is the re the AST is expressive enough for the quantities, so what the other six do is a question about those languages rather than about the model. -**Two targets write source their own toolchain refuses**, both recorded upstream and neither fixable -here: +**One target writes source its own toolchain refuses**, recorded upstream and not fixable here: | Target | What comes out | Why | |---|---|---| | Go | `type Length struct` with no parameters, then `func LengthFromMeter(value T) Length[T]` | ktsu-dev/Coder#63 — a generic type is deliberately written down rather than emitted, and the constructor was not given the same treatment. `go vet` says `undefined: T`. | -| Python | `class Length(IVector0[Length[T], T])` | ktsu-dev/Coder#64 — Python evaluates a base list eagerly, so the self-type idiom every quantity is declared with raises `NameError` on import. | -The tests **pin** both rather than skipping them, so the day either is fixed upstream the test fails -and is updated to assert the fix. +The test **pins** it rather than skipping it, so the day it is fixed upstream the test fails and is +updated to assert the fix. + +**Python was the second, and the pin is what caught the fix.** It wrote +`class Length(IVector0[Length[T], T])`, which raises `NameError` on import because Python evaluates +a base list eagerly. ktsu-dev/Coder#64 fixed that to the string forward reference +`IVector0["Length[T]", T]`, and the first build after the bump from ktsu.Coder 3.14.0 to 3.14.3 +failed here — which is the whole point of pinning rather than skipping, and is how the fix was +noticed at all. The test asserts the fix now, so a regression upstream fails the same way round. The probe lives in `Semantics.Cpp.Test` because that is where the reader of `dimensions.json` is, and that is itself the finding about this repository: `QuantityMetadata` and `MetadataProjection` diff --git a/Semantics.Cpp.Test/SevenTargetProjectionTests.cs b/Semantics.Cpp.Test/SevenTargetProjectionTests.cs index 09048a1..d4c8a71 100644 --- a/Semantics.Cpp.Test/SevenTargetProjectionTests.cs +++ b/Semantics.Cpp.Test/SevenTargetProjectionTests.cs @@ -32,10 +32,11 @@ namespace ktsu.Semantics.Cpp.Test; /// before a neutral projection could be a project of its own. /// /// -/// Two targets are known to produce source their own toolchain refuses, and the tests below -/// pin that rather than skipping it, so the day either is fixed upstream the test fails and says -/// so. Both are recorded against ktsu.Coder — ktsu-dev/Coder#63 and ktsu-dev/Coder#64 — and -/// neither is anything this repository can fix. +/// One target is still known to produce source its own toolchain refuses, and the test +/// below pins that rather than skipping it, so the day it is fixed upstream the test fails and +/// says so. It is recorded against ktsu.Coder as ktsu-dev/Coder#63 and is not anything this +/// repository can fix. Python was the second such target until ktsu-dev/Coder#64 was fixed; its +/// pin failed exactly as intended, and it now asserts the fix. /// /// [TestClass] @@ -142,20 +143,25 @@ public void GoSpellsAGenericTypeItDidNotDeclare() } /// - /// Python writes a base that names the class being declared, which Python evaluates before the - /// class exists. + /// Python quotes the self-type in its own base list, which is what makes the class importable. /// /// /// `IVector0<Length<T>, T>` is the self-type idiom every quantity here is declared /// with, and it is what C# needs to give an interface a method returning the implementing type. - /// Python evaluates a base list eagerly, so `class Length(IVector0[Length[T], T])` raises - /// `NameError: name 'Length' is not defined` on import. A string in the base list is the fix - /// Python has for this, and it is not something the AST currently says. ktsu-dev/Coder#64. + /// Python evaluates a base list eagerly, so naming the class inside its own bases — + /// `IVector0[Length[T], T]` — raises `NameError: name 'Length' is not defined` on import. A + /// string is the forward reference Python has for exactly this, and ktsu.Coder now writes one. + /// + /// This was pinned as a defect until ktsu-dev/Coder#64 was fixed, which the bump from + /// ktsu.Coder 3.14.0 to 3.14.3 brought in. The pin failed, as it was written to, and asserts + /// the fix instead. Keeping it pins the fix the same way round, so a regression upstream fails + /// here rather than shipping a module that cannot be imported. + /// /// [TestMethod] - public void PythonNamesTheClassInsideItsOwnBases() + public void PythonQuotesTheClassInsideItsOwnBases() { - Assert.Contains("class Length(IVector0[Length[T], T])", Written["python"]); + Assert.Contains("class Length(IVector0[\"Length[T]\", T])", Written["python"]); } ///