fix(brain): exclude host-policy actions on Ubuntu Core - #414
Conversation
vladimirrott
left a comment
There was a problem hiding this comment.
Reviewed at 24b88ea2. Approving.
Small and exactly on the thing I flagged as optional in #384. Two checks mattered here and both hold.
The discriminator is the real production value. hint.id == "ubuntu-core" is only useful if something actually produces that string. apps/sysknife-cli/src/runner.rs:58 is the single bridge between detection and planning, it maps DistroId::UbuntuCore to "ubuntu-core", and both callers go through it:
$ grep -rn 'distro_id_to_hint' --include='*.rs' crates apps
apps/sysknife-cli/src/mcp_server.rs:792: planner = planner.with_distro(crate::runner::distro_id_to_hint(d));
apps/sysknife-cli/src/runner.rs:1758: planner = planner.with_distro(distro_id_to_hint(d));
So the guard sits on both the CLI and the MCP surface, not on a string nothing emits.
The assertion is exhaustive rather than illustrative. Iterating every entry in KNOWN_ACTIONS and requiring membership to equal !action_requires_supported_host(action) means an action added later cannot quietly land in the Ubuntu Core catalogue. A hand-picked list of five names would not have held.
The interaction with #412
You have this and #412 both editing the same dispatcher test region, so I merged them and ran it rather than reading the diffs and hoping:
merged head 908317b, 0 conflicts
$ cargo test --offline --locked -p sysknife-daemon --lib -- a_low_risk_action_that_mutates_is_not_treated_as_a_read
test result: ok. 1 passed; 897 filtered out
Your comment edit and #412's change of the fixture from Some(12) to Some(11) coexist correctly: #412 makes 12 eligible, so the "this test needs an ineligible host" premise needed a different version, and the merged file has both.
Before merge
No closing keyword in the body. This one has no issue behind it, since it came out of review feedback on #384, so nothing to close. Worth a line in the body saying that, otherwise the next person auditing the tracker cannot tell whether an issue was missed.
docs/action-compatibility.md gained the paragraph and says plainly that this does not claim Ubuntu Core execution support. That distinction is the one this repo keeps having to make, and stating it in the doc rather than only in the commit is right.
An
ubuntu-corehint carries the Debian family tag, which previously offered apt actions despite Ubuntu Core being ineligible and immutable. Apply the shared host-policy filter before the family catalogue rules, retaining neutral diagnostics. Extend the existing distro catalogue test to cover every action under this hint.Follow-up to #384's optional review feedback. The production dispatcher comment and portable Observer-read explanation were already corrected in the merged base; this also corrects the remaining stale test comment and documents the Ubuntu Core catalogue boundary.
Validation: cargo fmt and diff checks. Full Linux workspace and live LLM/VM E2E are unavailable on this Windows host; CI will validate Rust integration. No Rust test count change. Codex assisted this change.