Skip to content

Pin module release builds to Rust 1.96.1 - #162

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix-module-rustc-skew
Sep 24, 2026
Merged

senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix-module-rustc-skew

Conversation

@senamakel

Copy link
Copy Markdown
Member

Pins the release compiler to the OpenHuman host toolchain (Rust 1.96.1). Cargo commands in CI and release workflows now select this version through rust-toolchain.toml. Part of tinyhumansai/openhuman#6614 and #6420. A new module release and an OpenHuman registry pin are still needed to replace already-published artifacts.

@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 55 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 4e6f42db-fd57-48be-b2ff-a5fc93a302d5

📥 Commits

Reviewing files that changed from the base of the PR and between b2a5e3a and 45ef4c0.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • rust-toolchain.toml

Comment @coderabbitai help to get the list of available commands.

@senamakel
senamakel merged commit 351d99f into tinyhumansai:main Sep 24, 2026
1 check passed
@tinysweeper

tinysweeper Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 3 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: high
Reviewed head: 45ef4c057669
Updated: 1790270043 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 0 Active findings 3
Tests 0 Noted findings 0
Documentation 0 Resolved findings 7
Configuration 2 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • medium · security · Pass the MSRV through the environment before invoking the shell — The MSRV output is currently expanded into the `run` script itself. Environment passing keeps the value as data and prevents shell parsing of repository-controlled content; define (\.github/workflows/ci\.yml:470)
  • medium · security · Install the components required by repository checks — The minimal rustup profile does not install `rustfmt`, `clippy`, or `llvm-tools-preview`, while the repository's CI checks require these components. Add them explicitly so a fresh (rust\-toolchain\.toml:3)
  • medium · description · Install toolchain components required by repository checks — The `minimal` profile includes only `rustc`, `cargo`, and `rust-std`. Repository checks (linting, formatting, etc.) require tools such as `clippy` and `rustfmt`. Without them, CI (\(pull request description\))

Resolved this pass

  • Install the components required by repository checks
  • Pass the MSRV through a quoted environment variable
  • Pass the MSRV through the environment before invoking the shell
  • Pass the MSRV through a quoted environment variable
  • Pass the MSRV through the environment before invoking the shell
  • Pass the MSRV through a quoted environment variable
  • Pass the MSRV through the environment before invoking the shell

Before merge

None.

Agent review details

critique

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 2 files; 3 findings. (3 already reported on an earlier push) _The code index is behind this pull request (indexed at `1142af252a15`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._

security

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 2 files; 3 findings. (1 already reported on an earlier push) _The code index is behind this pull request (indexed at `1142af252a15`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
  • Evidence: \.github/workflows/ci\.yml — Pass the MSRV through the environment before invoking the shell
  • Evidence: rust\-toolchain\.toml — Install the components required by repository checks

tests

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No behavioural change: nothing outside documentation, configuration and tests.

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The PR pins the release compiler to 1.96.1 and updates CI to use the MSRV version from `steps.msrv`, which fixes the earlier findings about passing the version. However, the `rust-toolchain.toml` uses `profile = "minimal"`, which does not include toolchain components (e.g., clippy, rustfmt) likely needed for repository checks, so that earlier finding remains untreated. The change is otherwise sound. (1 earlier finding(s) still open) _The code index is behind this pull request (indexed at `1142af252a15`), so retrieved context may be out of date._ _Memory was unavailable (model: cortex: v1/recall: timed out after 10s), so this review ran without it._
  • Evidence: \(pull request description\) — Install toolchain components required by repository checks

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No behavioural change: nothing outside documentation, configuration and tests.
Evidence and run details
  • Models: ladder/vectors, gpt-5.6-luna, deepseek-v4-flash
  • Spend: $0.003264
  • Tokens: 97531 input · 12734 output · 17835 cached · 51 embedding
Head State Pass summary
45ef4c057669 changes requested 3 active finding(s), 0 resolved finding(s) (at 1790269864)
45ef4c057669 changes requested 3 active finding(s), 7 resolved finding(s) (at 1790270043)

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0024 · 82,257 in / 6,876 out · 4,912 cached (6%)  · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 51 embedded
critique:    $0.0012 · 40,403 in / 2,997 out · 2,066 cached (5%)  · gpt-5.6-luna, deepseek-v4-flash
security:    $0.0010 · 36,922 in / 1,033 out · 1,822 cached (5%)  · gpt-5.6-luna
description: $0.0001 · 3,456 in  / 1,720 out · 1,024 cached (30%) · deepseek-v4-flash

Comment thread rust-toolchain.toml
@@ -0,0 +1,3 @@
[toolchain]
channel = "1.96.1"
profile = "minimal"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium critique confident

Install the components required by repository checks

A minimal rustup profile does not install rustfmt or clippy, so developers using this rust-toolchain.toml will fail the documented cargo fmt and cargo clippy commands with missing-component errors. Add both components to the toolchain configuration.

[RULE] missing-toolchain-components ·

Comment thread .github/workflows/ci.yml

- name: Build with the declared MSRV
run: cargo build --all-targets --all-features
run: cargo +${{ steps.msrv.outputs.version }} build --all-targets --all-features

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high security likely

Pass the MSRV through a quoted environment variable

steps.msrv.outputs.version is derived from Cargo.toml in the checked-out repository and is interpolated directly into a shell command. A crafted value that reaches this output could alter the command executed by the runner. Put the output in an environment variable and invoke Cargo with the quoted variable, for example cargo +"$MSRV" build ....


Additional critique observation

priority medium confident

Pass the MSRV through the environment before invoking the shell

[RULE] shell-interpolation

steps.msrv.outputs.version is derived from the repository's Cargo.toml, which is untrusted in pull-request CI, and is interpolated directly into run. Keep the expression in an env value and invoke cargo +"$MSRV" ... so repository-controlled data is not expanded as shell source.

[RULE] untrusted-shell-interpolation ·

@tinysweeper tinysweeper Bot added the priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. label Sep 24, 2026

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 2 lane(s) blocking, worst finding is medium.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0033 · 97,531 in / 12,734 out · 17,835 cached (18%) · ladder/vectors, gpt-5.6-luna, deepseek-v4-flash · 51 embedded
critique:    $0.0012 · 41,298 in / 2,919 out  · 7,733 cached (19%)  · gpt-5.6-luna, deepseek-v4-flash
security:    $0.0015 · 50,739 in / 3,248 out  · 7,286 cached (14%)  · gpt-5.6-luna
description: $0.0003 · 3,763 in  / 3,220 out  · 2,304 cached (61%)  · deepseek-v4-flash

Comment thread .github/workflows/ci.yml

- name: Build with the declared MSRV
run: cargo build --all-targets --all-features
run: cargo +${{ steps.msrv.outputs.version }} build --all-targets --all-features

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium security confident

Pass the MSRV through the environment before invoking the shell

The MSRV output is currently expanded into the run script itself. Environment passing keeps the value as data and prevents shell parsing of repository-controlled content; define an MSRV step environment variable from the output and use "+$MSRV" in the command.

[RULE] shell-injection ·

Comment thread rust-toolchain.toml
@@ -0,0 +1,3 @@
[toolchain]
channel = "1.96.1"
profile = "minimal"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium security confident

Install the components required by repository checks

The minimal rustup profile does not install rustfmt, clippy, or llvm-tools-preview, while the repository's CI checks require these components. Add them explicitly so a fresh checkout using this toolchain file can run the documented checks.

[RULE] missing-toolchain-components ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant