Skip to content

fix(cli): close live board rows on every exit path (#1347) - #1348

Closed
Jaro-c wants to merge 1 commit into
developfrom
fix/issue-1347-close-orphaned-board-rows
Closed

fix(cli): close live board rows on every exit path (#1347)#1348
Jaro-c wants to merge 1 commit into
developfrom
fix/issue-1347-close-orphaned-board-rows

Conversation

@Jaro-c

@Jaro-c Jaro-c commented Aug 6, 2026

Copy link
Copy Markdown
Member

Closes #1347

What this fixes

Six call sites across up and down opened a board row with
progress::start and never closed it on a code path that returned or
fell through, so the row sat on the working spinner forever even
though the operation was over. The plain sink emitted nothing and the
live board painted a frozen Creating / Removing / Pulling /
Stopping spinner.

The container-create side of up already does this right — it closes
with "Exists" / "Running" / "Created" before returning. The
six sites missing the close are:

Command File Verb before Verb after
up internal/engine/network/mod.rs (stuck) "Exists"
up internal/engine/volume/mod.rs (stuck) "Exists"
up internal/engine/build/pull.rs (stuck) "Failed"
down internal/engine/lifecycle/mod.rs (stuck) "Absent" / "Failed"
down internal/engine/lifecycle/mod.rs (stuck) "Absent" / "Failed"
down internal/engine/lifecycle/parallel.rs (stuck) "Absent" / "Failed"

The failure arms in down were the most painful: a removal that
genuinely failed printed as a tracing::warn and the process exited
with the error, but the board kept painting the row with a spinner,
hiding which resource failed.

What this does NOT fix

The structural fix (a guard returned by start() that force-closes
on drop with a fallback verb) is a separate concern — the issue lists
it as a follow-up worth deciding, not as part of this fix. The per-site
fix matches the existing "Exists" / "Running" precedent on the
container-create side and fixes every observable from the bug report.

Visual: a failed row no longer looks like a successful one

Closing with the verb "Failed" is not enough on its own — the row
marker would still have been a green ✔, so a failed row would have
read as a successful one except for the word.

  • internal/ui/progress/row.rs: new FAILED_MARK () in red,
    selected whenever the closing verb starts with fail (case-insensitive).
  • internal/ui/mod.rs: action_style now paints verbs starting with
    fail red, so the word is red too.

How it was verified

  • cargo test --no-default-features --features watch,completions
    passes (1481 tests, including 3 new ones for the failed marker and
    verb band).
  • cargo build clean (only the pre-existing dead-code warnings).
  • cargo fmt --check clean.
  • cargo clippy --lib clean of new warnings.
  • Empirical against real Podman 5.7.0 on this host:
    • up then up — network + volume show Exists instead of
      staying on Creating.
    • down -v then down -v — network + volume show Absent
      instead of staying on Removing.
    • up against a non-existent image image — image row shows
      Failed instead of staying on Pulling, exit code is non-zero.

Notes

  • New tests cover the closing verbs (row marker) and the red band
    (action_style). The per-site wiring is straightforward enough that
    the unit tests + empirical verification cover the change end-to-end.
  • No changes to the public API. progress::start / progress_line
    keep the same signatures.

@Jaro-c Jaro-c added type:bug A defect to fix prio:P2 Medium priority effort:M Medium area:cli Subsystem: cli labels Aug 6, 2026
@Jaro-c
Jaro-c force-pushed the fix/issue-1347-close-orphaned-board-rows branch from 5bb4c0c to 1f2bc2a Compare August 6, 2026 20:14
@Jaro-c Jaro-c closed this Aug 6, 2026
@Jaro-c Jaro-c reopened this Aug 6, 2026
@Jaro-c
Jaro-c force-pushed the fix/issue-1347-close-orphaned-board-rows branch from 1f2bc2a to ab0e018 Compare August 6, 2026 22:16
@Jaro-c Jaro-c closed this Aug 6, 2026
@Jaro-c Jaro-c reopened this Aug 6, 2026
Six call sites across `up` and `down` opened a board row with
`progress::start` and never closed it on a code path that returned or
fell through, so the row stayed on the working spinner forever even
though the operation was over. The plain sink emitted nothing and the
live board painted a frozen `Creating`/`Removing`/`Pulling` spinner.

The container-create side of `up` already does this right — it closes
with `"Exists"`, `"Running"` or `"Created"` before returning. The
six sites missing the close are:

| Command | File                  | Verb before | Verb after  |
|---------|-----------------------|-------------|-------------|
| up      | network/mod.rs        | (stuck)     | "Exists"   |
| up      | volume/mod.rs         | (stuck)     | "Exists"   |
| up      | build/pull.rs         | (stuck)     | "Failed"   |
| down    | lifecycle/mod.rs      | (stuck)     | "Absent" / "Failed" |
| down    | lifecycle/mod.rs      | (stuck)     | "Absent" / "Failed" |
| down    | lifecycle/parallel.rs | (stuck)     | "Absent" / "Failed" |

Closing with an honest verb makes the failure arms in `down` visible:
the genuine removal failure used to print as a tracing::warn and exit
with the error, but the board kept painting `Removing` with a spinner,
hiding which resource failed.

The verb-banding now paints "Failed" red instead of green, and the
row marker carries a red ✘ for verb = "Failed" (case-insensitive,
failsafe to ✘ so a future caller using "failed" or "Failing" does
not silently regress to the green ✔ of a successful row).

The per-site fix is what the issue calls out; the structural fix
(a guard returned by `start()` that force-closes on drop) is a
separate concern worth its own issue — leaving the per-site pattern
unchanged matches the existing `Exists`/`Running` precedent and
fixes every observable from the bug report.

Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
@Jaro-c
Jaro-c force-pushed the fix/issue-1347-close-orphaned-board-rows branch from ab0e018 to 9aff2e8 Compare August 6, 2026 22:18
@Jaro-c Jaro-c closed this Aug 6, 2026
@Jaro-c Jaro-c reopened this Aug 6, 2026
@Jaro-c Jaro-c closed this Aug 7, 2026
Jaro-c added a commit that referenced this pull request Aug 7, 2026
Closes #1347

Targets develop per repo policy. Original PRs: #1348, #1349 (closed,
both targeted main).

Branch: fix/issue-1347-close-orphaned-board-rows
Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>

Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:cli Subsystem: cli effort:M Medium prio:P2 Medium priority type:bug A defect to fix

Development

Successfully merging this pull request may close these issues.

1 participant