fix(tui): first-class shells on the work strip - #5699
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude encountered an error after 1m 51s —— View job Claude is reviewing this PR
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1b7fda40e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| primary_action: Some(SidebarRowAction::InspectWork { | ||
| title: format!("Shell {}", entry.id), | ||
| body: shell_inspector_body(app, entry, command, status), |
There was a problem hiding this comment.
For any verbose running shell, constructing every work-strip projection now calls shell_inspector_body, which locks the manager, calls inspect_job_for_session, and copies the shell's full retained stdout/stderr into the row action. A live shell can retain up to 16 MiB per stream (tools/shell/output.rs), while visible_rows_for_panel is called during layout, rendering, and input, so ordinary redraws can repeatedly allocate and copy tens of MiB per shell and make the TUI unresponsive. Defer output retrieval until the row is activated, or project only a bounded tail.
Useful? React with 👍 / 👎.
| if !matches!(entry.status.as_str(), "running" | "queued") { | ||
| return false; | ||
| } | ||
| entry.prompt_summary.starts_with("shell: ") || entry.id.starts_with("shell_") |
There was a problem hiding this comment.
Classifying shell liveness from the presentation strings "running" and "queued" makes row visibility depend on English status spelling rather than the existing typed ShellStatus; any new or renamed live state will silently disappear from the strip. Preserve a typed shell discriminator/status in TaskPanelEntry, or classify the typed ShellJobSnapshot before flattening it for display.
AGENTS.md reference: crates/tui/AGENTS.md:L13-L14
Useful? React with 👍 / 👎.
| // Live shells belong on the work strip (`▾ Shells N`), not this | ||
| // composer crumb. A dual surface hid the PTY behind hourglasses. | ||
| if entry.prompt_summary.starts_with("shell: ") || entry.id.starts_with("shell_") { | ||
| continue; |
There was a problem hiding this comment.
When rail_panel=context, work_surface/render/mod.rs renders the context fact list directly and visible_rows_for_panel returns no work rows, so skipping shells here removes their only persistent indicator: neither the composer crumb nor the new Shells rows are painted. A user with the persisted Context panel can therefore start a background shell and see no running state or cancellation id until manually invoking /jobs; retain a fallback indicator or include shell work alongside that panel.
AGENTS.md reference: crates/tui/AGENTS.md:L8-L10
Useful? React with 👍 / 👎.
| label: format!("Shells {}", shells.len()), | ||
| detail: "Watch live output · cancel by the id on the row".to_string(), |
There was a problem hiding this comment.
The new heading detail and inspector copy—including “Watch live output,” “Running shells,” and the status/output instructions—are hard-coded English, so every non-English locale displays an untranslated block. Add typed MessageId entries and render these strings through tr(locale, ...), leaving only command names, ids, and glyphs composed in code.
AGENTS.md reference: crates/tui/AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
| agent: Some(AgentRowFacts { | ||
| role_label: "shell".to_string(), | ||
| status: status.to_string(), |
There was a problem hiding this comment.
At the supported 26-column side-rail width, the shared agent identity cap is only about 10 columns, so the 14-column shell_<8 hex> label is dropped and this fallback paints only shell; because agent-style rows do not render detail, the exact cancellation id disappears from the row. This makes the new id-on-row contract unusable in compact Left/Right layouts, especially with multiple jobs, so preserve the full shell id ahead of optional role/status chrome or use a shell-specific layout.
AGENTS.md reference: crates/tui/AGENTS.md:L19-L21
Useful? React with 👍 / 👎.
| if !detail.stdout.is_empty() { | ||
| body.push_str("\nSTDOUT:\n"); | ||
| body.push_str(&detail.stdout); | ||
| } | ||
| if !detail.stderr.is_empty() { | ||
| body.push_str("\nSTDERR:\n"); | ||
| body.push_str(&detail.stderr); |
There was a problem hiding this comment.
When a running shell exceeds the raw stream's 16 MiB retention ceiling, inspect_job_for_session returns only the retained tail while snapshot.stdout_len/stderr_len still describe the full stream; this new inspector prints that tail under plain STDOUT/STDERR headings without any omission marker or byte counts. Users can therefore copy or review a clipped log as if it were complete, so propagate the omitted-byte metadata or explicitly label the retained tail before displaying it.
Useful? React with 👍 / 👎.
Shells were a composer crumb and `/jobs cancel all` looked up a task named "all". Project live shells beside subagents so you can open, watch, and cancel by the shell_* id on the row. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
b1b7fda to
1e663e5
Compare
|
Claude finished @Hmbown's task in 2m 12s —— View job Review: first-class shells on the work stripI verified the diff ( Confirmed issues1. Full stdout/stderr copied into every row on every render/layout/input pass — 2. Shell visibility disappears entirely when the Context rail panel is selected — this PR removes shells from the composer crumb ( 3. The row's own Lower-priority, worth a follow-up
Good choices
|
Version drift wants release-note receipts for the #5698/#5699 references; the dead-code budget grows by the 64 standalone Tideline items that lose their allows the moment frame wiring lands (the next Tideline slice must bring the number back down). No-Issue: gate receipts for the Tideline component family Signed-off-by: CodeWhale Bot <bot@codewhale.net>
- Spec for all three approved screens: component inventory with slot lineage, layout contract, color contract (ChromeInk only), motion table (wall-clock keyed, still frames), clickability registry, moment map. Impossible-in-Ratatui elements resolved explicitly. - Topbar component (tui/topbar.rs): deterministic widget, shed order, per-segment hitboxes, ascii-safe, meter warn ink; golden-buffer harness + 12 goldens at BLOCKER_SIZES. 6/6 via dev-test.sh tui topbar. - Not wired into frame.rs: collides with Hmbown#5698/Hmbown#5699 writers. No-Issue: design scaffolding on the ratified tideline direction
Standalone whale table (spec §2/§5a/§5b): WHALE 10 (never truncates),
ASSIGNMENT remainder (truncate with …, never wrap), STATE 12 glyph+word,
then ELAPSED/RECEIPTS/LAST UPDATE 8-wide time columns shedding
LAST UPDATE → RECEIPTS+ELAPSED per the §5b ladder — at 80 columns the
ledger is exactly WHALE │ ASSIGNMENT │ STATE. Selected row ▶ marker,
state marks carry words (color never invents state), failed rows are
the only Failure ink, row rects → inspector.
Deterministic injected rows (subagent_cache projection is the landing
slice's job); ASCII-safe ▶>/●./✓Y/✗X/│|. Goldens
ledger_{w}x{h} at the four blocker sizes; the 80-wide golden proves
the shed. Hmbown#5699 shell semantics untouched; not wired into ui/frame.rs
(Hmbown#5698 gate).
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Receipt stream (spec §5a, in history.rs/tideline_stream.rs): user ▎ and assistant ▏ turns, the pod-formation ├──/└── tree (same object as the ledger below — continuity), state-marked receipt rows with words, timestamps and ×N counts, indented conclusion block, and the legend row teaching the marks in place. Row rects span pod trees; marks pair with words so color never invents state. Rail (work_surface/tideline.rs, additive per spec — Hmbown#5699 shell semantics untouched): five groups RUNS/WHALES/POD/WORK/CONTEXT, context meter, help/settings meta rows, « collapse with » expander; width ladder 22/16/0 per §5b (rail sheds at 80). Group rects follow the painted rows. The composite renders rail │ stream; ledger and composer dock into it in the live shell and carry their own goldens. Goldens work_{w}x{h} at the four blocker sizes prove both components; the 80-wide golden proves the rail shed. Not wired into ui/frame.rs (Hmbown#5698 gate). Signed-off-by: CodeWhale Bot <bot@codewhale.net>
1 participant
Summary
▾ Shells Ngroup on the work strip. Open a row to watch output; theshell_*id on the row is the cancel target (/jobs cancel <id>)./jobs cancel all(space) and/jobs cancel-allboth cancel running shells. The old parse treatedallas a task id and printedCommand cancel failed: Task all not found.This is a new PR, not a ride on #5698. The #5698 startup-hero lane was left alone (no
underwater.rs, composer, orprompts/text.rsedits).What was ripped
⏳ 1 shell — cd …) as the only operator surface/jobs cancel alllooking up a TaskManager/shell-owner id namedall/jobsmisses sayingTask {id} not found(nowJob {id} not found)Test plan
parses_job_actions,handler_is_pure_and_argument_only×3 filter hits,list_shows_controls_and_stale_state,pending_work_from_app_picks_up_shell_and_agent_entries,live_shells_are_first_class_work_strip_rows,durable_tasks_are_not_promoted_as_shell_rows)tui::suite 3140 passed / 8184 skipped / 0 failed (includes work-strip subagent regression tests)b1b7fda40eedcff2251bcf862d458c7f9cea5b91)cw operatewith a live background shell: strip shows▾ Shells 1+ theshell_*id; Enter opens output;/jobs cancel allcancels that job; subagents group still worksMade with Cursor
No-Issue: founder operate fix for first-class shells and /jobs cancel all; tracked in the 0.9.12 ledger, not a standalone GitHub issue.