Skip to content

feat(agents): show a parked child as parked, not "waiting for input" - #5942

Merged
Hmbown merged 1 commit into
mainfrom
feat/parked-agents-view-5906
Sep 6, 2026
Merged

feat(agents): show a parked child as parked, not "waiting for input"#5942
Hmbown merged 1 commit into
mainfrom
feat/parked-agents-view-5906

Conversation

@Hmbown

@Hmbown Hmbown commented Sep 6, 2026

Copy link
Copy Markdown
Owner

No-Issue: display half of #5906 (the claim half closed with #5921); the founder's "2 sub agents waiting for input" report.

Refs #5906

Builds on the recording work that landed in #5921 (now on main), which
added parked_at_turn_end to SubAgentCheckpoint and AgentWorkerRecord
but deliberately left every UI projection of WaitingForUser alone. This
is the display half.

The complaint that remained

A child parked because its parent's turn ended shows up in the Agents
panel, the sidebar and Agent Details as "waiting for input" — the same
label as a child that actually asked the operator a question. There is no
way to tell them apart, and Agent Details prints a Pending question:
line quoting a resume recipe nobody asked for.

The runtime is why the surfaces got it wrong. Parking hands the child a
needs_input note phrased as a question:

Resume this parked child with agent(action="start", resume_from="agent_X").

Every projection keys off needs_input, so the parked husk reads as
answerable. The flag has to outrank it.

What changed

One derivation, one status vocabulary. AgentCurrentActivityStatus
gains Parked; it is derived in exactly one place —
subagent_routing::subagent_is_parked, reading the checkpoint flag —
ahead of needs_input, and made sticky across the Interrupted
terminal projection the same way Waiting already was. No surface sniffs
a reason string, and no second status enum was introduced.

Three surfaces say parked. The work-surface / Agents rows, the
sidebar and Agent Details render the state, with a one-line recovery
standing in for the fake question: resume_from to continue, cancel to
dismiss — the verbs the agent tool actually exposes, quoted exactly.
Agent Details also suppresses the parked child's needs_input note as a
pending question; it asks the operator nothing.

Parked sorts below live work and stops inflating the chip. Parked
lands in WorkBucket::Ready, not Attention, so it ranks under both
Active and Attention and the work heading's blocked count covers only
children a person is genuinely holding up. RosterState::Parked gives the
receipts roster its own state and glyph (, hollow-but-unfinished), and
parked husks sink below live and waiting rows there too despite the rail's
otherwise-chronological order. The wire state string gains parked.

Translated, not fallen back. AgentStatusParked and
AgentStatusParkedRecovery ship in all 15 complete packs. A new parity
test additionally pins resume_from and cancel as untranslatable
tokens in every pack, so no translation can tell an operator to type a
verb that does not exist.

Verified

cargo fmt --all -- --check                       clean
cargo check -p codewhale-tui                     clean
RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib \
  -- tui::work_surface tui::agent_details tui::sidebar \
     tui::agent_roster localization
  test result: ok. 227 passed; 0 failed; 0 ignored   (216 before this change)
RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib
  test result: ok. 11733 passed; 0 failed; 13 ignored

Eleven new tests, each built from a child constructed exactly as the
turn-end parking projection builds one (Interrupted + WaitingForUser

  • the question-shaped needs_input note + a checkpoint with the flag),
    paired against a child that genuinely asked:
  • sidebar: parked row says parked, the asking row still says
    waiting; the parked word follows the UI locale (asserted against
    tr(De, AgentStatusParked), not a literal).
  • work_surface: the parked row's detail leads with parked, names
    resume_from and cancel, and never replays the parking note; the
    asking row keeps waiting for input and its real question; the
    blocked chip counts one, not two; the parked row sorts below both the
    live and the asking row; and the status word survives the narrow-row
    ladder at widths 96 / 72 / 56, where the existing degradation test
    already pins running.
  • agent_details: parked body says State: parked, carries both recovery
    verbs, and has no Pending question line; a child that really asked
    still reports State: waiting with its question.
  • agent_roster: a parked record is RosterState::Parked with a distinct
    glyph and is not terminal; parked husks sort below live and waiting rows
    despite being oldest; clearing the flag on re-dispatch returns the row
    to Running.
  • localization: every complete pack translates both keys away from
    English and keeps resume_from / cancel intact.

The five UI guards were each shown failing on their exact assertion with
subagent_is_parked reverted to false.

Not verified / not done

  • Local-test evidence only. No provider call, no deploy, and no manual
    reproduction of the original incident in a running TUI.
  • whales.rs is untouched. WhaleState::for_subagent still maps a
    parked child to Waiting via needs_input. That is the ambient whale
    glyph vocabulary, not the Agents/sidebar/details rows this issue is
    about; folding it in would have widened the change past the surfaces
    named in the complaint.
  • The Ready bucket is a reuse, not a new one. A parked husk is not
    "queued", but Ready is the existing rank that sits below Active and
    Attention while staying actionable, and adding a fifth bucket would have
    rippled through the heading copy and every bucket match in the module
    for no behavioral gain.
  • resume_from is not offered as a keybinding. The recovery line
    names the two tool actions that exist; wiring a row action to invoke
    them is separate work.

🤖 Generated with Claude Code

https://claude.ai/code/session_0188XYyJaw9Mh9uSrqQBoqhm


Devin Review

Note

Low Risk
TUI presentation and sorting only; no changes to auth, persistence, or agent execution semantics beyond how parked checkpoints are displayed.

Overview
Fixes #5906 by treating turn-end–parked child agents as their own state instead of “waiting for input”, which previously matched real operator questions because parking still sets a question-shaped needs_input note.

Derivation and activity model. subagent_is_parked reads checkpoint.parked_at_turn_end in subagent_routing and maps to AgentCurrentActivityStatus::Parked before needs_input; that status stays sticky across Interrupted terminal projection like Waiting. Parked rows show localized recovery copy (resume_from / cancel) instead of the fake question, and Agent Details no longer surfaces a Pending question for parked husks.

Surfaces and sorting. Work surface, sidebar, agent details, and the agent roster all label parked (with RosterState::Parked, glyph , wire state: "parked"). Parked work lands in WorkBucket::Ready so it does not inflate the blocked chip or sort above live/answerable agents; the roster sinks parked rows below live and waiting rows.

i18n. AgentStatusParked and AgentStatusParkedRecovery ship in all locale packs, with a parity test that keeps resume_from and cancel as literal tool tokens.

Reviewed by Cursor Bugbot for commit ab9d13b. Bugbot is set up for automated code reviews on this repo. Configure here.

#5921 recorded the park/ask distinction (`parked_at_turn_end` on the
checkpoint and the worker record) but deliberately left every UI
projection alone. The result is the complaint that remains on #5906: a
child parked because its parent's turn ended sits in the Agents panel,
the sidebar and Agent Details wearing the same "waiting for input" label
as a child that actually asked a question — with no way to tell them
apart, and a "Pending question" line quoting a resume recipe nobody
asked for.

The runtime is why the surfaces got it wrong: parking hands the child a
`needs_input` note phrased as a question ("Resume this parked child with
..."), and every projection keys off `needs_input`. So the flag has to
outrank it.

- `AgentCurrentActivityStatus::Parked` joins the existing status
  vocabulary (no second one). It is derived in exactly one place —
  `subagent_routing::subagent_is_parked`, reading the checkpoint flag —
  ahead of `needs_input`, and made sticky across the `Interrupted`
  terminal projection the same way `Waiting` already was.
- The work-surface rows, the sidebar and Agent Details render it as
  `parked`, with a one-line recovery in place of the fake question:
  `resume_from` to continue, `cancel` to dismiss — the verbs the `agent`
  tool actually exposes, quoted.
- Parked lands in `WorkBucket::Ready`, not `Attention`: it sorts below
  live and answerable work and stops inflating the `blocked` chip.
- `RosterState::Parked` gives the receipts roster its own state and
  glyph, and parked husks sink below live and waiting rows there too.
  The wire `state` string gains `parked`.
- `AgentStatusParked` / `AgentStatusParkedRecovery` ship translated in
  all 15 complete packs; a new parity test also pins `resume_from` and
  `cancel` as untranslatable tokens, so no pack can tell an operator to
  type a verb that does not exist.

Agent Details additionally suppresses the parked child's `needs_input`
note as a pending question — it asks the operator nothing.

Verified (rebased onto origin/main, which now carries #5921)

  cargo fmt --all -- --check                     clean
  cargo check -p codewhale-tui                   clean
  RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib \
    -- tui::work_surface tui::agent_details tui::sidebar \
       tui::agent_roster localization
    test result: ok. 227 passed; 0 failed; 0 ignored   (216 before)
  RUST_MIN_STACK=16777216 cargo test -p codewhale-tui --lib
    test result: ok. 11733 passed; 0 failed; 13 ignored

Eleven new tests. The five UI guards were each shown failing on their
exact assertion with `subagent_is_parked` reverted to `false`.

Local-test evidence only: no provider call, no deploy, and no manual
reproduction of the original incident in a running TUI.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0188XYyJaw9Mh9uSrqQBoqhm
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Copilot AI lite review requested due to automatic review settings September 6, 2026 08:00
@cursor

cursor Bot commented Sep 6, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_bf594a95-9393-40bb-976b-a494ca8671e2)

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-06T08:10:24.741266Z ab9d13b PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 5m 13s —— View job


Review complete

  • Gather context (PR description, changed files list)
  • Read the diff against origin/main
  • Review core derivation logic (subagent_routing.rs, subagent_is_parked)
  • Review UI surfaces (agent_details.rs, sidebar.rs, work_surface, agent_roster.rs)
  • Review localization changes and parity test
  • Check provider/model-route safety implications — none, this PR doesn't touch routing/model selection
  • Post final review

Note: cargo/git fetch were blocked by the sandbox in this session (network/build commands require manual approval I couldn't grant), so this is a static read of the diff — I did not independently re-run the test suite the PR description reports.

Summary

The single-derivation design holds up: subagent_is_parked (crates/tui/src/tui/subagent_routing.rs:96) is the only place that reads checkpoint.parked_at_turn_end, it correctly outranks needs_input, RosterState/AgentCurrentActivityStatus both grew exactly one variant instead of a parallel enum, and every exhaustive match over the new variant (agent_details.rs, sidebar.rs, work_surface/model.rs) is handled with an explicit unreachable!() rather than a silent wildcard. WorkBucket ranking (Attention(0) < Active(1) < Ready(2)) and the attention chip count (crates/tui/src/tui/work_surface/model.rs:1085) correctly exclude Parked, matching the "1 blocked, not 2" test. Roster sort is a second sort_by_key on a bool, which is stable and correctly keeps insertion order within each group. Localization additions are complete across all 15 packs with a parity test pinning resume_from/cancel as untranslatable tokens. No provider/model-routing code is touched.

Finding: recovery text can briefly regress to raw, unlocalized runtime text

apply_subagent_terminal_projection (crates/tui/src/tui/subagent_routing.rs:124-151) now treats Waiting and Parked symmetrically as "sticky": if the existing status is one of those and the new worker_status is Interrupted, it keeps the old status but still overwrites detail with safe_result — the raw result string passed in.

For a parked child, that call path is reached from the SubAgentMailbox::Interrupted handler (crates/tui/src/tui/ui/event_loop.rs:3222-3239subagent_terminal_projection_from_mailbox, crates/tui/src/tui/ui/terminal.rs:206-210), and the reason string it carries is built at the runtime layer (crates/tui/src/tools/subagent/mod.rs:11219-11221):

"Parent turn ended before this turn-owned child settled. Work was parked instead of discarded; resume with agent(action="start", prompt="Continue the parked assignment.", resume_from="{agent_id}")."

So for the window between that mailbox event and the next full agent-list reconcile (reconcile_subagent_activity_state_at, which does compute the localized parked_recovery_detail), Agent Details / sidebar / the work surface would show the correctly-translated parked status word, but the Current:/detail line would show this raw, un-localized English sentence instead of the intended "resume_from to continue, cancel to dismiss" copy — and it's missing the cancel verb entirely during that window.

This mirrors a pre-existing quirk the Waiting branch already had before this PR (same overwrite-detail-on-sticky-status behavior), so it isn't a new bug class introduced here, and it self-heals on the next roster refresh. But it does undercut the PR's own stated invariant ("no surface sniffs a reason string") for the detail field specifically, and none of the eleven new tests catch it because they all call reconcile_subagent_activity_state directly, bypassing apply_subagent_terminal_projection/the mailbox path entirely. Worth at least a test asserting apply_subagent_terminal_projection preserves (or re-derives) the recovery detail when sticky-preserving Parked, mirroring the Waiting case it already generalizes.

Fix this →

Minor nit

In agent_details.rs, the "Current:" block (crates/tui/src/tui/agent_details.rs:246-275) excludes Waiting (since Pending question: already covers it) but not Parked, so a parked child's body renders both State: parked · … and Current: parked · resume_from to continue, cancel to dismiss — the word "parked" appears twice. This looks intentional (it's how the recovery detail reaches the body at all, and the new test asserts on it), just flagging in case the double-parked wording wasn't deliberate.

Nothing else stood out

Sort stability, bucket/rank math, roster is_terminal exclusion, and the wire state doc-comment update all check out. Good test coverage for the derivation itself; the gap above is specifically in the terminal-mailbox projection path, not the core subagent_is_parked logic.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Agent Details may incorrectly render a redundant/confusing Current: line for Parked status unless Parked is excluded like Waiting.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the TUI’s agent-status derivation and rendering so that turn-end–parked child agents are displayed as parked (distinct from truly waiting for input) across the work surface, sidebar, agent details, and receipts roster, using a single derived status and fully translated copy.

Changes:

  • Introduces AgentCurrentActivityStatus::Parked and derives it centrally from checkpoint.parked_at_turn_end, outranking needs_input.
  • Updates work-surface rows, sidebar rows, agent details, and the agent roster to render parked (including recovery guidance using resume_from / cancel) and to adjust sorting/bucketing so parked husks don’t inflate “blocked”.
  • Extends localization and protocol/parity wiring to support the new parked vocabulary and wire state.
File summaries
File Description
crates/tui/src/tui/work_surface/model.rs Uses localized label for Parked and buckets it into Ready to avoid “blocked” inflation.
crates/tui/src/tui/work_surface/mod.rs Adds work-surface tests covering parked vs real waiting behavior, sorting, and narrow-row rendering.
crates/tui/src/tui/subagent_routing.rs Centralizes parked detection + recovery detail and ensures parked outranks needs_input in derived activity.
crates/tui/src/tui/sidebar.rs Localizes parked status word in sidebar rows and adds coverage tests.
crates/tui/src/tui/app.rs Adds AgentCurrentActivityStatus::Parked and documents derivation constraints.
crates/tui/src/tui/agent_roster.rs Adds RosterState::Parked, glyph, and sorting rule to sink parked rows.
crates/tui/src/tui/agent_roster/tests.rs Adds roster tests validating parked state mapping, sorting, and redispatch clearing.
crates/tui/src/tui/agent_details.rs Localizes parked state and suppresses parked “Pending question” projection.
crates/tui/src/localization.rs Adds message IDs and parity tests ensuring parked copy is translated and preserves tool tokens.
crates/tui/src/core/protocol_parity.rs Adds parked to roster-state string mapping for protocol parity.
crates/tui/locales/en.json Adds English strings for parked status and recovery line.
crates/tui/locales/de.json Adds German parked strings.
crates/tui/locales/ca.json Adds Catalan parked strings.
crates/tui/locales/es-419.json Adds LATAM Spanish parked strings.
crates/tui/locales/fr.json Adds French parked strings.
crates/tui/locales/pt-BR.json Adds Brazilian Portuguese parked strings.
crates/tui/locales/ru.json Adds Russian parked strings.
crates/tui/locales/uk.json Adds Ukrainian parked strings.
crates/tui/locales/vi.json Adds Vietnamese parked strings.
crates/tui/locales/hi.json Adds Hindi parked strings.
crates/tui/locales/id.json Adds Indonesian parked strings.
crates/tui/locales/ja.json Adds Japanese parked strings.
crates/tui/locales/ko.json Adds Korean parked strings.
crates/tui/locales/zh-Hans.json Adds Simplified Chinese parked strings.
crates/tui/locales/zh-Hant.json Adds Traditional Chinese parked strings.
crates/protocol/src/event_msg.rs Updates roster state documentation to include parked.
Review details
  • Files reviewed: 26/26 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 253 to +257
| AgentCurrentActivityStatus::Interrupted
| AgentCurrentActivityStatus::Waiting
)
{
let mut current = vec![activity_status_label(activity.status).to_string()];
let mut current = vec![activity_status_label(activity.status, app.ui_locale).into_owned()];

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment on lines +103 to +107
pub(crate) fn subagent_is_parked(agent: &SubAgentResult) -> bool {
agent
.checkpoint
.as_ref()
.is_some_and(|checkpoint| checkpoint.parked_at_turn_end)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Parked agents still say waiting

The Agents modal still labels a parked child “Waiting for you” through WhaleState::for_subagent. This contradicts its parked status and falsely implies user input can resume it.

Prompt for agents
Extend the shared parked-state derivation to the /subagents Agents modal. WhaleState::for_subagent in crates/tui/src/tui/whales.rs currently checks needs_input first, so a parked checkpoint still becomes WhaleState::Waiting and renders the localized “Waiting for you” badge. Make that modal consume the typed parked distinction without reintroducing string inference or a parallel lifecycle system. Add focused coverage pairing a parked child with a genuinely asking child.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab9d13b0b3

ℹ️ 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".

Comment on lines +103 to +107
pub(crate) fn subagent_is_parked(agent: &SubAgentResult) -> bool {
agent
.checkpoint
.as_ref()
.is_some_and(|checkpoint| checkpoint.parked_at_turn_end)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop treating canceled checkpoints as parked

After agent(action="cancel") is used on a parked child, terminalize_settled_worker_on_cancel changes the worker record to Cancelled but leaves both the checkpoint's parked_at_turn_end flag and the record flag set. Because this helper treats that historical flag as unconditional, the next AgentList reconciliation changes the child back to Parked; RosterState::from_record likewise reports it as parked and nonterminal. The advertised “cancel to dismiss” action therefore leaves every new projection showing the recovery prompt and can keep parent settled counts wrong. Clear the parked flags on cancellation or require a nonterminal worker state when deriving Parked.

AGENTS.md reference: crates/tui/AGENTS.md:L16-L17

Useful? React with 👍 / 👎.

Comment thread crates/tui/src/tui/app.rs
/// actually answer. It is its own state here because the recovery is
/// different: nobody will answer it, and it is continued through
/// `resume_from` (a *new* agent) or dismissed with `cancel`.
Parked,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Freeze elapsed time when a child parks

A parked child has stopped executing, but adding this state without including it in the elapsed-time latch means agent_is_terminal returns false as soon as the Parked activity takes precedence. Consequently, freeze_terminal_agent_elapsed never records its duration even though the manager recomputes SubAgentResult::duration_ms from started_at.elapsed() on every snapshot, so the parked work row's elapsed time continues ticking until archival; the durable roster has the same problem because its WaitingForUser record has no completed_at_ms. Treat Parked as stopped for timing purposes without dropping its unfinished recovery detail.

AGENTS.md reference: crates/tui/AGENTS.md:L18-L19

Useful? React with 👍 / 👎.

// answerable (#5906). They are the one class of row the operator is not
// meant to scan past to find real work, and the sort is stable so the
// history order survives inside each group.
rows.sort_by_key(|row| row.state == RosterState::Parked);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve receipt order behind live agents

This boolean sort moves parked rows behind every nonparked row, including later completed, failed, and canceled receipts. For concurrent children where an older long-running child parks after a newer child has already completed, the historical roster therefore changes from creation order to showing the newer completion first, even though only live and answerable rows need to rank above parked work. Use a rank that promotes live/waiting rows while retaining creation order among parked and terminal receipts.

Useful? React with 👍 / 👎.

Comment on lines +249 to +253
let parked = subagent_is_parked(agent);
// Parked outranks `needs_input`: the park note *is* phrased as a
// question, and reading it as one is exactly the bug (#5906).
let mut structured_status = if parked {
AgentCurrentActivityStatus::Parked

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain parked rows until recovery

A turn-end parked child still has SubAgentStatus::Interrupted, and reconcile_terminal_subagent_card_retention runs before this new checkpoint-based derivation. On the first later reconciliation more than 45 seconds after parking, that retention pass therefore removes the child from subagent_cache as a terminal card, causing the work-surface Agents view, sidebar projection, and Agent Details entry to lose the newly advertised resume_from/cancel recovery even though Parked is classified as unfinished and actionable. Exempt parked checkpoints from terminal-card expiry until they are resumed or dismissed.

Useful? React with 👍 / 👎.

"WhaleStateBlocked": "Blocked",
"WhaleStateOffline": "Offline",
"AgentStatusParked": "parked",
"AgentStatusParkedRecovery": "nobody is waiting on it; resume_from to continue, or cancel to dismiss",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Compose agent tool tokens outside translations

The recovery strings now embed the resume_from argument and cancel action directly in every locale pack, while the scoped TUI contract requires commands and key names to be composed in code. The added parity test merely pins fifteen duplicated literals, so a tool-schema rename or a partial locale correction can leave user instructions referring to an obsolete action. Use named placeholders in the translated prose and interpolate the canonical tool tokens in parked_recovery_detail.

AGENTS.md reference: crates/tui/AGENTS.md:L25-L26

Useful? React with 👍 / 👎.

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codewhale review

PR introduces a centralized Parked status derived from checkpoint.parked_at_turn_end and propagates it through the work surface, sidebar, agent roster, and agent details, with localized copy. The approach is mostly clean and test coverage is strong, but the Agent Details current-activity guard does not appear to include the new variant, and the roster sort key may over-sink parked rows.

Findings

  • [ERROR] Agent Details current-activity guard does not include Parked (crates/tui/src/tui/agent_details.rs:254)
    project_agent_details still guards the block that renders the State: ... line with a matches! over Queued..Waiting. A parked child has AgentCurrentActivityStatus::Parked, so it will not enter this branch and State: parked will not render. The new test in this file should fail if this path is exercised, so the guard must be updated.
  • [WARNING] Roster sort puts parked rows below terminal states too (crates/tui/src/tui/agent_roster.rs:153)
    build_agent_roster sorts with row.state == RosterState::Parked, a bool key, so parked rows sink below every non-parked row, including Done/Failed/Cancelled. The comment and PR description describe sinking below live and answerable rows only, and there is no test covering terminal rows.
  • [INFO] Localization parity test only uses substring checks for tool tokens (crates/tui/src/localization.rs:4855)
    Asserting recovery.contains("cancel") does not prove cancel is an intact tool token; for example, cancelar or cancelled would pass but are not the verb the agent tool exposes. Current packs ship the literal token, but the parity gate should be stricter.

Suggestions

  • crates/tui/src/tui/agent_details.rs:254 — Add Parked to the guard so parked children render the status line.

                    | AgentCurrentActivityStatus::Parked
                    | AgentCurrentActivityStatus::Waiting
    

Assessment

Good centralized derivation and broad test coverage, but the Agent Details guard likely needs to add Parked before the feature works on that surface. The roster ordering and the token parity test could also be tightened.


Advisory review by Codewhale (codewhale review --pr 5942 --post, head ab9d13b0b32d891611dd563be7c44837ff2add3d). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

@@ -253,8 +253,8 @@
| AgentCurrentActivityStatus::Interrupted
| AgentCurrentActivityStatus::Waiting

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ERROR] Agent Details current-activity guard does not include Parked

project_agent_details still guards the block that renders the State: ... line with a matches! over Queued..Waiting. A parked child has AgentCurrentActivityStatus::Parked, so it will not enter this branch and State: parked will not render. The new test in this file should fail if this path is exercised, so the guard must be updated.

// ...except parked husks, which sink below everything still live or
// answerable (#5906). They are the one class of row the operator is not
// meant to scan past to find real work, and the sort is stable so the
// history order survives inside each group.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] Roster sort puts parked rows below terminal states too

build_agent_roster sorts with row.state == RosterState::Parked, a bool key, so parked rows sink below every non-parked row, including Done/Failed/Cancelled. The comment and PR description describe sinking below live and answerable rows only, and there is no test covering terminal rows.

"{} lost the resume verb the agent tool exposes: {recovery}",
locale.tag()
);
assert!(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[INFO] Localization parity test only uses substring checks for tool tokens

Asserting recovery.contains("cancel") does not prove cancel is an intact tool token; for example, cancelar or cancelled would pass but are not the verb the agent tool exposes. Current packs ship the literal token, but the parity gate should be stricter.

@@ -253,8 +253,8 @@
| AgentCurrentActivityStatus::Interrupted
| AgentCurrentActivityStatus::Waiting

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add Parked to the guard so parked children render the status line.

Suggested change
| AgentCurrentActivityStatus::Waiting
| AgentCurrentActivityStatus::Parked
| AgentCurrentActivityStatus::Waiting

@Hmbown
Hmbown merged commit b0b7c8b into main Sep 6, 2026
34 of 38 checks passed
@Hmbown
Hmbown deleted the feat/parked-agents-view-5906 branch September 6, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants