Tags: unslothai/llama.cpp
Tags
Reject pull_request_target in CI, so a sync cannot reintroduce it (#229) * Reject pull_request_target in CI, so a sync cannot reintroduce it Deleting bench.yml.disabled removed the file but not the hazard. This is a fork, and an upstream sync can bring back upstream's bench.yml under its original, ACTIVE name at any time. unsloth-upstream-sync-guard.yml checks that the recorded sync point is still an ancestor of master and says nothing about what the synced workflows do, so that reintroduction would arrive green. scripts/unsloth/check_workflow_triggers.py refuses pull_request_target outright and requires workflow_run to carry a written justification, matching the rule unslothai/unsloth enforces in scripts/lint_workflow_triggers.py, with the same escape-hatch comment spelling so the two repositories read the same way. Hosted in unsloth-pr-set-lint.yml because its push and pull_request triggers both already include .github/workflows/*.yml, so every workflow edit runs it, including the merge commit of a sync. It only reads .yml and .yaml, which is all GitHub loads: a neighbouring .disabled file is genuinely inert, and the moment someone renames it the check sees it. There is a test for exactly that pair. The two live workflow_run workflows now state why they are safe rather than leaving it to be re-derived. Neither was changed in behaviour: unsloth-prebuilt-retry.yml has no actions/checkout step at all, reads no artifact from the triggering run, holds no secrets, and uses actions: write only to POST a rerun in this same repository. The workflow it watches is schedule and dispatch only, so the trigger is never fork-controlled. unsloth-repin-bot.yml checks out with no ref:, which on a workflow_run event resolves to the default branch rather than any pull request head, with persist-credentials: false set deliberately so the automatic GITHUB_TOKEN extraheader cannot shadow the step-scoped REPIN_TOKEN, which is scrubbed from every log line it could reach. scripts/unsloth/test_check_workflow_triggers.py covers the live tree, both file extensions, a reintroduced bench.yml, the .disabled form being correctly ignored, the justified and unjustified workflow_run cases, and an empty or missing workflows directory failing rather than passing vacuously. 11 checks, all passing. Found during the organisation-wide audit against the 2026-09-21 cargo-miri cache disclosure. Not an instance of that class. * Run the trigger check when it changes, and make the waiver carry a reason Two review findings, both correct. The lint's own path filters did not list scripts/unsloth/check_workflow_triggers.py, so a commit weakening the checker would not have run it. Only test_*.py and the two older workflow checkers were listed. That is the same defect class this repository's sibling work has been closing, a filter that omits what the job executes, and it would have applied to the security check itself. Added to both trigger lists. The waiver was a bare substring test, so the marker counted anywhere in the file: alone with no argument at all, or embedded in a quoted run: string. Both let a workflow claim the exemption without recording the reason, which is the only thing requiring a marker buys. waiver_status now requires the marker on a COMMENT line and a non-empty reason, either on that line or on the comment lines directly beneath it, which is how a justification reads naturally. Three cases added to the test: the marker alone fails, the marker inside a run: string fails, a reason on the line beneath passes. 14 checks, all passing, and the two live workflow_run waivers already carry their reasons so neither changed. * Stop honouring the workflow_run waiver inside a run: block scalar waiver_status() required the marker to be on a comment line, which closed the quoted single-line case. It did not close the multi-line one, and that is how run: is actually written. A YAML block scalar body is opaque text, `#` opens a comment in shell exactly as it does in YAML, and a line-by-line reader cannot tell the two apart. So - run: | # lint:workflow_triggers-allow-workflow_run consumes no artifact satisfied the waiver while saying nothing about the workflow: it is script content, and script content is the part of a synced upstream workflow nobody reads closely. _block_scalar_lines() computes the extent of every block scalar and those lines no longer count, for the marker or for the reason lines under it. A block scalar runs until the indentation returns to its key's level, blank lines included, so the extent needs no YAML parser. The comment form with a reason still passes, which is the point: the rule rejects the location, not the justification. Three checks added to test_check_workflow_triggers.py: the block-scalar marker fails and reports the marker as absent rather than unreasoned, and the same file with a real comment above it passes. * Close two more ways the workflow_run waiver could be claimed without an argument Both indicator orders open a block scalar. YAML fixes no order between the chomping and the indentation indicator, so `|2-` is as valid as `|-2`, and matching one order only left the entire body of a `run: |2-` step back in scope as ordinary comment lines. That is the same hole the block-scalar fix just closed, reached by a different spelling of the same step. A continuation line must now announce itself with `# Justified:`. Accepting any adjacent comment as the reason meant the file's own `# SPDX-License-Identifier` and `# Copyright` header counted as the safety argument, so a bare marker placed directly above that header waived the trigger while saying nothing. Every workflow in this repository opens with that header, which made it the easiest reason in the tree to borrow by accident, and it would have read as perfectly ordinary in a diff. The introducer costs nothing here, because both live waivers already write `# Justified: ...` on the line under the marker. The failure message names the required form. Four checks: the `|2-` body, the license header rejected as a justification, `Justified:` with nothing after it rejected, and the real form still passing. * Take block-scalar ranges from the YAML parser instead of matching openers Two attempts at recognising a block-scalar opener with a regex both had holes, and they were holes of the same shape: first only `|-2` and not the equally valid `|2-`, now only `run:` and not `run :`. Each miss puts an entire script body back in scope as ordinary comment lines, so each one is a full bypass of the waiver rule rather than a rough edge, and the supply of valid YAML spellings is larger than the supply of patience for enumerating them. `yaml.compose` already knows exactly which lines are scalar content. The ranges now come from each node's `start_mark` and `end_mark`, which is shorter than the regex it replaces and closes the class rather than another instance of it. Separately, the optional inline label no longer stands in for the reason it introduces. `strip(" #:-")` turned a bare `Justified:` into the non-empty string `Justified` and accepted it, so the inline spelling passed where the equivalent separate `# Justified:` line was correctly refused. The label is removed explicitly and what remains has to be text. Three checks: the `run : |` body, the empty inline label rejected, and `Justified: <reason>` inline still passing. * Take the scalar extent from the marks properly, in both directions Two defects in the parser-based version, and they fail in opposite directions. The end mark is exclusive when its column is 0. PyYAML reports a block scalar followed by a dedented line as ending at (that line, column 0), so including the endpoint unconditionally swallowed the line AFTER the block. A real waiver comment placed immediately after a `run: |` step was therefore read as script content and the file failed with "has no comment". That is a false failure on a correct file, which is the worse of the two errors this rule can make, and it would have been hit by the next person who put the waiver at the bottom. Every scalar style counts, not just `|` and `>`. A double-quoted scalar can span lines too, and a continuation line of one can begin with `#`; PyYAML reads that as content while a `|`-and-`>`-only version read it as a comment, so the marker could be smuggled into a quoted `run:` value. The general rule needs no enumeration: whatever lies strictly inside a scalar's extent is that scalar's value, by definition of the marks. Renamed to _scalar_content_lines, since it is no longer about block scalars specifically. Both verified against PyYAML directly before fixing: the block in the first case reports end (6, 0) pointing at the waiver line, and the quoted scalar in the second spans three lines with the marker on the middle one. Two checks added, one per direction. * Require the waiver directive to end at a token boundary A substring test recognised the canonical directive inside `# lint:workflow_triggers-allow-workflow_runs` and then read the trailing `s` as the justification, so one mistyped character both claimed the waiver and supplied its own reason. Checked before fixing: that spelling returned (True, ''), while the correctly spelled bare marker on the same input was refused. A typo was a more effective waiver than the real thing. Matched with a `(?![\w-])` boundary now, so the directive has to end where it ends. `-anyway` and `_now` suffixes are refused for the same reason, and the canonical form followed by a space, a colon or ` Justified:` still reads its reason. Six checks, three each way. * Anchor the waiver directive, and host the check where a sync cannot skip it The directive now has to OPEN the comment. An unanchored search accepted it anywhere in any sentence, so # Never use lint:workflow_triggers-allow-workflow_run without review waived the trigger and offered `without review` as the justification: a comment saying the opposite of a waiver, read as one. Verified before fixing, it returned (True, ''). Together with the token boundary added in the previous commit, the directive now has to start and end where it says it does. The check also gets a second home in unsloth-upstream-sync-guard.yml, and that is the one that matters for the case it was written for. Its usual host filters on `paths`, and GitHub evaluates a `paths` filter against only the FIRST 300 changed files. An upstream sync is routinely far larger, so a sync reintroducing upstream's bench.yml under its active name could land without the host ever starting: the check skipped precisely when it applies, which is worse than not having it, because the module docstring claims that host guarantees coverage. The sync guard has no `paths` filter and already runs on every push to master, which is how a sync arrives. Keeping the check in both places is deliberate: the lint host gives fast feedback on an ordinary pull request, this one gives the guarantee. Fitting, since the sync guard already refuses to trust a truncated answer from the compare API for the same reason. Three checks for the anchoring, one per mid-comment shape. * Let the sync guard block a sync, not just report one after it lands The second host added in the previous commit was push-only, so for the case it was meant to cover it ran after the merge, with the reintroduced `pull_request_target` workflow already active on master. That is a notification, not a guard, and the finding it was answering was specifically about a sync pull request whose workflow file falls outside GitHub's 300-file path-filter window. The guard now also subscribes to `pull_request`, unfiltered. No `paths:` there either, for the same reason the filter was the problem in the first place. The two existing steps ask whether a recorded upstream commit is still an ancestor of master and whether the fork delta stays inside .github/ and scripts/unsloth/. Both are questions about master's own history and are meaningless against a pull request head, so they keep their current behaviour via `if: github.event_name != 'pull_request'`. Only the trigger check runs on both events, which is the one that has to be able to fail a sync before it merges. * Require a written reason, not leftover punctuation `strip(" #:-")` left `!!!` and `***` standing as non-empty strings, so a marker followed by punctuation claimed the waiver while recording nothing. Confirmed before fixing: both returned (True, ''). `_is_reason` requires three word characters. The threshold is arbitrary in the way any threshold here would be, and the docstring says so along with the honest scope of the rule: it rules out the degenerate case only. Whether an argument is a GOOD argument is a question for the reviewer, not for a lint. Applied to both the inline form and the `# Justified:` continuation line. Nine checks: four punctuation shapes each way, plus a short real reason still passing so the threshold does not demand an essay. * Count letters, not word characters, when judging a waiver reason Python's `\w` includes the underscore and digits, so `___` cleared the three-word-character threshold while saying exactly as much as the `!!!` the threshold was added to reject. Counted as letters now. The docstring keeps the same honest framing, and is a raw string so the `\w` in it does not become a syntax warning. Three checks added: `___`, `123` and `_ _ _`, each rejected inline and on a continuation line.
Reject pull_request_target in CI, so a sync cannot reintroduce it (#229) * Reject pull_request_target in CI, so a sync cannot reintroduce it Deleting bench.yml.disabled removed the file but not the hazard. This is a fork, and an upstream sync can bring back upstream's bench.yml under its original, ACTIVE name at any time. unsloth-upstream-sync-guard.yml checks that the recorded sync point is still an ancestor of master and says nothing about what the synced workflows do, so that reintroduction would arrive green. scripts/unsloth/check_workflow_triggers.py refuses pull_request_target outright and requires workflow_run to carry a written justification, matching the rule unslothai/unsloth enforces in scripts/lint_workflow_triggers.py, with the same escape-hatch comment spelling so the two repositories read the same way. Hosted in unsloth-pr-set-lint.yml because its push and pull_request triggers both already include .github/workflows/*.yml, so every workflow edit runs it, including the merge commit of a sync. It only reads .yml and .yaml, which is all GitHub loads: a neighbouring .disabled file is genuinely inert, and the moment someone renames it the check sees it. There is a test for exactly that pair. The two live workflow_run workflows now state why they are safe rather than leaving it to be re-derived. Neither was changed in behaviour: unsloth-prebuilt-retry.yml has no actions/checkout step at all, reads no artifact from the triggering run, holds no secrets, and uses actions: write only to POST a rerun in this same repository. The workflow it watches is schedule and dispatch only, so the trigger is never fork-controlled. unsloth-repin-bot.yml checks out with no ref:, which on a workflow_run event resolves to the default branch rather than any pull request head, with persist-credentials: false set deliberately so the automatic GITHUB_TOKEN extraheader cannot shadow the step-scoped REPIN_TOKEN, which is scrubbed from every log line it could reach. scripts/unsloth/test_check_workflow_triggers.py covers the live tree, both file extensions, a reintroduced bench.yml, the .disabled form being correctly ignored, the justified and unjustified workflow_run cases, and an empty or missing workflows directory failing rather than passing vacuously. 11 checks, all passing. Found during the organisation-wide audit against the 2026-09-21 cargo-miri cache disclosure. Not an instance of that class. * Run the trigger check when it changes, and make the waiver carry a reason Two review findings, both correct. The lint's own path filters did not list scripts/unsloth/check_workflow_triggers.py, so a commit weakening the checker would not have run it. Only test_*.py and the two older workflow checkers were listed. That is the same defect class this repository's sibling work has been closing, a filter that omits what the job executes, and it would have applied to the security check itself. Added to both trigger lists. The waiver was a bare substring test, so the marker counted anywhere in the file: alone with no argument at all, or embedded in a quoted run: string. Both let a workflow claim the exemption without recording the reason, which is the only thing requiring a marker buys. waiver_status now requires the marker on a COMMENT line and a non-empty reason, either on that line or on the comment lines directly beneath it, which is how a justification reads naturally. Three cases added to the test: the marker alone fails, the marker inside a run: string fails, a reason on the line beneath passes. 14 checks, all passing, and the two live workflow_run waivers already carry their reasons so neither changed. * Stop honouring the workflow_run waiver inside a run: block scalar waiver_status() required the marker to be on a comment line, which closed the quoted single-line case. It did not close the multi-line one, and that is how run: is actually written. A YAML block scalar body is opaque text, `#` opens a comment in shell exactly as it does in YAML, and a line-by-line reader cannot tell the two apart. So - run: | # lint:workflow_triggers-allow-workflow_run consumes no artifact satisfied the waiver while saying nothing about the workflow: it is script content, and script content is the part of a synced upstream workflow nobody reads closely. _block_scalar_lines() computes the extent of every block scalar and those lines no longer count, for the marker or for the reason lines under it. A block scalar runs until the indentation returns to its key's level, blank lines included, so the extent needs no YAML parser. The comment form with a reason still passes, which is the point: the rule rejects the location, not the justification. Three checks added to test_check_workflow_triggers.py: the block-scalar marker fails and reports the marker as absent rather than unreasoned, and the same file with a real comment above it passes. * Close two more ways the workflow_run waiver could be claimed without an argument Both indicator orders open a block scalar. YAML fixes no order between the chomping and the indentation indicator, so `|2-` is as valid as `|-2`, and matching one order only left the entire body of a `run: |2-` step back in scope as ordinary comment lines. That is the same hole the block-scalar fix just closed, reached by a different spelling of the same step. A continuation line must now announce itself with `# Justified:`. Accepting any adjacent comment as the reason meant the file's own `# SPDX-License-Identifier` and `# Copyright` header counted as the safety argument, so a bare marker placed directly above that header waived the trigger while saying nothing. Every workflow in this repository opens with that header, which made it the easiest reason in the tree to borrow by accident, and it would have read as perfectly ordinary in a diff. The introducer costs nothing here, because both live waivers already write `# Justified: ...` on the line under the marker. The failure message names the required form. Four checks: the `|2-` body, the license header rejected as a justification, `Justified:` with nothing after it rejected, and the real form still passing. * Take block-scalar ranges from the YAML parser instead of matching openers Two attempts at recognising a block-scalar opener with a regex both had holes, and they were holes of the same shape: first only `|-2` and not the equally valid `|2-`, now only `run:` and not `run :`. Each miss puts an entire script body back in scope as ordinary comment lines, so each one is a full bypass of the waiver rule rather than a rough edge, and the supply of valid YAML spellings is larger than the supply of patience for enumerating them. `yaml.compose` already knows exactly which lines are scalar content. The ranges now come from each node's `start_mark` and `end_mark`, which is shorter than the regex it replaces and closes the class rather than another instance of it. Separately, the optional inline label no longer stands in for the reason it introduces. `strip(" #:-")` turned a bare `Justified:` into the non-empty string `Justified` and accepted it, so the inline spelling passed where the equivalent separate `# Justified:` line was correctly refused. The label is removed explicitly and what remains has to be text. Three checks: the `run : |` body, the empty inline label rejected, and `Justified: <reason>` inline still passing. * Take the scalar extent from the marks properly, in both directions Two defects in the parser-based version, and they fail in opposite directions. The end mark is exclusive when its column is 0. PyYAML reports a block scalar followed by a dedented line as ending at (that line, column 0), so including the endpoint unconditionally swallowed the line AFTER the block. A real waiver comment placed immediately after a `run: |` step was therefore read as script content and the file failed with "has no comment". That is a false failure on a correct file, which is the worse of the two errors this rule can make, and it would have been hit by the next person who put the waiver at the bottom. Every scalar style counts, not just `|` and `>`. A double-quoted scalar can span lines too, and a continuation line of one can begin with `#`; PyYAML reads that as content while a `|`-and-`>`-only version read it as a comment, so the marker could be smuggled into a quoted `run:` value. The general rule needs no enumeration: whatever lies strictly inside a scalar's extent is that scalar's value, by definition of the marks. Renamed to _scalar_content_lines, since it is no longer about block scalars specifically. Both verified against PyYAML directly before fixing: the block in the first case reports end (6, 0) pointing at the waiver line, and the quoted scalar in the second spans three lines with the marker on the middle one. Two checks added, one per direction. * Require the waiver directive to end at a token boundary A substring test recognised the canonical directive inside `# lint:workflow_triggers-allow-workflow_runs` and then read the trailing `s` as the justification, so one mistyped character both claimed the waiver and supplied its own reason. Checked before fixing: that spelling returned (True, ''), while the correctly spelled bare marker on the same input was refused. A typo was a more effective waiver than the real thing. Matched with a `(?![\w-])` boundary now, so the directive has to end where it ends. `-anyway` and `_now` suffixes are refused for the same reason, and the canonical form followed by a space, a colon or ` Justified:` still reads its reason. Six checks, three each way. * Anchor the waiver directive, and host the check where a sync cannot skip it The directive now has to OPEN the comment. An unanchored search accepted it anywhere in any sentence, so # Never use lint:workflow_triggers-allow-workflow_run without review waived the trigger and offered `without review` as the justification: a comment saying the opposite of a waiver, read as one. Verified before fixing, it returned (True, ''). Together with the token boundary added in the previous commit, the directive now has to start and end where it says it does. The check also gets a second home in unsloth-upstream-sync-guard.yml, and that is the one that matters for the case it was written for. Its usual host filters on `paths`, and GitHub evaluates a `paths` filter against only the FIRST 300 changed files. An upstream sync is routinely far larger, so a sync reintroducing upstream's bench.yml under its active name could land without the host ever starting: the check skipped precisely when it applies, which is worse than not having it, because the module docstring claims that host guarantees coverage. The sync guard has no `paths` filter and already runs on every push to master, which is how a sync arrives. Keeping the check in both places is deliberate: the lint host gives fast feedback on an ordinary pull request, this one gives the guarantee. Fitting, since the sync guard already refuses to trust a truncated answer from the compare API for the same reason. Three checks for the anchoring, one per mid-comment shape. * Let the sync guard block a sync, not just report one after it lands The second host added in the previous commit was push-only, so for the case it was meant to cover it ran after the merge, with the reintroduced `pull_request_target` workflow already active on master. That is a notification, not a guard, and the finding it was answering was specifically about a sync pull request whose workflow file falls outside GitHub's 300-file path-filter window. The guard now also subscribes to `pull_request`, unfiltered. No `paths:` there either, for the same reason the filter was the problem in the first place. The two existing steps ask whether a recorded upstream commit is still an ancestor of master and whether the fork delta stays inside .github/ and scripts/unsloth/. Both are questions about master's own history and are meaningless against a pull request head, so they keep their current behaviour via `if: github.event_name != 'pull_request'`. Only the trigger check runs on both events, which is the one that has to be able to fail a sync before it merges. * Require a written reason, not leftover punctuation `strip(" #:-")` left `!!!` and `***` standing as non-empty strings, so a marker followed by punctuation claimed the waiver while recording nothing. Confirmed before fixing: both returned (True, ''). `_is_reason` requires three word characters. The threshold is arbitrary in the way any threshold here would be, and the docstring says so along with the honest scope of the rule: it rules out the degenerate case only. Whether an argument is a GOOD argument is a question for the reviewer, not for a lint. Applied to both the inline form and the `# Justified:` continuation line. Nine checks: four punctuation shapes each way, plus a short real reason still passing so the threshold does not demand an essay. * Count letters, not word characters, when judging a waiver reason Python's `\w` includes the underscore and digits, so `___` cleared the three-word-character threshold while saying exactly as much as the `!!!` the threshold was added to reject. Counted as letters now. The docstring keeps the same honest framing, and is a raw string so the `\w` in it does not become a syntax warning. Three checks added: `___`, `123` and `_ _ _`, each rejected inline and on a continuation line.
Reject pull_request_target in CI, so a sync cannot reintroduce it (#229) * Reject pull_request_target in CI, so a sync cannot reintroduce it Deleting bench.yml.disabled removed the file but not the hazard. This is a fork, and an upstream sync can bring back upstream's bench.yml under its original, ACTIVE name at any time. unsloth-upstream-sync-guard.yml checks that the recorded sync point is still an ancestor of master and says nothing about what the synced workflows do, so that reintroduction would arrive green. scripts/unsloth/check_workflow_triggers.py refuses pull_request_target outright and requires workflow_run to carry a written justification, matching the rule unslothai/unsloth enforces in scripts/lint_workflow_triggers.py, with the same escape-hatch comment spelling so the two repositories read the same way. Hosted in unsloth-pr-set-lint.yml because its push and pull_request triggers both already include .github/workflows/*.yml, so every workflow edit runs it, including the merge commit of a sync. It only reads .yml and .yaml, which is all GitHub loads: a neighbouring .disabled file is genuinely inert, and the moment someone renames it the check sees it. There is a test for exactly that pair. The two live workflow_run workflows now state why they are safe rather than leaving it to be re-derived. Neither was changed in behaviour: unsloth-prebuilt-retry.yml has no actions/checkout step at all, reads no artifact from the triggering run, holds no secrets, and uses actions: write only to POST a rerun in this same repository. The workflow it watches is schedule and dispatch only, so the trigger is never fork-controlled. unsloth-repin-bot.yml checks out with no ref:, which on a workflow_run event resolves to the default branch rather than any pull request head, with persist-credentials: false set deliberately so the automatic GITHUB_TOKEN extraheader cannot shadow the step-scoped REPIN_TOKEN, which is scrubbed from every log line it could reach. scripts/unsloth/test_check_workflow_triggers.py covers the live tree, both file extensions, a reintroduced bench.yml, the .disabled form being correctly ignored, the justified and unjustified workflow_run cases, and an empty or missing workflows directory failing rather than passing vacuously. 11 checks, all passing. Found during the organisation-wide audit against the 2026-09-21 cargo-miri cache disclosure. Not an instance of that class. * Run the trigger check when it changes, and make the waiver carry a reason Two review findings, both correct. The lint's own path filters did not list scripts/unsloth/check_workflow_triggers.py, so a commit weakening the checker would not have run it. Only test_*.py and the two older workflow checkers were listed. That is the same defect class this repository's sibling work has been closing, a filter that omits what the job executes, and it would have applied to the security check itself. Added to both trigger lists. The waiver was a bare substring test, so the marker counted anywhere in the file: alone with no argument at all, or embedded in a quoted run: string. Both let a workflow claim the exemption without recording the reason, which is the only thing requiring a marker buys. waiver_status now requires the marker on a COMMENT line and a non-empty reason, either on that line or on the comment lines directly beneath it, which is how a justification reads naturally. Three cases added to the test: the marker alone fails, the marker inside a run: string fails, a reason on the line beneath passes. 14 checks, all passing, and the two live workflow_run waivers already carry their reasons so neither changed. * Stop honouring the workflow_run waiver inside a run: block scalar waiver_status() required the marker to be on a comment line, which closed the quoted single-line case. It did not close the multi-line one, and that is how run: is actually written. A YAML block scalar body is opaque text, `#` opens a comment in shell exactly as it does in YAML, and a line-by-line reader cannot tell the two apart. So - run: | # lint:workflow_triggers-allow-workflow_run consumes no artifact satisfied the waiver while saying nothing about the workflow: it is script content, and script content is the part of a synced upstream workflow nobody reads closely. _block_scalar_lines() computes the extent of every block scalar and those lines no longer count, for the marker or for the reason lines under it. A block scalar runs until the indentation returns to its key's level, blank lines included, so the extent needs no YAML parser. The comment form with a reason still passes, which is the point: the rule rejects the location, not the justification. Three checks added to test_check_workflow_triggers.py: the block-scalar marker fails and reports the marker as absent rather than unreasoned, and the same file with a real comment above it passes. * Close two more ways the workflow_run waiver could be claimed without an argument Both indicator orders open a block scalar. YAML fixes no order between the chomping and the indentation indicator, so `|2-` is as valid as `|-2`, and matching one order only left the entire body of a `run: |2-` step back in scope as ordinary comment lines. That is the same hole the block-scalar fix just closed, reached by a different spelling of the same step. A continuation line must now announce itself with `# Justified:`. Accepting any adjacent comment as the reason meant the file's own `# SPDX-License-Identifier` and `# Copyright` header counted as the safety argument, so a bare marker placed directly above that header waived the trigger while saying nothing. Every workflow in this repository opens with that header, which made it the easiest reason in the tree to borrow by accident, and it would have read as perfectly ordinary in a diff. The introducer costs nothing here, because both live waivers already write `# Justified: ...` on the line under the marker. The failure message names the required form. Four checks: the `|2-` body, the license header rejected as a justification, `Justified:` with nothing after it rejected, and the real form still passing. * Take block-scalar ranges from the YAML parser instead of matching openers Two attempts at recognising a block-scalar opener with a regex both had holes, and they were holes of the same shape: first only `|-2` and not the equally valid `|2-`, now only `run:` and not `run :`. Each miss puts an entire script body back in scope as ordinary comment lines, so each one is a full bypass of the waiver rule rather than a rough edge, and the supply of valid YAML spellings is larger than the supply of patience for enumerating them. `yaml.compose` already knows exactly which lines are scalar content. The ranges now come from each node's `start_mark` and `end_mark`, which is shorter than the regex it replaces and closes the class rather than another instance of it. Separately, the optional inline label no longer stands in for the reason it introduces. `strip(" #:-")` turned a bare `Justified:` into the non-empty string `Justified` and accepted it, so the inline spelling passed where the equivalent separate `# Justified:` line was correctly refused. The label is removed explicitly and what remains has to be text. Three checks: the `run : |` body, the empty inline label rejected, and `Justified: <reason>` inline still passing. * Take the scalar extent from the marks properly, in both directions Two defects in the parser-based version, and they fail in opposite directions. The end mark is exclusive when its column is 0. PyYAML reports a block scalar followed by a dedented line as ending at (that line, column 0), so including the endpoint unconditionally swallowed the line AFTER the block. A real waiver comment placed immediately after a `run: |` step was therefore read as script content and the file failed with "has no comment". That is a false failure on a correct file, which is the worse of the two errors this rule can make, and it would have been hit by the next person who put the waiver at the bottom. Every scalar style counts, not just `|` and `>`. A double-quoted scalar can span lines too, and a continuation line of one can begin with `#`; PyYAML reads that as content while a `|`-and-`>`-only version read it as a comment, so the marker could be smuggled into a quoted `run:` value. The general rule needs no enumeration: whatever lies strictly inside a scalar's extent is that scalar's value, by definition of the marks. Renamed to _scalar_content_lines, since it is no longer about block scalars specifically. Both verified against PyYAML directly before fixing: the block in the first case reports end (6, 0) pointing at the waiver line, and the quoted scalar in the second spans three lines with the marker on the middle one. Two checks added, one per direction. * Require the waiver directive to end at a token boundary A substring test recognised the canonical directive inside `# lint:workflow_triggers-allow-workflow_runs` and then read the trailing `s` as the justification, so one mistyped character both claimed the waiver and supplied its own reason. Checked before fixing: that spelling returned (True, ''), while the correctly spelled bare marker on the same input was refused. A typo was a more effective waiver than the real thing. Matched with a `(?![\w-])` boundary now, so the directive has to end where it ends. `-anyway` and `_now` suffixes are refused for the same reason, and the canonical form followed by a space, a colon or ` Justified:` still reads its reason. Six checks, three each way. * Anchor the waiver directive, and host the check where a sync cannot skip it The directive now has to OPEN the comment. An unanchored search accepted it anywhere in any sentence, so # Never use lint:workflow_triggers-allow-workflow_run without review waived the trigger and offered `without review` as the justification: a comment saying the opposite of a waiver, read as one. Verified before fixing, it returned (True, ''). Together with the token boundary added in the previous commit, the directive now has to start and end where it says it does. The check also gets a second home in unsloth-upstream-sync-guard.yml, and that is the one that matters for the case it was written for. Its usual host filters on `paths`, and GitHub evaluates a `paths` filter against only the FIRST 300 changed files. An upstream sync is routinely far larger, so a sync reintroducing upstream's bench.yml under its active name could land without the host ever starting: the check skipped precisely when it applies, which is worse than not having it, because the module docstring claims that host guarantees coverage. The sync guard has no `paths` filter and already runs on every push to master, which is how a sync arrives. Keeping the check in both places is deliberate: the lint host gives fast feedback on an ordinary pull request, this one gives the guarantee. Fitting, since the sync guard already refuses to trust a truncated answer from the compare API for the same reason. Three checks for the anchoring, one per mid-comment shape. * Let the sync guard block a sync, not just report one after it lands The second host added in the previous commit was push-only, so for the case it was meant to cover it ran after the merge, with the reintroduced `pull_request_target` workflow already active on master. That is a notification, not a guard, and the finding it was answering was specifically about a sync pull request whose workflow file falls outside GitHub's 300-file path-filter window. The guard now also subscribes to `pull_request`, unfiltered. No `paths:` there either, for the same reason the filter was the problem in the first place. The two existing steps ask whether a recorded upstream commit is still an ancestor of master and whether the fork delta stays inside .github/ and scripts/unsloth/. Both are questions about master's own history and are meaningless against a pull request head, so they keep their current behaviour via `if: github.event_name != 'pull_request'`. Only the trigger check runs on both events, which is the one that has to be able to fail a sync before it merges. * Require a written reason, not leftover punctuation `strip(" #:-")` left `!!!` and `***` standing as non-empty strings, so a marker followed by punctuation claimed the waiver while recording nothing. Confirmed before fixing: both returned (True, ''). `_is_reason` requires three word characters. The threshold is arbitrary in the way any threshold here would be, and the docstring says so along with the honest scope of the rule: it rules out the degenerate case only. Whether an argument is a GOOD argument is a question for the reviewer, not for a lint. Applied to both the inline form and the `# Justified:` continuation line. Nine checks: four punctuation shapes each way, plus a short real reason still passing so the threshold does not demand an essay. * Count letters, not word characters, when judging a waiver reason Python's `\w` includes the underscore and digits, so `___` cleared the three-word-character threshold while saying exactly as much as the `!!!` the threshold was added to reject. Counted as letters now. The docstring keeps the same honest framing, and is a raw string so the `\w` in it does not become a syntax warning. Three checks added: `___`, `123` and `_ _ _`, each rejected inline and on a continuation line.
Publish in parallel again, and defer instead of giving up (#223) #221 made assemble `needs:` every build child so it could not miss a deadline. That fixed the lost releases and cost the thing the old design was built for: assemble only starts acquiring a runner once the last leg is green, so it joins the back of the queue to run a job that takes seconds. Today run 35359064416 went green across all 42 legs in 24 minutes and then sat waiting for a runner behind roughly 680 pending jobs. The workflow already recorded 109 minutes for that wait on a reference run. Both designs had a real failure. Early-start could not outlive GitHub's 6h per-job cap, so its waiter carried a deadline and run 34538187859 published nothing with 42 green builds. `needs:` has no cap but pays full queue latency at the end, every night. So keep the parallel start and remove what made it fatal. The waiter is back, and hitting its deadline now prints PUBLISH_DEFERRED_WAITER_DEADLINE, which unsloth-prebuilt-retry.yml recognises as infrastructure. The rerun re-runs assemble alone with a fresh budget, the legs have long finished by then, and the waiter sees them all green and publishes immediately. Fast path unchanged; the pathological path publishes late instead of never. The classifier stays fail-closed. Verified against the cases that matter: it retries the waiter deadline and a failed cache restore, and refuses a compile error, a GGML_ASSERT and a corrupt archive. Verified: 8/8 scripts/unsloth tests, every workflow parses, check_workflow_scalars 18375 of 21000, check_workflow_outputs 0 dangling.
Build a CUDA bundle for Windows on ARM (#218) * Build a CUDA bundle for Windows on ARM Windows ARM64 hosts with an NVIDIA GPU are the only CUDA hosts we publish nothing for: the installer looks for a windows-arm64-cuda bundle, finds none, and falls back to ggml-org's single llama-bin-win-cuda-13.4-arm64.zip. Add the leg that produces it. It cross compiles ggml-cuda on an x64 runner with the amd64_arm64 MSVC toolset and CUDA 13.4 (the only Windows toolkit with ARM64 target libraries), then merges the backend into the arm64 CPU bundle the same run already builds. Same split upstream uses for its arm64 CUDA zip. * Quote the toolchain path in the pwsh configure line
Build a CUDA bundle for Windows on ARM (#218) * Build a CUDA bundle for Windows on ARM Windows ARM64 hosts with an NVIDIA GPU are the only CUDA hosts we publish nothing for: the installer looks for a windows-arm64-cuda bundle, finds none, and falls back to ggml-org's single llama-bin-win-cuda-13.4-arm64.zip. Add the leg that produces it. It cross compiles ggml-cuda on an x64 runner with the amd64_arm64 MSVC toolset and CUDA 13.4 (the only Windows toolkit with ARM64 target libraries), then merges the backend into the arm64 CPU bundle the same run already builds. Same split upstream uses for its arm64 CUDA zip. * Quote the toolchain path in the pwsh configure line
Build a CUDA bundle for Windows on ARM (#218) * Build a CUDA bundle for Windows on ARM Windows ARM64 hosts with an NVIDIA GPU are the only CUDA hosts we publish nothing for: the installer looks for a windows-arm64-cuda bundle, finds none, and falls back to ggml-org's single llama-bin-win-cuda-13.4-arm64.zip. Add the leg that produces it. It cross compiles ggml-cuda on an x64 runner with the amd64_arm64 MSVC toolset and CUDA 13.4 (the only Windows toolkit with ARM64 target libraries), then merges the backend into the arm64 CPU bundle the same run already builds. Same split upstream uses for its arm64 CUDA zip. * Quote the toolchain path in the pwsh configure line
PreviousNext