You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(minimald): run the hook advisory proof over a stand-in tree root - #3100
a_hook_launch_does_not_advise_unenforced_placement read the host's real classifier tree at /sys/fs/cgroup/minimald.slice. It handled two host states:
No tree: the test ran.
A tree this process can place into: the test skipped.
It failed on the third state: a tree is installed, but the test runs outside the delegated slice. That is any developer's shell on a host where min finalize-install has run. In that state the hook launch logs "this daemon cannot place a process in the classifier tree…", a note the test didn't accept.
Change
Both launches now answer over a root under the test's temp state dir, with an empty mount table. They use the launcher's classifier_root / classifier_mountinfo seam, which the stand-in-tree tests already use. As a result:
Every host reads as tree-less.
The test expects exactly one note ("this host has no classifier tree to place a box in").
The skip is removed.
#[allow(clippy::await_holding_lock)] becomes #[expect], since the test can no longer return before its first await.
Test-only change; no production code is touched.
Verified
On a host with the classifier installed and the test outside the slice, the case that failed before: it now passes.
Mutation check: with hook_launcher.for_hooks = false, the test fails with "a hook launch is not a session start and records no advisory". It still catches the regression it exists for.
Full suite:cargo nextest run -p minimal -p minimald passes 2359/2364. The 5 failures are the netns integration tests that need root, the same as on main on this host.
clippy -D warnings and cargo fmt --check are clean.
The issue also mentioned the guest refusal test's similar skip. It skips rather than fails on this host, so I left it alone here.
a_hook_launch_does_not_advise_unenforced_placement read the host's real
classifier tree. It accepted two host states (no tree; a tree it can
place into, where it skipped) and failed on the third: a tree installed
but the test running outside the delegated slice, which is any
developer's shell on a host where `min finalize-install` has run. There
the launch logs "this daemon cannot place a process in the classifier
tree", a note the test did not accept.
Both launches now answer over a root under the test's temp state dir,
with an empty mount table, through the launcher's classifier_root and
classifier_mountinfo seam the stand-in-tree tests already use. Every
host reads as tree-less, so the test records one note, needs no skip,
and the lint allow becomes an expect.
Fixesgominimal/inbox#1215.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016CYvTxBZ7SjPrDqmzAv4No
No actionable comments were generated in the recent review.
Walkthrough
The test no longer skips based on host classifier placement. Both launches use an uncovered stand-in root, and the test expects the single tree-absent note.
Changes: Placement advisory test
Layer / File(s)
Summary
Stand-in root and note assertion crates/minimald/src/box_runtime/tests.rs
The test removes the host-placement probe and skip. It configures both launches with an uncovered stand-in root and empty mount info, then checks for the single tree-absent note.
gominimal/minimal#3081: Both PRs update the hook-launch advisory test to use a stand-in classifier root and remove the host-dependent skip.
Suggested reviewers:mitodrummer
Merge Risk: Minimal · up to 3b799
The test now uses a host-independent fixture while retaining its hook-advisory check. No merge-blocking risk was identified.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title clearly describes the stand-in tree root used by the test.
Description check
The description explains the change, links the issue, and reports test results. It omits the template checklist and test output.
Linked Issues check
[#1215] The hook test gives both launches a temporary classifier root and an empty mount table. It removes the host-dependent skip and checks the tree-absent note. The session control and hook asserti…
Out of Scope Changes check
[#1215] The changes are limited to the affected test and its documentation. The lint change follows the removal of the early return.
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check
Explanation
[#1215] The hook test gives both launches a temporary classifier root and an empty mount table. It removes the host-dependent skip and checks the tree-absent note. The session control and hook assertion remain. The author checked the guest-refusal skip and reports that it skips rather than fails on the affected host.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the launch once more No host-grown tree can sway the score Two stand-in roots are set in place One absent-tree note marks the case The test now hops through every space
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Essentials
Run ID: 7d6e615a-7d0e-441c-8b29-43359210885c
Commits
Reviewing files that changed from the base of the PR and between e04514e and 3b799c6.
Files selected for processing: 1
crates/minimald/src/box_runtime/tests.rs
Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes gominimal/inbox#1215.
Problem
a_hook_launch_does_not_advise_unenforced_placementread the host's real classifier tree at/sys/fs/cgroup/minimald.slice. It handled two host states:It failed on the third state: a tree is installed, but the test runs outside the delegated slice. That is any developer's shell on a host where
min finalize-installhas run. In that state the hook launch logs "this daemon cannot place a process in the classifier tree…", a note the test didn't accept.Change
Both launches now answer over a root under the test's temp state dir, with an empty mount table. They use the launcher's
classifier_root/classifier_mountinfoseam, which the stand-in-tree tests already use. As a result:#[allow(clippy::await_holding_lock)]becomes#[expect], since the test can no longer return before its first await.Test-only change; no production code is touched.
Verified
hook_launcher.for_hooks = false, the test fails with "a hook launch is not a session start and records no advisory". It still catches the regression it exists for.cargo nextest run -p minimal -p minimaldpasses 2359/2364. The 5 failures are the netns integration tests that need root, the same as on main on this host.-D warningsandcargo fmt --checkare clean.The issue also mentioned the guest refusal test's similar skip. It skips rather than fails on this host, so I left it alone here.
🤖 Generated with Claude Code
https://claude.ai/code/session_016CYvTxBZ7SjPrDqmzAv4No
Summary by CodeRabbit