feat(minimald): refactor progress, display session setup on first attach by twitchyliquid64 · Pull Request #591 · gominimal/minimal

twitchyliquid64 · 2026-06-29T01:07:04Z

This will avoid the apparent hang when you first launch a session with packages that need to be downloaded or built.

Summary by CodeRabbit

New Features
- Added live progress rendering for SSH/channel-based operations while tasks run, including during host startup.
- Introduced a scoped rendering flow that keeps progress output smooth and supports multiple active viewers.
Bug Fixes
- Reduced flicker and half-painted updates during live rendering.
- Improved session/host attach error reporting when startup completes before attachment is possible.
Other
- Progress output is now properly cleared after rendering completes.

coderabbitai · 2026-06-29T01:07:17Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 19e62e4e-5e53-43a5-be51-d7471b47b248

📥 Commits

Reviewing files that changed from the base of the PR and between 3dcd0b0 and 95fdc32.

📒 Files selected for processing (7)

crates/minimald/src/channel_progress.rs
crates/minimald/src/lib.rs
crates/minimald/src/session.rs
crates/ot/Cargo.toml
crates/ot/src/indicatif_shim.rs
crates/ot/src/lib.rs
docs/specs/04-spec-ot-render-decoupling/04-spec-ot-render-decoupling.md

✅ Files skipped from review due to trivial changes (1)

docs/specs/04-spec-ot-render-decoupling/04-spec-ot-render-decoupling.md

🚧 Files skipped from review as they are similar to previous changes (6)

crates/minimald/src/lib.rs
crates/ot/Cargo.toml
crates/minimald/src/session.rs
crates/minimald/src/channel_progress.rs
crates/ot/src/lib.rs
crates/ot/src/indicatif_shim.rs

📝 Walkthrough

Walkthrough

Replaces OpTracker change signaling with tokio::sync::watch, adds render_operations_while plus ChannelTerm frame pumping, introduces ChannelProgress in minimald, and rewires session host launch to render progress on the SSH channel before attaching.

Changes

Scoped SSH channel progress rendering

Layer / File(s)	Summary
OpTracker: Notify → watch::Sender change API `crates/ot/src/lib.rs`, `crates/ot/Cargo.toml`	`RootShared` replaces `AtomicU64 + Notify` with `watch::Sender<u64> + AtomicU64`; `OpTracker::changed()` is removed and replaced with `subscribe()` returning a `watch::Receiver<u64>`; `version()` reads from the watch sender; tests updated to use `rx.changed().await`.
ChannelTerm, pump_frames, and render_operations_while `crates/ot/src/indicatif_shim.rs`	Adds `ChannelTerm` (`TermLike` impl buffering ANSI frames with synchronized-update escape sequences), `pump_frames` (async drain loop writing frames to `AsyncWrite`), and `render_operations_while` (public async API composing these); updates `IndicatifShim::run` and `render_while` to drive repaints via `rx.changed()`. Tests added for both `render_operations_while` and `ChannelTerm`.
ChannelProgress wrapper in minimald `crates/minimald/src/channel_progress.rs`, `crates/minimald/src/lib.rs`	New `ChannelProgress` struct holding `Channel<Msg>`, `OpTracker`, and terminal size; `run()` creates a channel writer and calls `ot::render_operations_while`, returning the channel and future output.
Session tracker and launch wiring `crates/minimald/src/session.rs`	`Session` gains a `tracker: OpTracker` field initialized in `run()`; `context()` injects it via `with_operation_tracker()`; `mint_session_host` now spawns the host without an attached channel, renders launch progress via `ChannelProgress`, then attaches the channel post-render.
Spec update `docs/specs/04-spec-ot-render-decoupling/...`	Spec is revised to document watch-based notification, `subscribe()` API, `render_operations_while` primitive, `ChannelProgress` wrapper, and updated migration step 4.

Sequence Diagram(s)

sequenceDiagram
  participant SSHClient
  participant Session
  participant ChannelProgress
  participant render_operations_while
  participant ChannelTerm
  participant pump_frames
  participant AsyncWrite

  SSHClient->>Session: new attachment request
  Session->>ChannelProgress: new(channel, tracker, size)
  ChannelProgress->>render_operations_while: root, channel_writer, size, spawn_host_fut
  render_operations_while->>ChannelTerm: construct TermLike with mpsc tx
  render_operations_while->>pump_frames: spawn(rx, channel_writer as AsyncWrite)
  ChannelTerm-->>pump_frames: flush() sends synchronized-update ANSI frame
  pump_frames-->>AsyncWrite: write+flush bytes to SSH channel
  render_operations_while-->>ChannelProgress: host spawn result
  ChannelProgress-->>Session: (channel, spawn_result)
  Session->>Session: attach channel to running host
  Session-->>SSHClient: host attached

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

gominimal/minimal#515: Earlier ot refactor that introduced the initial OpTracker tracking/notification API and indicatif_shim structure that this PR evolves.

Suggested reviewers

0chroma
evanspearman
norrietaylor

Poem

🐇 I hop through frames with watchful glee,
ANSI sparkles drift to the tee,
The channel hums, the spinner sings,
Progress wraps on silken strings,
Then back to host the bytes all flee—
A tidy little victory!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main changes: refactoring progress handling and showing session setup on first attach.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/minimald/src/session.rs`:
- Around line 243-261: The launcher is being created before
ChannelProgress::run, so the context/setup work done by
session_launcher(session_hnd) happens outside the rendered progress scope. Move
the session_launcher call inside the future passed to ChannelProgress::run in
session.rs so the launcher is built under the same rendering channel before
session_host::Host::spawn starts. This should keep the session_launcher,
context, and Host::spawn flow within the progress-rendered operation tree.

In `@crates/ot/Cargo.toml`:
- Line 14: The indicatif feature in Cargo.toml is missing the Tokio runtime
needed by indicatif_shim::tests using #[tokio::test(flavor = "multi_thread")].
Update the indicatif feature definition to include tokio/rt-multi-thread
alongside the existing tokio dependencies so the multi-thread test runtime is
enabled.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 419495aa-5fe7-475c-a4b3-645c6245b446

📥 Commits

Reviewing files that changed from the base of the PR and between 98b7ae9 and 3dcd0b0.

📒 Files selected for processing (7)

crates/minimald/src/channel_progress.rs
crates/minimald/src/lib.rs
crates/minimald/src/session.rs
crates/ot/Cargo.toml
crates/ot/src/indicatif_shim.rs
crates/ot/src/lib.rs
docs/specs/04-spec-ot-render-decoupling/04-spec-ot-render-decoupling.md

twitchyliquid64 requested review from 0chroma, evanspearman and norrietaylor June 29, 2026 01:07

coderabbitai Bot reviewed Jun 29, 2026

View reviewed changes

Comment thread crates/minimald/src/session.rs

Comment thread crates/ot/Cargo.toml

feat(minimald): refactor progress, display session setup on first attach

95fdc32

twitchyliquid64 force-pushed the tom/session-destroy branch from 3dcd0b0 to 95fdc32 Compare June 29, 2026 02:15

twitchyliquid64 enabled auto-merge (squash) June 29, 2026 02:26

evanspearman approved these changes Jun 29, 2026

View reviewed changes

twitchyliquid64 merged commit 75a42d9 into main Jun 29, 2026
21 checks passed

twitchyliquid64 deleted the tom/session-destroy branch June 29, 2026 15:53

gominimal-aw-bot Bot mentioned this pull request Aug 12, 2026

fix: render download progress during min add #1213

Draft

Navigation Menu

Uh oh!

feat(minimald): refactor progress, display session setup on first attach - #591

feat(minimald): refactor progress, display session setup on first attach#591
twitchyliquid64 merged 1 commit into
mainfrom
tom/session-destroy

twitchyliquid64 commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

twitchyliquid64 commented Jun 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

twitchyliquid64 commented Jun 29, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 29, 2026 •

edited

Loading