Config screen's agents pane hides rows it cannot fit, silently - #178
Merged
Merged
Conversation
The pane drew what it could fit and dropped the rest, with a bottom border that looked the same whether it had shown every agent or two of six. Removing the fixed cap made that rarer but not impossible: a short terminal with several agents configured left the rest reachable only by resizing or running `voro agent list` at the shell. It now scrolls with `J`/`K` and the page keys — the cockpit focus card's gesture, taken for the same reason it has there: the pane holds no selection of its own, `j`/`k` on this screen belonging to the viewers list below it, and a second focus is a heavier thing to add than a scroll. The bottom border carries the offset and the keys only when something is hidden, so a pane showing everything still says nothing. Verified at 80x24 and 60x10 in a live TUI against a scratch config of eight agents: every agent reads out, and the border names the overflow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YareUYKHBQiHfthFE48KDg
MJohnson459
added a commit
that referenced
this pull request
Aug 20, 2026
Twenty-one test scratch roots across the workspace built their name from the process id and a SystemTime nanosecond stamp. That reads as though it could not repeat, and on one thread it does not, because each SystemTime::now() is ordered after the last. Across threads there is no such ordering and the clock does not advance a nanosecond at a time, so two threads reading inside one step read the same number and land on the same directory — the same database, the same git repo. Four of those sites are shared helpers with several callers and no tag to tell one call from another, so they could collide today rather than after some future copy-paste: worktree::repo (eight callers), cli::ctx_with_toml (six), app::app_with_agents (four) and app::pr_ready_app (three). Move them all onto tempfile, which creates the directory exclusively and retries under a different name on a clash, so distinctness is a property of the call rather than an argument about clock granularity. Fold in store::scratch_db too: its own counter kept concurrent callers apart but left the file exposed to reuse by a later run under a recycled pid, and leaving it in place would mean the workspace still had two mechanisms. keep() holds today's behaviour — the directories stay behind, as they always have, so a failing test's scratch state survives for inspection. Cleanup is deliberately not part of this change: TempDir deletes on drop, so the guard would have to outlive each test and every helper returning a PathBuf would have to return it, reshaping call sites the dispatch fixture alone has fifty-eight of. The dispatch fixture's counter and the test that measured its clock collisions go with it. That test named four thousand roots to show a clock-only name repeats; under tempfile every name is created, so re-running it would leave four thousand kept directories behind per run to prove a property that now holds by construction. CLAUDE.md gains the convention under its testing rules, so it sits where a contributor looks rather than inside one test module. The twenty-first site arrived in #178 while this change was being written, which is the copy-paste the convention exists to stop. tempfile is dev-only, so it reaches neither the shipped binary nor a consumer build, and it adds one leaf crate: every other transitive dependency was already in Cargo.lock. Verified: cargo clippy --workspace --all-targets -D warnings clean, and cargo test --workspace (895 tests) green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MJohnson459
added a commit
that referenced
this pull request
Aug 20, 2026
Twenty-one test scratch roots across the workspace built their name from the process id and a SystemTime nanosecond stamp. That reads as though it could not repeat, and on one thread it does not, because each SystemTime::now() is ordered after the last. Across threads there is no such ordering and the clock does not advance a nanosecond at a time, so two threads reading inside one step read the same number and land on the same directory — the same database, the same git repo. Four of those sites are shared helpers with several callers and no tag to tell one call from another, so they could collide today rather than after some future copy-paste: worktree::repo (eight callers), cli::ctx_with_toml (six), app::app_with_agents (four) and app::pr_ready_app (three). Move them all onto tempfile, which creates the directory exclusively and retries under a different name on a clash, so distinctness is a property of the call rather than an argument about clock granularity. Fold in store::scratch_db too: its own counter kept concurrent callers apart but left the file exposed to reuse by a later run under a recycled pid, and leaving it in place would mean the workspace still had two mechanisms. keep() holds today's behaviour — the directories stay behind, as they always have, so a failing test's scratch state survives for inspection. Cleanup is deliberately not part of this change: TempDir deletes on drop, so the guard would have to outlive each test and every helper returning a PathBuf would have to return it, reshaping call sites the dispatch fixture alone has fifty-eight of. The dispatch fixture's counter and the test that measured its clock collisions go with it. That test named four thousand roots to show a clock-only name repeats; under tempfile every name is created, so re-running it would leave four thousand kept directories behind per run to prove a property that now holds by construction. CLAUDE.md gains the convention under its testing rules, so it sits where a contributor looks rather than inside one test module. The twenty-first site arrived in #178 while this change was being written, which is the copy-paste the convention exists to stop. tempfile is dev-only, so it reaches neither the shipped binary nor a consumer build, and it adds one leaf crate: every other transitive dependency was already in Cargo.lock. Verified: cargo clippy --workspace --all-targets -D warnings clean, and cargo test --workspace (895 tests) green. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
The Config screen's agents pane now scrolls instead of silently dropping the rows it cannot fit.
What changed:
draw_configmeasures the agent lines against the pane's inner height, records the overflow onApp::config_agents_max_scroll, and renders the paragraph scrolled. When there is an overflow the block's bottom border carries<scroll>/<max> ↕ J/K PgDn/PgUp; when everything fits it says nothing, exactly as the cockpit focus card behaves.key_configbindsJ/KandPgUp/PgDnto that scroll, clamped against the last measured overflow.j/kstill belong to the viewers list, which is why the shifted pair was chosen over giving the pane a selection of its own — a second focus is a heavier thing to add to this screen than a scroll, andJ/Kalready mean "scroll the pane the selection cannot reach" on the cockpit.?key map's Config Navigation section lists both, andCASE_EXCEPTIONSgains (Config, "J") and (Config, "K") so the case-convention test still passes.J/Kto "the pane the screen's selection cannot reach" across both screens. CHANGELOG entry under Fixed.Why this option: of the three the task listed, a selection costs a second focus on a screen whose keys all address the viewers, and a
… +N moremarker tells the operator what they are missing without letting them read it. Scrolling reaches every agent and — with the border indicator — also satisfies the "says plainly what it is hiding" half.Verified:
cargo test --workspace(878 tests, all pass),cargo clippy --workspace --all-targets -- -D warningsclean,cargo fmt --all --checkclean.config_agents_pane_scrolls_to_the_agents_it_cannot_fitrenders the screen at 80x24 with eight agents, asserts the pane reports its overflow, walksJto the bottom and asserts the last agent is on screen, then checksPgUpwalks back andjstill moves the viewer selection.0/7 ↕ J/K PgDn/PgUpand two PgDn presses bring the last agent into view; at 60x10 it reads0/21and pages down to21/21with the last agent visible. The?map shows both new rows.Branch
config-agents-scroll, one commit, not pushed.