Conversation
Reject cross command vectors that accidentally inject a `+<toolchain>` argument while preserving cargo fallback behaviour. Add regression tests for normal cross command construction, the cargo fallback path, and the guard's negative case.
Avoid requiring `uv`, `uvx`, or `action-validator` to be discoverable through hook PATH settings. Prefer known local executable locations for Ruff and the GitHub Action validator while keeping command-name fallbacks for other developer environments.
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactor action to build explicit argv lists for cross and cargo invocations; add GitHub Actions logging helpers; enforce rejecting Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Runner/CLI
participant Main as action:main.py
participant GHA as gha helper
participant Exec as cross/cargo
participant Container as Docker/container
CLI->>Main: invoke with config (use_cross, toolchain, target)
Main->>Main: _assemble_build_command(build_info)
Main->>GHA: debug("final argv: ...")
Main->>Exec: execute argv
Exec-->>Main: ProcessExecutionError(retcode)
alt retcode == 125 and requires_cross_container
Main->>GHA: warning("container startup failed, falling back to cargo")
Main->>Exec: execute fallback cargo argv (may include +toolchain)
else other retcode
Main->>GHA: error("cross command rejected +toolchain" / re-raise)
Main->>CLI: exit(1)
end
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 6 warnings)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Reviewer's GuideHardens rust-build-release cross command construction to forbid passing + overrides to cross while keeping cargo fallbacks unchanged, and makes Makefile lint targets resolve Ruff and action-validator from known locations before falling back to PATH-based resolution. Sequence diagram for hardened cross command construction and executionsequenceDiagram
participant Main as main
participant CrossDecision as _CrossDecision
participant BuildCross as _build_cross_command
participant Guard as _assert_cross_command_has_no_toolchain_override
participant Executor as plumbum_local_cross
participant Cross as cross_process
Main->>CrossDecision: compute_decision()
alt decision_use_cross
Main->>BuildCross: _build_cross_command(decision, target_to_build, manifest_path, features)
BuildCross->>Executor: select_executor(cross_executable)
BuildCross->>Guard: _assert_cross_command_has_no_toolchain_override(cmd)
Guard-->>BuildCross: return
BuildCross->>Executor: executor[cmd_slice]
BuildCross-->>Main: build_cmd
alt decision_has_toolchain_name
Main->>Main: build_cmd.with_env(RUSTUP_TOOLCHAIN)
end
Main->>Main: typer.echo("::debug:: cross argv: {build_cmd}")
Main->>Cross: execute(build_cmd)
else decision_use_cargo
Main->>Main: _build_cargo_command(...)
Main->>Cross: execute(cargo_build_cmd)
end
Class diagram for updated rust-build-release cross command helpersclassDiagram
class main {
+main()
}
class _CrossDecision {
+cross_path: str | None
+cargo_toolchain_spec: str | None
+toolchain_name: str | None
}
class _build_cross_command {
+_build_cross_command(decision, target_to_build, manifest_path, features) SupportsFormulate
}
class _assert_cross_command_has_no_toolchain_override {
+_assert_cross_command_has_no_toolchain_override(cmd) None
}
class _build_cargo_command {
+_build_cargo_command(cargo_toolchain_spec, target_to_build, manifest_argument, features) SupportsFormulate
}
class plumbum_local_executor {
+__getitem__(argv_slice) SupportsFormulate
}
main --> _CrossDecision : uses
main --> _build_cross_command : uses for cross builds
main --> _build_cargo_command : uses for cargo fallback
_build_cross_command --> _CrossDecision : reads
_build_cross_command --> _assert_cross_command_has_no_toolchain_override : validates cmd
_build_cross_command --> plumbum_local_executor : wraps cross_executable
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Scan the full cross argv after the executable for accidental `+<toolchain>` overrides. Strengthen command construction tests to assert the expected `cross build` prefix and cover cargo fallback behaviour when no toolchain specification is configured.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Use real executable permission bits when creating the fake cross binary in command tests. Include Bun's global binary directory when resolving `action-validator` for restricted hook environments.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Build the full cross argv, including normalised feature arguments, before checking for accidental `+<toolchain>` overrides. This keeps the guard aligned with the command vector passed to plumbum for execution.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Tighten the no-toolchain fallback test to assert the complete cargo argv. Omit the cargo toolchain slot when the specification is blank so fallback commands do not carry an empty argument.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Build cargo commands from a single argv list before handing them to plumbum so feature arguments are part of the exact command lookup.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Make command wrapper warning output injectable and report cross argv validation failures from main with a GitHub Actions error annotation. Add property and unit coverage for cross command guards plus root development and changelog notes for the argv assembly pattern.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Discover markdownlint from common Bun, local, and system install paths. Limit the markdownlint target to Markdown files changed from the configured base so existing unrelated documentation debt does not block this PR gate.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: Complex Method.github/actions/rust-build-release/src/main.py: main What lead to degradation?main increases in cyclomatic complexity from 10 to 12, threshold = 9 Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai have the following now been resolved? .github/actions/rust-build-release/src/main.py (2)
🔧 Patchprevious_engine, applied_engine = _configure_cross_container_engine(decision)
manifest_argument = _manifest_argument(manifest_path)
- if decision.use_cross:
- try:
- build_cmd = _build_cross_command(
- decision, target_to_build, manifest_argument, features
- )
- except ValueError as exc:
- typer.echo(
- f"::error:: cross command validation failed for target "
- f"'{target_to_build}': {exc}",
- err=True,
- )
- raise typer.Exit(1) from None
- if explicit_toolchain:
- build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env(
- RUSTUP_TOOLCHAIN=toolchain_name
- )
- typer.echo(f"::debug:: cross argv: {build_cmd}")
- else:
- build_cmd = _build_cargo_command(
- decision.cargo_toolchain_spec, target_to_build, manifest_argument, features
- )
try:
+ if decision.use_cross:
+ try:
+ build_cmd = _build_cross_command(
+ decision, target_to_build, manifest_argument, features
+ )
+ except ValueError as exc:
+ typer.echo(
+ f"::error:: cross command validation failed for target "
+ f"'{target_to_build}': {exc}",
+ err=True,
+ )
+ raise typer.Exit(1) from None
+ if explicit_toolchain:
+ build_cmd = typ.cast("_SupportsEnvFormulate", build_cmd).with_env(
+ RUSTUP_TOOLCHAIN=toolchain_name
+ )
+ typer.echo(f"::debug:: cross argv: {build_cmd}")
+ else:
+ build_cmd = _build_cargo_command(
+ decision.cargo_toolchain_spec,
+ target_to_build,
+ manifest_argument,
+ features,
+ )
run_cmd(build_cmd)
except ProcessExecutionError as exc:
_handle_cross_container_error(
exc, decision, target_to_build, manifest_argument, features
)🤖 Prompt for AI Agents
🤖 Prompt for AI Agents |
|
@coderabbitai have the following now been resolved? .github/actions/rust-build-release/src/main.py (1)
Refactor patchdef _build_cargo_command(
cargo_toolchain_spec: str, target_to_build: str, manifest_path: Path, features: str
) -> SupportsFormulate:
executor = local["cargo"]
cmd = [
"build",
"--manifest-path",
str(manifest_path),
"--release",
"--target",
target_to_build,
]
if cargo_toolchain_spec:
cmd.insert(0, cargo_toolchain_spec)
- build_cmd = executor[cmd]
normalized_features = _normalize_features(features)
if normalized_features:
- build_cmd = build_cmd["--features", normalized_features]
+ cmd.extend(["--features", normalized_features])
+ build_cmd = executor[cmd]
return _CommandWrapper(build_cmd, "cargo")🤖 Prompt for AI Agents |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 24778
Issue 1 — container-engine cleanup on the validation-failure path
Issue 2 —
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1388
|
|
@coderabbitai have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity. |
This comment was marked as resolved.
This comment was marked as resolved.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1547 🏁 Script executed: Length of output: 4351
|
This comment was marked as resolved.
This comment was marked as resolved.
Move workflow command formatting into a small `gha` adapter and keep `_CommandWrapper` focused on command display behaviour. Add snapshot and entrypoint coverage for cross debug and validation annotations, and move the developer guide under `docs/`.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Use `PurePosixPath` for the snapshot manifest fixture so the recorded cross argv stays identical on Windows and Unix hosts.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Keep `_build_cargo_command` as a pure command builder and emit the cargo argv debug line from `_assemble_build_command` after construction.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/rust-build-release/src/main.py:
- Around line 634-663: The function _assemble_build_command currently declares
an unused parameter toolchain_name; remove toolchain_name from the function
signature and update all call sites (e.g., the invocation in main() that passes
toolchain_name) to stop passing that argument, adjust the type hints/signature
documentation if present, and run a quick static check to ensure no other
references remain to toolchain_name in _assemble_build_command,
_build_cross_command, or related callers.
In @.github/actions/rust-build-release/tests/conftest.py:
- Around line 246-262: Add a clear class-level docstring to _EchoRecorder that
explains its dual-purpose design: that the class itself subclasses list[str] to
capture global echo messages, while calling _EchoRecorder(module) (the __call__
method) patches module.typer.echo and returns a separate list[tuple[str, bool]]
of (message, err) pairs for module-scoped captures with stderr tracking; mention
key symbols (_EchoRecorder, __call__, fake_echo, and monkeypatch) so maintainers
know where to look and how the two capture modes differ and interact.
In @.github/actions/rust-build-release/tests/test_commands.py:
- Around line 50-72: The conditional cleanup of packaging.version in
_load_main_module should be extracted into a small context manager (e.g.,
_preserve_packaging_version) so the restore logic is reusable and clearer;
implement _preserve_packaging_version to import packaging and packaging.version,
yield, and in its finally block delete packaging.version only if it still equals
the captured packaging_version, then call spec.loader.exec_module(module) inside
a with _preserve_packaging_version(): block in _load_main_module and remove the
inline finally block.
- Around line 138-144: The function injecting_guard currently defines an unused
parameter cmd which triggers ARG001; rename the parameter to _cmd (i.e., change
def injecting_guard(cmd: object) -> None: to def injecting_guard(_cmd: object)
-> None:) so the intent to ignore the argument is explicit; update the
monkeypatch.setattr reference remains the same for
_assert_cross_command_has_no_toolchain_override and no other behavior changes.
In `@Makefile`:
- Around line 75-76: The Makefile target using MARKDOWNLINT_BASE calls git diff
which errors when the base (e.g., origin/main) doesn't exist; fix by guarding or
defaulting MARKDOWNLINT_BASE before running the git diff command: add a short
check that verifies the ref (use git rev-parse --verify) and if it fails set
MARKDOWNLINT_BASE to a safe fallback (like empty string or HEAD) so the pipeline
`git diff --name-only --diff-filter=ACMRT $(MARKDOWNLINT_BASE) -- '*.md' | xargs
... $(MDLINT)` won't exit with non-zero; update the Makefile variable/recipe
that references MARKDOWNLINT_BASE and keep the MDLINT usage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 14460415-ed2c-459f-ae05-53e5ba3fbf20
📒 Files selected for processing (8)
.github/actions/rust-build-release/src/gha.py.github/actions/rust-build-release/src/main.py.github/actions/rust-build-release/tests/__snapshots__/test_commands.ambr.github/actions/rust-build-release/tests/conftest.py.github/actions/rust-build-release/tests/test_commands.pyMakefiledocs/developers-guide.mdpyproject.toml
Remove the unused `_assemble_build_command` parameter, clarify echo recorder capture modes, extract packaging import cleanup for command tests, and make markdown lint tolerate missing diff bases.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. ❌ Failed checks (2 errors, 5 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
Return command assembly errors as values so `main` owns annotations and process exits. Add guard spy coverage, cargo argv-shape tests, completion logging, and document cross toolchain selection.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Treat `subprocess.CalledProcessError` the same as plumbum process failures when cargo-based cross installation fails, so Windows fallback handling can continue without cross as intended.
There was a problem hiding this comment.
Gates Failed
(1 file with Excess Number of Function Arguments)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| test_commands.py | 1 advisory rule | 9.69 | Suppress |
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/rust-build-release/tests/conftest.py:
- Around line 261-269: echo_recorder.__call__ currently only replaces
module.typer.echo so emitted workflow commands sent via gha_debug, gha_warning,
and gha_error (which capture typer.echo at import time) are not recorded; update
__call__ to also patch the imported emitters by setting module.gha_debug.echo =
fake_echo, module.gha_warning.echo = fake_echo, and module.gha_error.echo =
fake_echo (or replace their callables to route through fake_echo) so messages
from gha_debug/gha_warning/gha_error are captured; alternatively remove the
callable capture mode and enforce global capture before importing the module
under test so those emitters bind to the patched echo.
In @.github/actions/rust-build-release/tests/test_commands.py:
- Around line 466-474: Replace the loose substring check for the validation
error with an exact GitHub Actions annotation assertion: after calling
main_module.main("aarch64-unknown-linux-gnu", "") and catching typer.Exit,
locate the recorded line in echo_recorder that exactly starts with the
"::error::" prefix and matches the full failure message produced by the code
(e.g. "::error::cross command validation failed: aarch64-unknown-linux-gnu");
assert that such a line exists and contains the target string so the test
verifies the exact ::error:: workflow annotation produced by main_module.main.
In `@Makefile`:
- Around line 17-19: Update docs/developers-guide.md to match the Makefile
changes: document the new MARKDOWNLINT_BASE variable and its role as the
diff-base/fallback, explain how MDLINT_CANDIDATES is discovered (checks
HOME/.bun/bin/markdownlint, HOME/.local/bin/markdownlint,
/usr/local/bin/markdownlint, /usr/bin/markdownlint) and that MDLINT chooses the
first found candidate or falls back to the literal "markdownlint"; also mirror
this explanation where the doc previously described the older behavior (the
section that discussed markdownlint discovery/diff-base) so the docs no longer
drift from the Makefile.
- Line 24: Add a deterministic Ruff version and use it for the fallback:
introduce a RUFF_VERSION variable (e.g., RUFF_VERSION ?= 0.11.13) and update the
RUFF fallback assignment so the fallback uses `uv tool run --from
ruff==$(RUFF_VERSION) ruff` instead of unpinned `uv tool run ruff`; update the
existing RUFF ?= ... line and add the RUFF_VERSION ?= ... declaration near it to
ensure consistent lint/format behavior across runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7c92d9d1-3121-49bd-a7bb-561714f30541
📒 Files selected for processing (4)
.github/actions/rust-build-release/src/main.py.github/actions/rust-build-release/tests/conftest.py.github/actions/rust-build-release/tests/test_commands.pyMakefile
| def __call__(self, module: ModuleType) -> list[tuple[str, bool]]: | ||
| """Patch *module*.typer.echo to record (message, err) tuples.""" | ||
| messages: list[tuple[str, bool]] = [] | ||
|
|
||
| def fake_echo(message: str, *, err: bool = False) -> None: | ||
| messages.append((message, err)) | ||
|
|
||
| self._monkeypatch.setattr(module.typer, "echo", fake_echo) | ||
| return messages |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Callable echo_recorder() usages:"
rg -n '\becho_recorder\s*\(' .github/actions/rust-build-release/tests
echo
echo "main.py gha imports:"
rg -n 'from gha import (debug|error|warning)' .github/actions/rust-build-release/src/main.py
echo
echo "gha helper definitions:"
rg -n 'def (debug|warning|error)\(' .github/actions/rust-build-release/src/gha.py -A2 -B1Repository: leynos/shared-actions
Length of output: 1728
Patch the gha_* emitters in module-scoped capture mode.
echo_recorder(module) only swaps module.typer.echo. main.py now emits through gha_debug, gha_warning, and gha_error, which bind echo=typer.echo at import time. Tests using callable mode cease to observe workflow-command output on this changed code path. Eight test cases are affected. Patch the imported gha_* callsites, or remove callable mode and enforce global capture before importing the module under test.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/rust-build-release/tests/conftest.py around lines 261 - 269,
echo_recorder.__call__ currently only replaces module.typer.echo so emitted
workflow commands sent via gha_debug, gha_warning, and gha_error (which capture
typer.echo at import time) are not recorded; update __call__ to also patch the
imported emitters by setting module.gha_debug.echo = fake_echo,
module.gha_warning.echo = fake_echo, and module.gha_error.echo = fake_echo (or
replace their callables to route through fake_echo) so messages from
gha_debug/gha_warning/gha_error are captured; alternatively remove the callable
capture mode and enforce global capture before importing the module under test
so those emitters bind to the patched echo.
| with pytest.raises(main_module.typer.Exit) as exc_info: | ||
| main_module.main("aarch64-unknown-linux-gnu", "") | ||
|
|
||
| assert exc_info.value.exit_code == 1 | ||
| error_lines = [ | ||
| line for line in echo_recorder if "cross command validation failed" in line | ||
| ] | ||
| assert len(error_lines) == 1 | ||
| assert "aarch64-unknown-linux-gnu" in error_lines[0] |
There was a problem hiding this comment.
Assert the full ::error:: workflow command here.
The current substring check still passes if this branch stops emitting a GitHub Actions annotation and falls back to plain typer.echo with the same text. Assert the exact ::error:: prefix and failure message so this test actually locks the contract the PR claims to cover.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/rust-build-release/tests/test_commands.py around lines 466 -
474, Replace the loose substring check for the validation error with an exact
GitHub Actions annotation assertion: after calling
main_module.main("aarch64-unknown-linux-gnu", "") and catching typer.Exit,
locate the recorded line in echo_recorder that exactly starts with the
"::error::" prefix and matches the full failure message produced by the code
(e.g. "::error::cross command validation failed: aarch64-unknown-linux-gnu");
assert that such a line exists and contains the target string so the test
verifies the exact ::error:: workflow annotation produced by main_module.main.
| MARKDOWNLINT_BASE ?= origin/main | ||
| MDLINT_CANDIDATES := $(wildcard $(HOME)/.bun/bin/markdownlint $(HOME)/.local/bin/markdownlint /usr/local/bin/markdownlint /usr/bin/markdownlint) | ||
| MDLINT ?= $(if $(MDLINT_CANDIDATES),$(firstword $(MDLINT_CANDIDATES)),markdownlint) |
There was a problem hiding this comment.
Synchronize developer documentation with markdownlint behaviour.
Update docs/developers-guide.md to document MDLINT candidate discovery and MARKDOWNLINT_BASE/diff-base fallback behaviour introduced on Lines 17-19 and Lines 75-77. Remove the current doc/code drift.
Also applies to: 75-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` around lines 17 - 19, Update docs/developers-guide.md to match the
Makefile changes: document the new MARKDOWNLINT_BASE variable and its role as
the diff-base/fallback, explain how MDLINT_CANDIDATES is discovered (checks
HOME/.bun/bin/markdownlint, HOME/.local/bin/markdownlint,
/usr/local/bin/markdownlint, /usr/bin/markdownlint) and that MDLINT chooses the
first found candidate or falls back to the literal "markdownlint"; also mirror
this explanation where the doc previously described the older behavior (the
section that discussed markdownlint discovery/diff-base) so the docs no longer
drift from the Makefile.
| ACTION_VALIDATOR_CANDIDATES := $(wildcard $(HOME)/.cargo/bin/action-validator $(HOME)/.bun/bin/action-validator /usr/local/bin/action-validator /usr/bin/action-validator) | ||
| ACTION_VALIDATOR ?= $(if $(ACTION_VALIDATOR_CANDIDATES),$(firstword $(ACTION_VALIDATOR_CANDIDATES)),action-validator) | ||
| RUFF_CANDIDATES := $(wildcard $(CURDIR)/.venv/bin/ruff $(HOME)/.local/bin/ruff /usr/local/bin/ruff /usr/bin/ruff) | ||
| RUFF ?= $(if $(RUFF_CANDIDATES),$(firstword $(RUFF_CANDIDATES)),uv tool run ruff) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
uv tool run unpinned package version resolution documentation
💡 Result:
When using uv tool run (or its alias uvx) without specifying a version (i.e., unpinned tool name like ruff), uv resolves and installs the latest compatible version of the tool package and its dependencies into a temporary, cached virtual environment isolated from any project. By default, uv's resolution strategy prefers the highest (latest) compatible version for each package, both direct (the tool) and transitive (its dependencies). This is the standard behavior across uv commands like uv pip install, uv run --with, and uv tool run. For uv tool run specifically: - On first run, it performs full resolution to select the latest versions satisfying the tool's declared requirements. - Subsequent runs reuse the cached environment unless the cache is pruned/refreshed, a different version is requested (e.g., ruff@latest or --from ruff==0.3.0), or --isolated is used. - The cache is disposable (in ~/.cache/uv), unlike installed tools via uv tool install which are persistent. To override the default "highest" resolution (e.g., for testing lowest versions), use the --resolution flag, which applies to uv tool run as it inherits from uv's run interface: - --resolution lowest: Lowest compatible versions for all dependencies. - --resolution lowest-direct: Lowest for direct (tool), highest for transitive. Example: uv tool run --resolution lowest-direct ruff check . Official documentation confirms uv resolves unpinned requirements to latest versions by default, with configurable strategies via --resolution. For reproducible exact versions, specify the tool version explicitly (e.g., uvx ruff@0.3.0) or use uv tool install first. No dedicated lockfile for tools; caching provides reproducibility until refreshed.
Citations:
- 1: https://docs.astral.sh/uv/concepts/resolution/
- 2: https://docs.astral.sh/uv/concepts/tools/
- 3: https://melmass.github.io/uv/resolution.html
- 4: https://docs.astral.sh/uv/reference/cli/
Pin the Ruff fallback version.
The unpinned uv tool run ruff fallback resolves to the latest compatible version of Ruff on first run and reuses the cached environment thereafter. However, uv's cache is disposable, meaning the fallback can drift to newer Ruff releases across CI and local runs, destabilising lint and format output. Specify an explicit version using --from ruff==<version> to ensure deterministic behaviour.
Apply a deterministic Ruff fallback
+RUFF_VERSION ?= 0.11.13
-RUFF ?= $(if $(RUFF_CANDIDATES),$(firstword $(RUFF_CANDIDATES)),uv tool run ruff)
+RUFF ?= $(if $(RUFF_CANDIDATES),$(firstword $(RUFF_CANDIDATES)),uv tool run --from ruff==$(RUFF_VERSION) ruff)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Makefile` at line 24, Add a deterministic Ruff version and use it for the
fallback: introduce a RUFF_VERSION variable (e.g., RUFF_VERSION ?= 0.11.13) and
update the RUFF fallback assignment so the fallback uses `uv tool run --from
ruff==$(RUFF_VERSION) ruff` instead of unpinned `uv tool run ruff`; update the
existing RUFF ?= ... line and add the RUFF_VERSION ?= ... declaration near it to
ensure consistent lint/format behavior across runs.
1 participant
Summary
This branch hardens
rust-build-releasecross command construction socrosscannot receive a+<toolchain>CLI override. The action continues to rely onrust-toolchain.tomlorRUSTUP_TOOLCHAINfor cross builds, while cargo-only paths and cargo fallback commands still permit the existing+<toolchain>argument.It also makes the repository check targets more resilient in hook environments by resolving Ruff and
action-validatorfrom known executable locations before falling back to command-name lookup.No linked issue, roadmap task, or execplan was found for this branch.
Review walkthrough
action-validatorare resolved for restricted hookPATHsettings.Validation
make check-fmt: passed.make typecheck: passed.make lint: passed.make test: passed,669 passed, 90 skipped.env PATH=/usr/bin:/bin HOME="$HOME" make check-fmt lint typecheck: passed.cross build --manifest-path Cargo.toml --release --target aarch64-unknown-linux-gnu.Notes
The PR is draft because it was opened by an agent and should receive maintainer review before it is marked ready.
Summary by Sourcery
Harden rust-build-release cross command construction to forbid passing toolchain overrides to cross while preserving cargo fallback behaviour, and make local lint tooling resolution more robust in constrained environments.
Enhancements:
Tests: