Repository navigation
os/v3/puppy/console-tui: the header says which system is running - LIVE or INSTALLED (closes #1162) - #1163
Conversation
|
pi-prs: #1163 ready for review at Short version: the header now carries the one fact of the brief's header spec it never had — LIVE or INSTALLED — read from what The five cells are paid for rather than clipped: 58/58, clippy Not in this PR, deliberately: item 5's label on an installed box (still says "Install 1bit OS to a disk", as the dialog menu does; the guard refuses the boot disk by name, so it is safe but not the brief's installed-facts screen) — that needs its own review. |
|
Fix commit The first version asked the Found by driving item 5's new installed branch in QEMU, where the marker can be flipped at runtime (a boot cannot produce the inconsistency, which is why the original ordering passed everything here — that is the honest reason it survived review-read).
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
$ cd os/v3/puppy/console-tui && cargo test --offline
test result: ok. 58 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.20s
with three new tests, and they are the three that matter: the_mode_is_what_init_wrote, the_header_says_which_system_is_running, and the_header_drops_a_fact_rather_than_letting_it_be_clipped.
The probe is init's rule, not a second derivation. facts.rs:60 quotes the same expression init.sh uses (grep -q " /data " /proc/mounts && [ -e /data/.installed ]), and mode_in() checks that first (:95), then /etc/1bit/installed (:98), then /etc/1bit/live (:101) - so an installed /data outranks a stale live marker, which is the head commit and the same correction the console menu makes in #1168. Two screens, one rule.
The snapshot change is a deliberate drop, and the test says so. At 80x25 the header line becomes
1bit OS LIVE rc-2026-11-03-c16 strix 192.168.50.110 up 2h 13m
i.e. Secure Boot enabled leaves that line to make room for LIVE; at 240x67 the frame still carries it (grep -c -> 1), so the fact is not lost, only moved out of a line that cannot hold it - which is exactly what the_header_drops_a_fact_rather_than_letting_it_be_clipped asserts. Dropping a security fact would have been the wrong trade; dropping it only where it does not fit, in favour of the fact the header exists for is the right one.
Unknown as a third state is the right shape too: a boot where neither marker was written (the case release-check.sh:648 warns about) now says "unknown" instead of guessing LIVE.
Not run: the console on a real installed box - that needs the appliance/QEMU. The mode precedence, the header text and the drop rule are all covered by the tests above, and the frame fixtures are what the appliance draws.
|
Third commit, New harness The This is also the first time this PR's header has been rendered against the appliance's own filesystem rather than a fixture: with only 59/59 tests (one for One trap for whoever repeats this, measured the hard way: |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY
Re-verdicted at 758da295. The new commit adds init's else branch, and it is the right reading of init's own line:
init.sh:211 grep -q " /data " /proc/mounts && [ -e /data/.installed ] && INSTALLED=1so a boot with /data mounted and no .installed is LIVE in init's words, not "unknown" - and resolve() now says so: Mode::Unknown if data_mounted => Mode::Live, leaving Unknown only for the case where the box genuinely cannot say (/data not mounted and no marker written).
cargo test --offline: 59 passed, 0 failed, including the new facts::tests::a_boot_with_data_mounted_and_no_marker_is_live. data_mounted() reads /proc/mounts and compares field 2 to /data, which is the same test init makes with grep -q " /data " (both match the mount point, not a substring of a path).
The precedence is now init's, in full: /data + .installed → Installed, then /etc/1bit/installed → Installed, then /etc/1bit/live → Live, then the else branch above. That is the third time this rule has needed a fix in one evening (#1168's menu, #1163's first commit, this one) which is worth knowing: the Rust console and the shell menu each re-derive it, and only one of them can be the authority.
Unchanged from my 1262658c review and still passing: 3 new tests were there, the 80x25 header drops Secure Boot enabled to make room for the mode and keeps it at 240x67, and the_header_drops_a_fact_rather_than_letting_it_be_clipped asserts exactly that.
…E or INSTALLED) The brief's header spec is "edition name, LIVE/INSTALLED, IP, Lemonade URL, API key hint", and the word that was missing is the first of them: the console never said whether this box is the stick (everything in RAM, nothing installed) or this machine's own disk. #711 is the owner's report of exactly that confusion - "live usb is loaded up" while the banner facts came from the NVMe - and after the install step the console would have looked identical to the stick it was installed from, which is the screen the demo films. `Mode` reads what `init.sh` already decided at boot instead of re-deriving it: `/etc/1bit/installed` or `/etc/1bit/live` (the markers init touches beside its MODE banner), falling back to init's own rule (`/data/.installed`) when neither marker is there. `Mode::Unknown` renders nothing at all - a console started by hand on a dev box invents no mode. The header gained five cells, so `header(f, width)` now drops the two facts that may give way - the temperature first, then Secure Boot - instead of letting ratatui clip the line at the edge ("SoC 4", "Secure Boot ena"). The mode, the release, the host and the address never go, and neither do the blocks that may not be trimmed (the nine actions, the footer). The appliance's own tty1 is 240x67, where everything still fits; at 80x25 the header reads `1bit OS LIVE rc-2026-11-03-c16 strix 192.168.50.110 up 2h 13m`. Both committed snapshot fixtures are re-rendered for the new header (they are byte-compared, so the layout change is deliberate and frozen); every gate string survives - `the_c18_gate_row_finds_its_strings_on_this_screen` still passes, and `1bit-tty1 | head -10` still sees `Show the API key` / `Reboot / power off` on the rows it reads. `cargo test` 58/58 (3 new: the probe against fake /etc/1bit roots, both words on the frame, and the header's drop order swept over 65..=256 columns), clippy --all-targets -D warnings clean, the two fmt hunks that remain are the ones already red on main, copyright --check rc 0. Static rebuild: 54c6849c96333869, 1,587,416 B. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…an installed /data outranks a stale live marker) The first version asked the markers first and the data partition second. init.sh decides the other way round - its one rule is `grep -q " /data " /proc/mounts && [ -e /data/.installed ]`, and the `/etc/1bit` markers are written FROM that decision - so a stale `live` marker outranked a real installed data partition and the console said LIVE on an installed box. Found by driving the installed case in QEMU (where the marker can be flipped at runtime; a boot cannot produce the inconsistency, which is why the first ordering passed everything here). `mode_in` now tests `/data/.installed`, then `/etc/1bit/installed`, then `/etc/1bit/live`, and the test covers the stale-live case explicitly. 58/58, clippy -D warnings clean, the two fmt hunks that remain are the ones already red on main. Static rebuild: c67bdc3f3755a5ad, 1,587,408 B. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…a box with no marker still says LIVE
init.sh's rule is one line - INSTALLED needs /data mounted AND .installed on it, everything else is LIVE - and the probe
implemented the first half only. Measured with the marker matrix in QEMU (all seven combinations of the two /etc/1bit
markers and /data/.installed, /data mounted throughout): in the `(none)` row the menu said live and the header said
nothing. Silent is not wrong, but it is not what init's banner says on the same boot.
`mode()` now resolves through `resolve(seen, data_mounted)`: Unknown + /data mounted -> Live (init's own else branch),
and Unknown without /data stays Unknown, because a boot whose data partition did not mount cannot say whether it is
installed and must not invent an answer.
The matrix (one boot, two binaries, both halves compared):
(none) live | LIVE
installed installed | INSTALLED
live live | LIVE
data/.installed installed | INSTALLED
installed,live installed | INSTALLED
installed,data/.installed installed | INSTALLED
live,data/.installed installed | INSTALLED
installed,live,data/.installed installed | INSTALLED
combinations that disagree: 0
59/59 tests (one new for `resolve`), clippy -D warnings clean, both committed snapshot fixtures byte-identical,
copyright rc 0. Static rebuild: 585b799e9d507a19, 1,591,528 B.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
758da29 to
c4cb3aa
Compare
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Re-verdict at c4cb3aae: READY, and the PR's own change is the one I reviewed at 758da295 — the push was a merge of main (which by then carried #1155's wrap_hanging), not new work.
Checked by comparing the two versions of this PR's own patch rather than eyeballing the head:
old merge-base e670036 -> 758da295 (416-line patch)
new merge-base fa757d1 -> c4cb3aae (412-line patch)
The only differences are context: hunk offsets, blob hashes, and one use crate::facts::{… wrap_hanging …} import line that main now has. Every added and removed line of the change itself — the Mode enum, the header's LIVE/INSTALLED line, the two snapshot updates — is byte-identical.
So the verdict carries: the console header says which system is running, cargo test 59/59 at that head, and the third fix to the same "is this installed?" rule (the Rust console and the shell menu now both derive it, which is the useful part of this PR).
BLOCKED at this head is the build leg re-running, not the diff.
|
Re-verdict request at Both PRs added the same test helper to fn span_text(line: &Line<'static>) -> String {
line.spans.iter().map(|s| s.content.as_ref()).collect()
}So the rebase resolution is: keep use crate::facts::{human_bytes, human_uptime, wrap_hanging, wrap_words, Facts, Mode, SecureBoot};
// ^^^^^^^^^^^^^ from #1155 ^^^^ from this PRThe three commits are unchanged in content otherwise; the rebase was rewritten, so the branch was force-pushed with Measured on the rebased head:
For the record, the merge order is now free in both directions because the duplicate is gone from this branch (the reverse order can no longer reintroduce it: |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at c4cb3aae — this PR's own patch is unchanged from 758da295 (only hunk offsets and #1155's import moved); detail in the re-verdict above.
|
Preparing review... |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 89ae9f57 — re-derived both claims at this head, and the rest of the PR is what I reviewed.
cargo test62/62, 0 failed (the body's number): run in a cleangit worktreeat89ae9f57; the thermal guard paused the scope twice at 79 C and resumed, the suite finished62 passed; 0 failed.- The duplicate
span_texthelper is gone:grep -rn "fn span_text" src/-> exactly one definition (src/ui.rs:1250), 5 call sites, none infacts.rs. - Stacked, and that is fine in this order: the branch contains
babcf55= #1160's commit, so its diff is 7 files today and shrinks to its own 4 (the mode probe and the two snapshots) once #1160 lands. Merge #1160 first, then this. - The mode rule itself is unchanged from my review: the probe obeys init's own rule (an installed
/dataoutranks a stale live marker) and a box with no marker takes init's else branch and still says LIVE.
…tead of offering an install (closes #1167) (#1168) * os/v3 puppy: the console's Install entry shows the installed disk instead of offering an install (brief item 5's installed branch) The brief's item 5 is "Install - only when NOT installed: the target disk, size, model, the warning, a typed confirmation ... When installed: show the installed-disk facts instead." The first half was built and driven; the installed half was never built, so on an installed box the console offered "Install 1bit OS to a disk", the person answered yes, and the guard then refused the disk this system booted from - which reads as a broken install rather than as "this is already installed", on the one screen a person looks at after installing. `item_install` now branches on the same decision init.sh makes (`installed_mode`: /data mounted with .installed on it, then init's markers), and on an installed box it shows 1bit OS is INSTALLED on /dev/nvme0n1. 490.5G INTEL SSDPEKNW512G8 data: 401.2G, 12.9G free The install is permanent and nothing on this screen erases it. Reboot to restart the installed system; remove the stick first. To install onto a DIFFERENT disk as well, answer yes next: /dev/nvme0n1 is this system's own disk, and the installer refuses it by name. and only then asks the yes/no for a *different* disk, so installing onto a second disk still works. `boot_disk` asks exactly what 1bit-console-install asks before it refuses a target, so the screen cannot name a different disk from the one the installer would refuse. NONINT (`--run 5`, `--selftest`) still prints `install-guard-checked` - vm-check's `U2 menu actions` row greps it - and now says which branch answered. Measured in QEMU, one boot of the c26 image, the menu script swapped in, all four cases: A live, --run 5 -> the disk list with "this boot (refused)" + install-guard-checked B installed, --run 5 -> "installed: /dev/sda (data 7.8G, 6.2G free)" + install-guard-checked C installed, --action 5 -> the facts box above (read off the drawn pty), and /tmp/1bit-install.log never created D live, --action 5 -> the old "Install 1bit OS to a disk?" question, unchanged --selftest -> "--- item 5 / install-guard-checked", selftest-rc=0 Found while writing this: the first version of both this and #1163's probe asked the /etc/1bit markers before the /data test, so a stale `live` marker outranked a real installed data partition. Both now obey init's own rule first. Not changed here, recorded: with stdin closed (a terminal that disappeared) dialog answers its default button, which is Yes - so the install continues to the installer's own device prompt. Pre-existing for every destructive confirm in this menu, and the installer still refuses the boot disk and needs the name twice; a No default is a UX change for the shared dialog helper, not this item. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * os/v3 puppy: the installed-branch wording parses in bash too The two dialog strings interpolated a default word that carried an apostrophe (${d:-this machine's disk}, ${d:-This system's own disk}). busybox ash and dash read the apostrophe as literal text inside a double-quoted ${...}, but bash 1.52 treats it as an opening quote and refuses the whole file: bash -n 1bit-console-menu: line 680: syntax error near unexpected token `(' Measured on this branch before the fix: bash -n failed, busybox sh -n and dash -n passed; on main's own copy all three pass. Nothing in the image invokes the file through bash (every call site - vm-check's U2/U3 rows, 1bit-console - runs it as a command, so the /bin/sh shebang picks ash), so this is a portability landmine rather than a live break: one bash -n, one bash -c, or one person reading the file with bash hits it. The default words now carry no quote character inside ${...}: ${d:-the boot disk} "1bit OS is INSTALLED on the boot disk." ${d:-The disk this system booted from} bash -n, busybox sh -n and dash -n are all clean after the change, and the installed branch still prints the same facts in the same order. Also swept the tree for the class, not the instance: grep -rnP over os/ and tools/ for an apostrophe or quote inside ${...} finds nothing else. --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: agent <agent@local>
…rded label cannot lie (#1214) The nine action labels exist twice: `ACTIONS` in src/ui.rs and the dialog's `--menu ... 1 "Show the API key" ... 9 "Reboot / power off"` table in embed/usr/local/bin/1bit-console-menu. Nothing held them together. The hand-off runs `1bit-console-menu --action N`, so row N's label is a promise about what key N does: type 5 and the dialog's item 5 runs, whatever this screen calls it. #935 and #1163 both reworded a label and each time the gate (`vm-check.sh` `U2 menu`, reading tty1) was the only thing that objected; a drift that changes one file and not the other would ship a row that says one thing and does another. One test, no behaviour change: parse the item table from the shipped script (anchored on the main dialog's own `1bit OS console` title, because the script has three other --menu calls - network, models, reboot - whose rows are a different thing) and require all nine to equal ACTIONS, in order. Red/green, both directions, with `cargo test every_menu_row`: ACTIONS[0] "Show the API key" -> "Show the API key now" FAILED (parity broken) script item 5 "Install 1bit OS to a disk" -> "...on a disk" FAILED (parity broken) both restored ok, 64/64 `cargo test --offline` 64/64 (63 + this), clippy -D warnings clean, fmt clean, tools/copyright.py --check rc 0. Co-authored-by: agent <agent@local>
Closes #1162.
Why
The brief's header spec is "edition name, LIVE/INSTALLED, IP, Lemonade URL, API key hint". The word that was
missing is the first fact of them: the shipped console had no notion of it at all, so the appliance's tty1 looked
identical running from the stick (everything in RAM) and running from the machine's own disk.
init.shdecides itat boot and prints it once, but that banner scrolls away and the TUI is the screen a person looks at all day - so
after the owner's install step the console still read like the stick, on camera, and an ssh user never learned
whether the box is installed. Closed #711 was the boot-side half of this; this is the console's half.
The fix (
src/facts.rs,src/ui.rs)Mode { Live, Installed, Unknown }reads whatinit.shalready decided instead of re-deriving it:/etc/1bit/installedMode::Installed)/etc/1bit/liveMode::Live)/data/.installedMode::Unknown- renders no word at all; a console started by hand on a dev box invents nothingHeader:
1bit OS+ the mode (LIVEgreen,INSTALLEDcyan) + the release, then host / address / uptime.The five cells are paid for, not clipped.
header(f, width)now drops the two facts that may give way - thetemperature first, then
Secure Boot …- instead of letting ratatui cut the line at the edge (SoC 4,Secure Boot ena). The mode, the release, the host and the address never go; the blocks that may not be trimmed(the nine actions, the footer) are not this function's.
Measured (
--fixture, this box):1bit OS LIVE rc-2026-11-03-c16 strix 192.168.50.110 up 2h 13m Secure Boot enabled SoC 43C1bit OS LIVE rc-2026-11-03-c16 strix 192.168.50.110 up 2h 13m(temp then Secure Boot give way)Gate safety
renders_at_the_two_console_sizes, so the change is deliberate and frozen).the_c18_gate_row_finds_its_strings_on_this_screenstill passes:CONNECT,web UI http://,api http://,ssh root@,Lemonade Mobile,Show the API key,Reboot / power offall survive at both sizes.vm-check.sh'sU2 tty1/U2 menu/U2 menu connectrows read lines(
1bit-tty1 1 | head -10/head -30) and grep the CONNECT/menu strings; Secure Boot is checked throughefivarfs/mokutil/efibootmgr, not off the screen (grep -n 'Secure Boot\|SoC ' vm-check.shfinds onlycomments and the efivarfs rows).
Checks
cargo test58/58 (3 new: the probe against fake/etc/1bitroots - marker wins, fallback rule, empty box;both words on a rendered frame plus Unknown inventing none; the drop order swept over 65..=256 columns, requiring
the mode/release/address at every width and everything at 240).
cargo clippy --all-targets -- -D warningsclean.cargo fmt --checkreports only the two hunks already red onmain(facts.rs, untouched by this branch'sformatting).
tools/copyright.py --checkrc 0. Static rebuild54c6849c96333869, 1,587,416 B.Deliberately not done
Install 1bit OS to a diskis still offered on an installed box, exactly asthe dialog menu offers it; the guard refuses the boot disk by name, so it is safe but it is not the brief's
"show the installed-disk facts instead". That is a second change (an installed-branch screen for item 5) and it
wants its own review, not a rider on this one.
vm-check.shrow (pi-release's file, andU2 menualready covers the menu's shape), no image build, noboard run, no device ticket - TUI-only, no GPU/NPU work,
:18097untouched.Co-Authored-By: Claude Fable 5.1 noreply@anthropic.com