Feat/formal phase 2 3 kani and db proptests#15
Merged
Conversation
Add the Ratatui-based kcore-console appliance screen and wire it into the ISO/NixOS console path so local login is replaced by a read-only hypervisor status UI.
Update Ratatui and related lockfile entries so the appliance console dependency tree passes cargo audit before push.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughIntroduces a new TUI-based appliance console ( Changes
Sequence DiagramsequenceDiagram
participant Main as Main Thread
participant Worker as Background Worker
participant Inv as Inventory Collectors
participant Term as Terminal/Ratatui
Main->>Main: Initialize app state, enter raw mode
Main->>Worker: Spawn worker thread
loop Event Loop
Worker->>Inv: load_snapshot() periodically
Inv->>Inv: probe API, list_nics, list_disks, kcore_diagnostics, load_meta
Inv-->>Worker: Snapshot {meta, nics, disks, diag}
Worker-->>Main: Send snapshot via channel
Main->>Main: Receive & apply snapshot
Main->>Term: draw(app_state)
Term->>Term: Render tabs, tables, metadata
Main->>Main: Poll keyboard events
Main->>Main: Update selection / navigate pages / refresh
end
Main->>Main: Cleanup raw mode, restore screen
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 32 minutes and 31 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (5)
crates/kcore-console/src/inventory/api.rs (1)
26-30: Note:healthyis alwaystruecurrently.The
healthyfield is always set totruewhen reachable, making theReachable { healthy: false }→"degraded"path inDisplayunreachable. This is fine for the initial TCP-only probe, but consider adding a doc comment noting this is a placeholder for future gRPC health checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/kcore-console/src/inventory/api.rs` around lines 26 - 30, The reachable branch currently always sets ApiStatus::Reachable { healthy: true } (created where matches_peer(port) is checked), making the degraded path unreachable; add a brief doc comment on that branch or above the matches_peer check indicating this is a TCP-only probe placeholder and healthy is hard-coded true for now and will be replaced by a future gRPC health check to drive healthy=false when appropriate (reference ApiStatus::Reachable and matches_peer).crates/kcore-console/src/inventory/diagnostics.rs (1)
22-30: Consider distinguishing between systemctl status states.The current logic groups all non-
activestates under"(not installed or inactive)", butsystemctl is-activecan return distinct values likeinactive,failed,activating, ornot-found(for missing units). Distinguishingfailedfrominactivecould help operators diagnose issues faster.♻️ Suggested enhancement
if let Some(out) = run_active(unit) { if out == "active" { "healthy".to_string() + } else if out == "failed" { + "failed".to_string() + } else if out == "inactive" { + "inactive".to_string() } else { - format!("{out} (not installed or inactive)") + format!("{out}") } } else { "—".to_string() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/kcore-console/src/inventory/diagnostics.rs` around lines 22 - 30, The current mapping in diagnostics.rs that converts run_active(unit) output to "healthy" or a generic "(not installed or inactive)" should be expanded to handle distinct systemctl states: detect exact outputs from run_active(unit) such as "inactive", "failed", "activating", and "not-found"/"unknown" and map them to more descriptive strings (e.g., "inactive", "failed", "activating", "not found") while keeping "active" → "healthy" and the None case → "—"; modify the conditional block that calls run_active(unit) to branch on the returned string (or match) and return those explicit diagnostics so operators can distinguish failures from mere inactivity.docs/user/appliance-console.md (1)
78-82: Minor style: Rephrase to avoid repetitive sentence starts.Three consecutive sentences begin with "Confirm". Consider varying the phrasing for readability.
📝 Suggested rephrasing
If the console still shows a Linux login prompt: 1. Confirm `kcore-console.service` is enabled and running. -2. Confirm `getty@tty1.service` and `autovt@.service` are masked or disabled. -3. Confirm `getty@tty2.service` through `getty@tty6.service` are also disabled, +2. Verify that `getty@tty1.service` and `autovt@.service` are masked or disabled. +3. Check that `getty@tty2.service` through `getty@tty6.service` are also disabled, because users can switch virtual terminals with `Ctrl+Alt+F2` to `F6`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/appliance-console.md` around lines 78 - 82, The three consecutive lines starting with "Confirm" are repetitive; rephrase them for variety and clarity by using different verbs and sentence structures around the same service names (e.g., "Ensure kcore-console.service is enabled and running" for kcore-console.service, "Mask or disable getty@tty1.service and autovt@.service" for getty@tty1.service and autovt@.service, and "Disable getty@tty2.service through getty@tty6.service to prevent VT switching" for the range getty@tty2.service–getty@tty6.service), then keep the final step as a brief imperative like "Reboot and verify the screen on tty1" so the steps read smoothly without repeating "Confirm."modules/kcore-branding.nix (1)
106-109: Consider adding restart rate limiting.The service uses
Restart = "always"withRestartSec = "2s", but noStartLimitIntervalorStartLimitBurstis configured. If the console crashes repeatedly (e.g., due to a persistent TTY issue), systemd will restart it indefinitely every 2 seconds without rate limiting, which could impact system resources or flood logs.♻️ Suggested addition for rate limiting
serviceConfig = { Type = "simple"; Restart = "always"; RestartSec = "2s"; + }; + unitConfig = { + Conflicts = [ + "getty@tty1.service" + "autovt@tty1.service" + ]; + StartLimitInterval = "60s"; + StartLimitBurst = 5; };This allows 5 restarts within 60 seconds before systemd stops attempting restarts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/kcore-branding.nix` around lines 106 - 109, The unit's serviceConfig block currently sets Restart = "always" and RestartSec = "2s" without rate limiting; add systemd rate-limiting settings by defining StartLimitInterval (e.g., "60s") and StartLimitBurst (e.g., 5) alongside the existing fields so systemd will stop attempting rapid endless restarts after the configured burst; update the serviceConfig (and any related unit-generating function) to include StartLimitInterval and StartLimitBurst keys next to Restart and RestartSec.crates/kcore-console/src/ui/render.rs (1)
381-395: Render a placeholder when diagnostics data is missing.When
app.snapshot.diagis empty, this page renders a blank bordered panel instead of the required—fallback. That makes missing systemd data look like a rendering bug rather than “no data”.Proposed fix
fn draw_diagnostics(f: &mut Frame<'_>, area: Rect, app: &AppState) { - let mut line = String::new(); - for s in &app.snapshot.diag { - line.push_str(&format!(" {}: {} │", s.name, s.status)); - } + let line = if app.snapshot.diag.is_empty() { + "—".to_string() + } else { + app.snapshot + .diag + .iter() + .map(|s| format!("{}: {}", s.name, s.status)) + .collect::<Vec<_>>() + .join(" │ ") + }; f.render_widget( Paragraph::new(line).block( Block::default()As per coding guidelines "Treat missing host data as normal in the appliance console; display
—instead of panicking or leaving empty cells for unavailable sysfs,ip,lsblk, API, or systemd data".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/kcore-console/src/ui/render.rs` around lines 381 - 395, The diagnostics panel currently builds a concatenated line from app.snapshot.diag and leaves the widget blank when the Vec is empty; update draw_diagnostics to detect when app.snapshot.diag is empty and set the rendered text to a single placeholder "—" (em dash) instead of an empty string so the Paragraph in draw_diagnostics always shows the required fallback; locate the draw_diagnostics function and replace the current line-building logic to use the placeholder when app.snapshot.diag.is_empty(), otherwise join each entry as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/rules/kcore-console-tui.mdc:
- Around line 27-34: Add the workspace test gate to the shipping checklist by
inserting a step to run the full workspace tests before the package-specific
test; specifically, add "nix develop -c cargo test --workspace" just before the
existing "nix develop -c cargo test -p kcore-console" line in the checklist so
that the workspace-wide tests run prior to the package-level test.
In `@crates/kcore-console/build.rs`:
- Around line 1-20: In main() of build.rs emit cargo rerun directives so Cargo
will re-run the build script when the KCORE_GIT_REV env changes or when git refs
change: add a println!("cargo:rerun-if-env-changed=KCORE_GIT_REV") and one or
more println!("cargo:rerun-if-changed=...") calls for the repo refs (for example
.git/HEAD and .git/refs or the specific ref files you read) at the top of main()
before reading the env or invoking git (referencing the main function and the
KCORE_GIT_REV handling and the git rev-parse block).
In `@crates/kcore-console/src/app.rs`:
- Around line 104-126: Add focused unit tests in the existing tests module to
cover selection clamping when inventories shrink: create AppState instances (via
AppState::new with Snapshot::default()) and exercise scenarios where network_sel
and storage_sel would become out-of-range (1) when inventories are empty (verify
selections are clamped/reset to a valid value) and (2) when an inventory shrinks
from a size that left a previously valid selection out-of-bounds (simulate
shrinking and assert network_sel/storage_sel are clamped). Reference the
AppState, Snapshot, network_sel, and storage_sel symbols so tests explicitly
validate the new clamp helpers' behavior after inventory changes.
In `@crates/kcore-console/src/inventory/network.rs`:
- Around line 60-65: The current logic in mgmt_if uses
default_if.clone().or_else(...) so environment vars KCORE_MGMT_IFACE /
KCORE_MANAGEMENT_IF are only consulted when no default route exists; change the
selection to prefer env vars first: read KCORE_MGMT_IFACE and
KCORE_MANAGEMENT_IF into an Option (or try env first) and if neither present
fall back to read_default_ifname(); update the mgmt_if creation (and any uses of
default_if/read_default_ifname) so explicit env overrides take precedence over
the detected default interface.
In `@crates/kcore-console/src/inventory/route.rs`:
- Around line 17-22: The else-if branch checking `metric == best_metric &&
best.is_none()` is unreachable and should be removed; in the block where
`metric`, `best_metric`, `best`, and `ifname` are handled (in
crates/kcore-console/src/inventory/route.rs) replace the three-branch logic with
a single comparison that sets `best_metric = metric` and `best = Some(ifname)`
when `metric < best_metric`, and drop the redundant `else if metric ==
best_metric && best.is_none()` branch entirely to simplify the `best` selection.
In `@crates/kcore-console/src/inventory/system.rs`:
- Around line 147-150: The hostname fallback uses "unknown" but should use the
console standard placeholder "—" for consistency; update the read_to_string call
that sets the hostname variable in inventory::system (the let hostname =
std::fs::read_to_string(...).map(...).unwrap_or_else(...)) to return "—" instead
of "unknown" so the Overview rendering uses the unified unavailable placeholder.
- Around line 92-101: The blocking subprocess call in run_ip_json() (and the
similar code at the other block) must be moved into a background worker that
periodically runs the "ip -j address show" command and writes parsed JSON into a
shared cache (e.g., an Arc<Mutex>/RwLock or sends updates over a channel);
change load_meta to read the last-known JSON from that cache/non-blocking
accessor instead of calling Command::output() directly, and have the worker
handle errors and update timestamps so load_meta can fall back gracefully when
data is stale or missing.
- Around line 200-203: api_endpoint is built from an unchecked env string; parse
env("KCORE_API_PORT") to u16 with the same fallback behavior as the probe before
formatting so the endpoint is always a valid port. Change the logic that
constructs api_endpoint to attempt env("KCORE_API_PORT").and_then(|s|
s.parse::<u16>().ok()).map_or(super::api::KCORE_API_PORT, |p| p) and then
format!("127.0.0.1:{}", port_value.to_string()) so you use the parsed u16 or
fall back to super::api::KCORE_API_PORT.
- Around line 103-119: parse_primary_ipv4 currently uses the `?` operator inside
the loop which aborts the whole function on the first malformed interface entry;
change the implementation to treat missing fields as non-fatal by replacing `?`
with safe checks (e.g., use as_array()/get().and_then(|v| v.as_str()) or nested
if let/when matching) so each interface is skipped when required fields are
missing and only return Some(local) when you successfully find an addr_info
entry with family == "inet", scope == "global", and a present "local" string;
keep the function signature and return None only after scanning all interfaces.
- Around line 131-237: Add focused unit tests for the new parsing/formatting and
env/file-fallback behavior: create a #[cfg(test)] module that tests
format_duration for 0s, seconds-only (<60), minutes, hours, and days; add tests
for parse_primary_ipv4 covering valid JSON, invalid JSON, missing fields, and
fallback when no management IP is present; and add tests for load_meta that
stub/override environment variables and temporary files to verify hostname
fallback, missing /proc/uptime handling, KCORE_API_PORT custom composition of
api_endpoint, management_url normalization (with/without scheme), and
remote_hint generation. Target the functions format_duration,
parse_primary_ipv4, and load_meta (and use helper functions env and
file_first_line behaviors) so tests exercise URL normalization and env/file
fallbacks as described.
In `@crates/kcore-console/src/lib.rs`:
- Around line 40-41: The terminal is put into raw/alternate-screen mode via
enable_raw_mode() and execute!(stdout, EnterAlternateScreen,
crossterm::cursor::Hide) but later early-returns (from Terminal::new, draw,
poll, read, etc.) can skip the explicit cleanup code (disable_raw_mode,
LeaveAlternateScreen, Show), leaving the terminal in an unusable state; fix by
introducing a RAII guard (e.g., TerminalGuard) created immediately after calling
enable_raw_mode/execute! that implements Drop to always run the cleanup (calling
disable_raw_mode(), execute!(stdout, LeaveAlternateScreen,
crossterm::cursor::Show)), or use a scopeguard so that any error path will still
restore the terminal before returning; ensure TerminalGuard is held for the
entire lifetime of the code that uses Terminal::new, draw, poll, read so cleanup
runs on all exits.
In `@crates/kcore-console/src/tty.rs`:
- Around line 6-28: Add focused tests for attach_tty covering Unix success, Unix
failure, and non-Unix error: create a test helper binary (under tests/) that
calls attach_tty(tty) and then writes to stdout/stderr, and in the Unix success
test spawn that helper via std::process::Command with a temp file or /dev/null
as the tty path and assert the child exits OK and produced expected output (this
avoids mutating the test runner's fds); for the Unix failure test spawn the
helper with a non-existent path and assert the child exits with an error (or the
attach_tty call returns Err with NotFound), and for non-Unix add a
#[cfg(not(unix))] unit/integration test that calls attach_tty and asserts it
returns Err with the "--tty is only supported on Unix-like systems" message;
reference the attach_tty function and the helper test binary name when adding
these tests.
In `@crates/kcore-disk-layout-yaml/src/lib.rs`:
- Around line 245-262: In nix_string, prevent Nix interpolation and fix
non-ASCII escapes: when iterating chars in fn nix_string(s: &str), detect the
"${" sequence and emit "\${" instead of leaving "${" untouched (e.g., if current
char is '$' and next is '{' output "\\${" and advance), and stop emitting
Rust-style \u{...} for non-ASCII chars—append non-ASCII characters directly
(they will be valid UTF-8 in the output) rather than formatting them as \u{...};
retain existing escapes for backslash, double-quote, newline, carriage return,
and tab, and keep the surrounding double quotes. Ensure you update the loop
logic in nix_string to handle the multi-char "${" case and to treat non-ASCII by
pushing the char itself.
In `@docs/appliance-console.md`:
- Around line 71-73: Update the docs to match the actual UI text rendered by the
console: replace occurrences like "API: unavailable" / "API: available" with the
exact string the renderer prints ("API endpoint: … Status: available" or "API
endpoint: … Status: unavailable") so the guide points operators to the correct
on-screen status; update both the paragraph around lines 71–73 and the similar
text around lines 120–121 to reference the renderer's "API endpoint: … Status:
available|unavailable" output (as produced by
crates/kcore-console/src/ui/render.rs).
In `@packaging/systemd/kcore-console.service`:
- Line 16: The unit's Conflicts setting only lists getty@tty1.service which can
allow autovt@tty1.service to race for tty1; update the Conflicts entry in the
kcore-console.service unit (the Conflicts line) to include autovt@tty1.service
as well (comma-separate or add a space-separated token depending on existing
formatting) so the unit conflicts with both getty@tty1.service and
autovt@tty1.service.
---
Nitpick comments:
In `@crates/kcore-console/src/inventory/api.rs`:
- Around line 26-30: The reachable branch currently always sets
ApiStatus::Reachable { healthy: true } (created where matches_peer(port) is
checked), making the degraded path unreachable; add a brief doc comment on that
branch or above the matches_peer check indicating this is a TCP-only probe
placeholder and healthy is hard-coded true for now and will be replaced by a
future gRPC health check to drive healthy=false when appropriate (reference
ApiStatus::Reachable and matches_peer).
In `@crates/kcore-console/src/inventory/diagnostics.rs`:
- Around line 22-30: The current mapping in diagnostics.rs that converts
run_active(unit) output to "healthy" or a generic "(not installed or inactive)"
should be expanded to handle distinct systemctl states: detect exact outputs
from run_active(unit) such as "inactive", "failed", "activating", and
"not-found"/"unknown" and map them to more descriptive strings (e.g.,
"inactive", "failed", "activating", "not found") while keeping "active" →
"healthy" and the None case → "—"; modify the conditional block that calls
run_active(unit) to branch on the returned string (or match) and return those
explicit diagnostics so operators can distinguish failures from mere inactivity.
In `@crates/kcore-console/src/ui/render.rs`:
- Around line 381-395: The diagnostics panel currently builds a concatenated
line from app.snapshot.diag and leaves the widget blank when the Vec is empty;
update draw_diagnostics to detect when app.snapshot.diag is empty and set the
rendered text to a single placeholder "—" (em dash) instead of an empty string
so the Paragraph in draw_diagnostics always shows the required fallback; locate
the draw_diagnostics function and replace the current line-building logic to use
the placeholder when app.snapshot.diag.is_empty(), otherwise join each entry as
before.
In `@docs/user/appliance-console.md`:
- Around line 78-82: The three consecutive lines starting with "Confirm" are
repetitive; rephrase them for variety and clarity by using different verbs and
sentence structures around the same service names (e.g., "Ensure
kcore-console.service is enabled and running" for kcore-console.service, "Mask
or disable getty@tty1.service and autovt@.service" for getty@tty1.service and
autovt@.service, and "Disable getty@tty2.service through getty@tty6.service to
prevent VT switching" for the range getty@tty2.service–getty@tty6.service), then
keep the final step as a brief imperative like "Reboot and verify the screen on
tty1" so the steps read smoothly without repeating "Confirm."
In `@modules/kcore-branding.nix`:
- Around line 106-109: The unit's serviceConfig block currently sets Restart =
"always" and RestartSec = "2s" without rate limiting; add systemd rate-limiting
settings by defining StartLimitInterval (e.g., "60s") and StartLimitBurst (e.g.,
5) alongside the existing fields so systemd will stop attempting rapid endless
restarts after the configured burst; update the serviceConfig (and any related
unit-generating function) to include StartLimitInterval and StartLimitBurst keys
next to Restart and RestartSec.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c1470a3-12e1-4aeb-ac78-df0db8a83759
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
.cursor/rules/kcore-console-tui.mdcCargo.tomlcrates/kcore-console/Cargo.tomlcrates/kcore-console/build.rscrates/kcore-console/src/app.rscrates/kcore-console/src/inventory/api.rscrates/kcore-console/src/inventory/diagnostics.rscrates/kcore-console/src/inventory/disk.rscrates/kcore-console/src/inventory/format.rscrates/kcore-console/src/inventory/mod.rscrates/kcore-console/src/inventory/network.rscrates/kcore-console/src/inventory/route.rscrates/kcore-console/src/inventory/system.rscrates/kcore-console/src/lib.rscrates/kcore-console/src/main.rscrates/kcore-console/src/theme.rscrates/kcore-console/src/tty.rscrates/kcore-console/src/ui/mod.rscrates/kcore-console/src/ui/render.rscrates/kcore-disk-layout-yaml/Cargo.tomlcrates/kcore-disk-layout-yaml/src/lib.rscrates/kctl/Cargo.tomlcrates/kctl/src/commands/disk_layout.rscrates/kctl/src/main.rsdocs/README.mddocs/appliance-console.mddocs/day-2-filesystem-operations.mddocs/storage.mddocs/user/README.mddocs/user/appliance-console.mdflake.nixmodules/kcore-branding.nixpackaging/systemd/kcore-console.service
Comment on lines
+27
to
+34
| Before shipping a console change, run at least: | ||
|
|
||
| ```bash | ||
| nix develop -c cargo fmt | ||
| nix develop -c cargo test -p kcore-console | ||
| nix develop -c cargo clippy -p kcore-console --all-targets -- --deny warnings | ||
| nix build .#kcore-console | ||
| ``` |
There was a problem hiding this comment.
Add the workspace test gate to the shipping checklist.
This checklist only runs cargo test -p kcore-console, so a console change can still satisfy the local steps while breaking another crate in the workspace.
Proposed fix
```bash
nix develop -c cargo fmt
+nix develop -c cargo test --workspace
nix develop -c cargo test -p kcore-console
nix develop -c cargo clippy -p kcore-console --all-targets -- --deny warnings
nix build .#kcore-console</details>
As per coding guidelines "Run `cargo test --workspace` before committing to ensure all tests pass".
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.cursor/rules/kcore-console-tui.mdc around lines 27 - 34, Add the workspace
test gate to the shipping checklist by inserting a step to run the full
workspace tests before the package-specific test; specifically, add "nix develop
-c cargo test --workspace" just before the existing "nix develop -c cargo test
-p kcore-console" line in the checklist so that the workspace-wide tests run
prior to the package-level test.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper:8198686e-9684-49a1-b55c-f46c8a0b0a5f -->
<!-- d98c2f50 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+1
to
+20
| //! Embeds an optional VCS revision when `KCORE_GIT_REV` is set in the build environment. | ||
| fn main() { | ||
| if let Ok(rev) = std::env::var("KCORE_GIT_REV") { | ||
| if !rev.trim().is_empty() { | ||
| println!("cargo:rustc-env=KCORE_GIT_REV={rev}"); | ||
| return; | ||
| } | ||
| } | ||
| if let Some(r) = std::process::Command::new("git") | ||
| .args(["rev-parse", "--short", "HEAD"]) | ||
| .output() | ||
| .ok() | ||
| .filter(|o| o.status.success()) | ||
| .and_then(|o| String::from_utf8(o.stdout).ok()) | ||
| .map(|s| s.trim().to_string()) | ||
| .filter(|s| !s.is_empty()) | ||
| { | ||
| println!("cargo:rustc-env=KCORE_GIT_REV={r}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add cargo:rerun-if-* directives for correct incremental builds.
The build script doesn't declare when it should re-run, which may cause stale git revision info in incremental builds. Cargo defaults to re-running only when build.rs itself changes.
Proposed fix
//! Embeds an optional VCS revision when `KCORE_GIT_REV` is set in the build environment.
fn main() {
+ println!("cargo:rerun-if-env-changed=KCORE_GIT_REV");
+ // Re-run when HEAD changes (covers branch switches and commits)
+ println!("cargo:rerun-if-changed=.git/HEAD");
+
if let Ok(rev) = std::env::var("KCORE_GIT_REV") {
if !rev.trim().is_empty() {
println!("cargo:rustc-env=KCORE_GIT_REV={rev}");
return;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| //! Embeds an optional VCS revision when `KCORE_GIT_REV` is set in the build environment. | |
| fn main() { | |
| if let Ok(rev) = std::env::var("KCORE_GIT_REV") { | |
| if !rev.trim().is_empty() { | |
| println!("cargo:rustc-env=KCORE_GIT_REV={rev}"); | |
| return; | |
| } | |
| } | |
| if let Some(r) = std::process::Command::new("git") | |
| .args(["rev-parse", "--short", "HEAD"]) | |
| .output() | |
| .ok() | |
| .filter(|o| o.status.success()) | |
| .and_then(|o| String::from_utf8(o.stdout).ok()) | |
| .map(|s| s.trim().to_string()) | |
| .filter(|s| !s.is_empty()) | |
| { | |
| println!("cargo:rustc-env=KCORE_GIT_REV={r}"); | |
| } | |
| } | |
| //! Embeds an optional VCS revision when `KCORE_GIT_REV` is set in the build environment. | |
| fn main() { | |
| println!("cargo:rerun-if-env-changed=KCORE_GIT_REV"); | |
| // Re-run when HEAD changes (covers branch switches and commits) | |
| println!("cargo:rerun-if-changed=.git/HEAD"); | |
| if let Ok(rev) = std::env::var("KCORE_GIT_REV") { | |
| if !rev.trim().is_empty() { | |
| println!("cargo:rustc-env=KCORE_GIT_REV={rev}"); | |
| return; | |
| } | |
| } | |
| if let Some(r) = std::process::Command::new("git") | |
| .args(["rev-parse", "--short", "HEAD"]) | |
| .output() | |
| .ok() | |
| .filter(|o| o.status.success()) | |
| .and_then(|o| String::from_utf8(o.stdout).ok()) | |
| .map(|s| s.trim().to_string()) | |
| .filter(|s| !s.is_empty()) | |
| { | |
| println!("cargo:rustc-env=KCORE_GIT_REV={r}"); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-console/build.rs` around lines 1 - 20, In main() of build.rs
emit cargo rerun directives so Cargo will re-run the build script when the
KCORE_GIT_REV env changes or when git refs change: add a
println!("cargo:rerun-if-env-changed=KCORE_GIT_REV") and one or more
println!("cargo:rerun-if-changed=...") calls for the repo refs (for example
.git/HEAD and .git/refs or the specific ref files you read) at the top of main()
before reading the env or invoking git (referencing the main function and the
KCORE_GIT_REV handling and the git rev-parse block).
Comment on lines
+104
to
+126
| #[cfg(test)] | ||
| mod tests { | ||
| use super::{AppState, Page, Snapshot}; | ||
|
|
||
| #[test] | ||
| fn page_tabs_wrap() { | ||
| assert_eq!(Page::Overview.next(), Page::Network); | ||
| assert_eq!(Page::Help.next(), Page::Overview); | ||
| assert_eq!(Page::Overview.prev(), Page::Help); | ||
| } | ||
|
|
||
| #[test] | ||
| fn production_mode_disallows_q_exit() { | ||
| let s = AppState::new(false, Snapshot::default()); | ||
| assert!(!s.allow_exit_on_q()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn dev_mode_allows_q_exit() { | ||
| let s = AppState::new(true, Snapshot::default()); | ||
| assert!(s.allow_exit_on_q()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add tests for selection clamping after inventory shrink.
The new clamp helpers are what keep refreshes from leaving network_sel / storage_sel out of range, but the current test module only covers page wrap and q exit. Please add cases for empty inventories and for shrinking from an out-of-bounds selection.
As per coding guidelines "Add or update focused tests for parsing, formatting, navigation, and production-vs-dev exit policy in kcore-console whenever behavior changes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-console/src/app.rs` around lines 104 - 126, Add focused unit
tests in the existing tests module to cover selection clamping when inventories
shrink: create AppState instances (via AppState::new with Snapshot::default())
and exercise scenarios where network_sel and storage_sel would become
out-of-range (1) when inventories are empty (verify selections are clamped/reset
to a valid value) and (2) when an inventory shrinks from a size that left a
previously valid selection out-of-bounds (simulate shrinking and assert
network_sel/storage_sel are clamped). Reference the AppState, Snapshot,
network_sel, and storage_sel symbols so tests explicitly validate the new clamp
helpers' behavior after inventory changes.
Comment on lines
+60
to
+65
| let default_if = read_default_ifname(); | ||
| let mgmt_if = default_if.clone().or_else(|| { | ||
| std::env::var("KCORE_MGMT_IFACE") | ||
| .ok() | ||
| .or_else(|| std::env::var("KCORE_MANAGEMENT_IF").ok()) | ||
| }); |
There was a problem hiding this comment.
Environment variable overrides are ignored when a default route exists.
The current logic uses default_if.clone().or_else(...) which means KCORE_MGMT_IFACE / KCORE_MANAGEMENT_IF are only checked when there is no default route. Typically, explicit environment variable configuration should override automatic detection.
Proposed fix to prioritize env vars
let default_if = read_default_ifname();
- let mgmt_if = default_if.clone().or_else(|| {
- std::env::var("KCORE_MGMT_IFACE")
- .ok()
- .or_else(|| std::env::var("KCORE_MANAGEMENT_IF").ok())
- });
+ let mgmt_if = std::env::var("KCORE_MGMT_IFACE")
+ .ok()
+ .or_else(|| std::env::var("KCORE_MANAGEMENT_IF").ok())
+ .or(default_if.clone());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let default_if = read_default_ifname(); | |
| let mgmt_if = default_if.clone().or_else(|| { | |
| std::env::var("KCORE_MGMT_IFACE") | |
| .ok() | |
| .or_else(|| std::env::var("KCORE_MANAGEMENT_IF").ok()) | |
| }); | |
| let default_if = read_default_ifname(); | |
| let mgmt_if = std::env::var("KCORE_MGMT_IFACE") | |
| .ok() | |
| .or_else(|| std::env::var("KCORE_MANAGEMENT_IF").ok()) | |
| .or(default_if.clone()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-console/src/inventory/network.rs` around lines 60 - 65, The
current logic in mgmt_if uses default_if.clone().or_else(...) so environment
vars KCORE_MGMT_IFACE / KCORE_MANAGEMENT_IF are only consulted when no default
route exists; change the selection to prefer env vars first: read
KCORE_MGMT_IFACE and KCORE_MANAGEMENT_IF into an Option (or try env first) and
if neither present fall back to read_default_ifname(); update the mgmt_if
creation (and any uses of default_if/read_default_ifname) so explicit env
overrides take precedence over the detected default interface.
Comment on lines
+17
to
+22
| if metric < best_metric { | ||
| best_metric = metric; | ||
| best = Some(ifname); | ||
| } else if metric == best_metric && best.is_none() { | ||
| best = Some(ifname); | ||
| } |
There was a problem hiding this comment.
Unreachable branch can be removed.
The condition metric == best_metric && best.is_none() on line 20 is unreachable. Since best_metric starts at u32::MAX and any successfully parsed metric will be < u32::MAX, the first matching default route will always trigger the metric < best_metric branch (lines 17-19), setting best to Some(...). Subsequent matches with equal metrics will have best.is_some(), making the && best.is_none() condition always false.
Proposed simplification
if metric < best_metric {
best_metric = metric;
best = Some(ifname);
- } else if metric == best_metric && best.is_none() {
- best = Some(ifname);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-console/src/inventory/route.rs` around lines 17 - 22, The
else-if branch checking `metric == best_metric && best.is_none()` is unreachable
and should be removed; in the block where `metric`, `best_metric`, `best`, and
`ifname` are handled (in crates/kcore-console/src/inventory/route.rs) replace
the three-branch logic with a single comparison that sets `best_metric = metric`
and `best = Some(ifname)` when `metric < best_metric`, and drop the redundant
`else if metric == best_metric && best.is_none()` branch entirely to simplify
the `best` selection.
Comment on lines
+40
to
+41
| enable_raw_mode()?; | ||
| execute!(stdout, EnterAlternateScreen, crossterm::cursor::Hide)?; |
There was a problem hiding this comment.
Restore terminal state on every error path, not just on normal exit.
After lines 40-41, any later ? (Terminal::new, draw, poll, read) returns before the cleanup at lines 157-162 runs. That can leave the caller stuck in raw mode / alternate screen after an I/O failure.
Proposed fix
pub fn run(opts: Options) -> io::Result<()> {
+ struct TerminalGuard;
+ impl Drop for TerminalGuard {
+ fn drop(&mut self) {
+ let _ = disable_raw_mode();
+ let _ = execute!(io::stdout(), LeaveAlternateScreen, crossterm::cursor::Show);
+ }
+ }
+
if let Some(ref p) = opts.tty {
tty::attach_tty(p)?;
}
let mut stdout = stdout();
enable_raw_mode()?;
execute!(stdout, EnterAlternateScreen, crossterm::cursor::Hide)?;
+ let _terminal_guard = TerminalGuard;
install_panic_hook(!opts.dev);
@@
- execute!(
- terminal.backend_mut(),
- LeaveAlternateScreen,
- crossterm::cursor::Show
- )?;
- disable_raw_mode()?;
Ok(())
}Also applies to: 72-83, 85-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-console/src/lib.rs` around lines 40 - 41, The terminal is put
into raw/alternate-screen mode via enable_raw_mode() and execute!(stdout,
EnterAlternateScreen, crossterm::cursor::Hide) but later early-returns (from
Terminal::new, draw, poll, read, etc.) can skip the explicit cleanup code
(disable_raw_mode, LeaveAlternateScreen, Show), leaving the terminal in an
unusable state; fix by introducing a RAII guard (e.g., TerminalGuard) created
immediately after calling enable_raw_mode/execute! that implements Drop to
always run the cleanup (calling disable_raw_mode(), execute!(stdout,
LeaveAlternateScreen, crossterm::cursor::Show)), or use a scopeguard so that any
error path will still restore the terminal before returning; ensure
TerminalGuard is held for the entire lifetime of the code that uses
Terminal::new, draw, poll, read so cleanup runs on all exits.
Comment on lines
+6
to
+28
| pub fn attach_tty(tty: &str) -> std::io::Result<()> { | ||
| #[cfg(unix)] | ||
| { | ||
| use std::os::unix::io::AsRawFd; | ||
| let f = OpenOptions::new().read(true).write(true).open(tty)?; | ||
| let fd = f.as_raw_fd(); | ||
| unsafe { | ||
| for stream in [0, 1, 2] { | ||
| if libc::dup2(fd, stream) < 0 { | ||
| return Err(std::io::Error::last_os_error()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| #[cfg(not(unix))] | ||
| { | ||
| let _ = tty; | ||
| return Err(std::io::Error::other( | ||
| "--tty is only supported on Unix-like systems", | ||
| )); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only check for tests related to tty attach behavior in kcore-console.
rg -n --type=rust -C2 '\battach_tty\b|--tty|tty::attach_tty'
rg -n --type=rust -C2 '\battach_tty\b|--tty|tty::attach_tty' -g '*test*' -g '*tests*'Repository: rtacconi/kcore
Length of output: 1003
Add focused tests for attach_tty behavior.
This function introduces critical file descriptor redirection via libc::dup2 and a platform-specific fallback; no existing tests reference this function. Per coding guidelines, focused tests must cover at least Unix success/failure paths and non-Unix error handling before this behavior is committed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-console/src/tty.rs` around lines 6 - 28, Add focused tests for
attach_tty covering Unix success, Unix failure, and non-Unix error: create a
test helper binary (under tests/) that calls attach_tty(tty) and then writes to
stdout/stderr, and in the Unix success test spawn that helper via
std::process::Command with a temp file or /dev/null as the tty path and assert
the child exits OK and produced expected output (this avoids mutating the test
runner's fds); for the Unix failure test spawn the helper with a non-existent
path and assert the child exits with an error (or the attach_tty call returns
Err with NotFound), and for non-Unix add a #[cfg(not(unix))] unit/integration
test that calls attach_tty and asserts it returns Err with the "--tty is only
supported on Unix-like systems" message; reference the attach_tty function and
the helper test binary name when adding these tests.
Comment on lines
+245
to
+262
| /// Escape a string for use inside Nix double quotes. | ||
| fn nix_string(s: &str) -> String { | ||
| let mut out = String::with_capacity(s.len() + 2); | ||
| out.push('"'); | ||
| for ch in s.chars() { | ||
| match ch { | ||
| '\\' => out.push_str("\\\\"), | ||
| '"' => out.push_str("\\\""), | ||
| '\n' => out.push_str("\\n"), | ||
| '\r' => out.push_str("\\r"), | ||
| '\t' => out.push_str("\\t"), | ||
| c if c.is_ascii() => out.push(c), | ||
| c => out.push_str(&format!("\\u{{{:x}}}", c as u32)), | ||
| } | ||
| } | ||
| out.push('"'); | ||
| out | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Nix double-quoted strings, how do you emit a literal ${...}sequence, and are\u{...} escapes valid syntax?
💡 Result:
In Nix double-quoted strings, to emit a literal ${...} sequence (prevent interpolation), escape the opening ${ with a backslash: use ${. Example: "echo ${PATH}" results in the literal text echo ${PATH}. Are \u{...} escapes valid syntax in Nix double-quoted strings? No. The Nix reference manual documents only a small set of escape sequences (e.g., \n, \r, \t, plus escaping for ", , and the interpolation-trigger sequence ${). It does not list \u{...} as a supported escape sequence, so \u{...} is not valid escape syntax according to the documented grammar.
Citations:
- 1: https://nix.dev/manual/nix/2.25/language/string-interpolation.html
- 2: https://nixos.org/manual/nix/2.34/language/string-literals
- 3: https://releases.nixos.org/nix/nix-2.24.5/manual/language/syntax.html
- 4: https://nix.dev/manual/nix/2.26/language/string-literals
- 5: https://nix.dev/manual/nix/2.18/language/values
🏁 Script executed:
# Find test files for this crate
find crates/kcore-disk-layout-yaml -name "*.rs" -type f | head -20Repository: rtacconi/kcore
Length of output: 99
🏁 Script executed:
# Search for tests of nix_string function
rg "nix_string" crates/kcore-disk-layout-yaml --type rs -B 2 -A 5Repository: rtacconi/kcore
Length of output: 85
🏁 Script executed:
# Check if there are tests in the file itself or nearby
rg "#\[test\]|#\[cfg\(test\)\]" crates/kcore-disk-layout-yaml --type rs -B 2 -A 10Repository: rtacconi/kcore
Length of output: 85
🏁 Script executed:
# Search for nix_string function and tests
rg "nix_string" crates/kcore-disk-layout-yamlRepository: rtacconi/kcore
Length of output: 772
🏁 Script executed:
# Look for test modules
rg "#\[test\]|#\[cfg.*test" crates/kcore-disk-layout-yaml -A 5Repository: rtacconi/kcore
Length of output: 2350
🏁 Script executed:
# Check the actual context around the nix_string function
sed -n '240,270p' crates/kcore-disk-layout-yaml/src/lib.rsRepository: rtacconi/kcore
Length of output: 856
🏁 Script executed:
# Check if there are integration tests
find crates/kcore-disk-layout-yaml/tests -type f 2>/dev/nullRepository: rtacconi/kcore
Length of output: 40
🏁 Script executed:
# Search for any direct tests of nix_string function
rg "nix_string\(" crates/kcore-disk-layout-yaml -B 3 -A 3Repository: rtacconi/kcore
Length of output: 2938
🏁 Script executed:
# Check for any tests that might be testing special characters
rg "assert.*contains\|assert_eq" crates/kcore-disk-layout-yaml/src/lib.rs | head -20Repository: rtacconi/kcore
Length of output: 40
🏁 Script executed:
# Get more context on the test cases to see if they cover edge cases
sed -n '280,400p' crates/kcore-disk-layout-yaml/src/lib.rsRepository: rtacconi/kcore
Length of output: 1901
🏁 Script executed:
# Let's run cargo clippy on this crate to check for warnings
cd crates/kcore-disk-layout-yaml && cargo clippy --all-targets 2>&1 | head -100Repository: rtacconi/kcore
Length of output: 305
🏁 Script executed:
# Also check cargo fmt
cd crates/kcore-disk-layout-yaml && cargo fmt --check 2>&1Repository: rtacconi/kcore
Length of output: 305
🏁 Script executed:
# Let's verify the test actually runs
cd crates/kcore-disk-layout-yaml && cargo test --lib 2>&1 | tail -50Repository: rtacconi/kcore
Length of output: 305
Escape ${...} sequences and fix non-ASCII handling in Nix strings.
The nix_string function is used to emit user-controlled values (device paths, mountpoints, etc.) into generated Nix code. Two bugs:
-
${...}sequences are passed through unchanged. In Nix double-quoted strings,${...}triggers interpolation, allowing manifest data to inject code instead of being treated as a literal string. - Non-ASCII characters are emitted as
\u{...}, which is not valid Nix escape syntax and breaks output for any non-ASCII input.
Per the Nix language manual, the only way to escape the ${ sequence is \${, and non-ASCII characters should be directly encoded in UTF-8 (no special escaping needed).
Proposed fix
fn nix_string(s: &str) -> String {
let mut out = String::with_capacity(s.len() + 2);
out.push('"');
- for ch in s.chars() {
+ let mut chars = s.chars().peekable();
+ while let Some(ch) = chars.next() {
match ch {
'\\' => out.push_str("\\\\"),
'"' => out.push_str("\\\""),
'\n' => out.push_str("\\n"),
'\r' => out.push_str("\\r"),
'\t' => out.push_str("\\t"),
- c if c.is_ascii() => out.push(c),
- c => out.push_str(&format!("\\u{{{:x}}}", c as u32)),
+ '$' if matches!(chars.peek(), Some('{')) => out.push_str("\\$"),
+ c => out.push(c),
}
}
out.push('"');
out
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// Escape a string for use inside Nix double quotes. | |
| fn nix_string(s: &str) -> String { | |
| let mut out = String::with_capacity(s.len() + 2); | |
| out.push('"'); | |
| for ch in s.chars() { | |
| match ch { | |
| '\\' => out.push_str("\\\\"), | |
| '"' => out.push_str("\\\""), | |
| '\n' => out.push_str("\\n"), | |
| '\r' => out.push_str("\\r"), | |
| '\t' => out.push_str("\\t"), | |
| c if c.is_ascii() => out.push(c), | |
| c => out.push_str(&format!("\\u{{{:x}}}", c as u32)), | |
| } | |
| } | |
| out.push('"'); | |
| out | |
| } | |
| /// Escape a string for use inside Nix double quotes. | |
| fn nix_string(s: &str) -> String { | |
| let mut out = String::with_capacity(s.len() + 2); | |
| out.push('"'); | |
| let mut chars = s.chars().peekable(); | |
| while let Some(ch) = chars.next() { | |
| match ch { | |
| '\\' => out.push_str("\\\\"), | |
| '"' => out.push_str("\\\""), | |
| '\n' => out.push_str("\\n"), | |
| '\r' => out.push_str("\\r"), | |
| '\t' => out.push_str("\\t"), | |
| '$' if matches!(chars.peek(), Some('{')) => out.push_str("\\$"), | |
| c => out.push(c), | |
| } | |
| } | |
| out.push('"'); | |
| out | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/kcore-disk-layout-yaml/src/lib.rs` around lines 245 - 262, In
nix_string, prevent Nix interpolation and fix non-ASCII escapes: when iterating
chars in fn nix_string(s: &str), detect the "${" sequence and emit "\${" instead
of leaving "${" untouched (e.g., if current char is '$' and next is '{' output
"\\${" and advance), and stop emitting Rust-style \u{...} for non-ASCII
chars—append non-ASCII characters directly (they will be valid UTF-8 in the
output) rather than formatting them as \u{...}; retain existing escapes for
backslash, double-quote, newline, carriage return, and tab, and keep the
surrounding double quotes. Ensure you update the loop logic in nix_string to
handle the multi-char "${" case and to treat non-ASCII by pushing the char
itself.
Comment on lines
+71
to
+73
| This screen is not a recovery shell. If the TUI shows `API: unavailable`, keep | ||
| using SSH or out-of-band management for troubleshooting; the console will still | ||
| render local NIC and disk inventory where Linux can provide it. |
There was a problem hiding this comment.
Match the documented API status text to the actual UI.
The renderer shows API endpoint: … Status: available|unavailable in crates/kcore-console/src/ui/render.rs lines 154-165, not API: unavailable / API: available. As written, the guide tells operators to look for a literal string that never appears on screen.
Also applies to: 120-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/appliance-console.md` around lines 71 - 73, Update the docs to match the
actual UI text rendered by the console: replace occurrences like "API:
unavailable" / "API: available" with the exact string the renderer prints ("API
endpoint: … Status: available" or "API endpoint: … Status: unavailable") so the
guide points operators to the correct on-screen status; update both the
paragraph around lines 71–73 and the similar text around lines 120–121 to
reference the renderer's "API endpoint: … Status: available|unavailable" output
(as produced by crates/kcore-console/src/ui/render.rs).
| Documentation=https://kcore.ai/docs | ||
| After=network-online.target local-fs.target | ||
| Wants=network-online.target | ||
| Conflicts=getty@tty1.service |
There was a problem hiding this comment.
Add autovt@tty1.service to Conflicts.
The NixOS module in modules/kcore-branding.nix conflicts with both getty@tty1.service and autovt@tty1.service, but this reference unit only conflicts with getty@tty1.service. On systems where autovt@ is enabled, this could lead to a race condition where both services try to control tty1.
🔧 Proposed fix
-Conflicts=getty@tty1.service
+Conflicts=getty@tty1.service autovt@tty1.service📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Conflicts=getty@tty1.service | |
| Conflicts=getty@tty1.service autovt@tty1.service |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packaging/systemd/kcore-console.service` at line 16, The unit's Conflicts
setting only lists getty@tty1.service which can allow autovt@tty1.service to
race for tty1; update the Conflicts entry in the kcore-console.service unit (the
Conflicts line) to include autovt@tty1.service as well (comma-separate or add a
space-separated token depending on existing formatting) so the unit conflicts
with both getty@tty1.service and autovt@tty1.service.
Collapse q/Q match arm to satisfy clippy::collapsible_match under -D warnings. Use accent #6366f1, version in title, and reserved ASCII logo panel.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
Release Notes
New Features
diskLayoutschema in kctl, enabling declarative storage configuration without writing raw Nix.Documentation