Skip to content

fix(tui): name the work that blocks a session switch - #12

Open
SparkofSpike wants to merge 4 commits into
mainfrom
fix/session-transition-blockers
Open

SparkofSpike wants to merge 4 commits into
mainfrom
fix/session-transition-blockers

Conversation

@SparkofSpike

@SparkofSpike SparkofSpike commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

A session-switch command refused because runtime work is active reports only a
generic line:

Cannot start a new session while runtime work is active. Wait for the current
turn, maintenance, and background tasks to finish, or cancel that specific
work.

Observed in an operator session where the real blocker was a single background
script that had been spinning for five hours: the refusal never named it, so the
first question — which work? — had no answer in the UI. /new --force was
mentioned as if it were the way out; it only discards draft or queued input and
does nothing about a running task.

The gate itself is correct and unchanged: a session transition must not detach
a live runtime producer (App::session_transition_blocked, U02-10). What was
missing is the explanation.

Changes

  • App::session_transition_blockers() -> Vec<String> (new): the same
    predicates as session_transition_blocked(), rendered in operator terms. The
    boolean now derives from this list (!blockers.is_empty()), so the gate and
    its explanation cannot drift. Task rows use the task panel's own vocabulary:
    id, status, agent_roster::format_duration for the age, and the summary
    bounded to 60 characters. At most five rows are listed, then …and N more.

  • Facet: CommandSessionLifecycleContext and CommandSessionControlContext
    gain transition_blockers(). The lifecycle adapter forwards to the App. No
    App/SessionManager type crosses the facet; the value type is Vec<String>
    because the only consumer is the message text.

  • One shared renderer: transition_blocked_message(verb, blockers) in
    commands/groups/session/mod.rs. /new, /load, /fork (both routes),
    /branch and /resume all use it, so a new blocker kind changes one place.
    The message now names the work and points at the supported exit:

    Cannot start a new session while runtime work is active:
      • shell_a3f2  running  5h 18m  cw-leftovers.ps1
      • task_9c81   queued   2m      agnes endpoint scan
    
    Wait for them to finish, or cancel them with /jobs cancel-all.
    
  • Not in this change: the gate's decision logic (unchanged), --force
    semantics (a later layer), background-task stall detection (a later layer),
    and the other consumers of the same gate — /clear
    (commands/groups/core/core.rs) and the restore-switch path
    (tui/ui/apply.rs) keep their own wording; they are not part of the
    lifecycle command slice.

Evidence

Local focused runs on Windows (the remaining gates are CI's):

cargo test -p codewhale-tui --lib transition_blockers
test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 14803 filtered out

cargo test -p codewhale-tui --lib lifecycle_portable_tests
test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 14797 filtered out

cargo test -p codewhale-tui --lib session_lifecycle_regression
test result: ok. 32 passed; 0 failed; 0 ignored; 0 measured; 14773 filtered out

cargo check -p codewhale-tui -p codewhale-command-contract is clean and
cargo fmt --all -- --check passes.

The new consistency assertion pins session_transition_blocked() == !session_transition_blockers().is_empty(); without it, the two could drift
while every other test kept passing. Exact-message tests updated where they
intentionally pin the composed string; three assertions that depended on the
removed --force sentence now assert the named blocker instead.

Review responses

Adversarial review (SpikeBot 005) found that the shared tail advised
/jobs cancel-all for blockers that command cannot clear: it only kills shell
processes (ShellJobAction::CancelAll → kill_running_for_session) and
answers "No running commands to cancel." for a running turn, an in-flight
dispatch, compaction, or cleanup. 3ba1c1d65 responds: the tail names both
exits with their scope — Ctrl+C stops a running turn, /jobs cancel-all
cancels running shell jobs — and keeps "Wait for them to finish" as the advice
that is always true.

Adversarial review (SpikeBot 003) left seven findings; fd23382e2 responds to
all of them. The two majors were the row budget and a test that could not fail:

  • A row was bounded by a 60-character summary, but rows render into a
    transcript Note body that is 74 display columns wide in an 80-column
    terminal. A CJK summary (60 ideographs = 120 columns) doubled every row and
    wrapped the whole message. The budget is now derived from the assembled
    columns and truncation goes through truncate_line_to_width, the existing
    display-width-aware helper.
  • The "stays bounded" assertion allowed 100 characters — eighteen more than a
    row can occupy — so it passed even with the alignment broken. It now
    measures display width against the real 74-column body, and the test's
    summary is CJK so a character-count budget cannot pass it.

The rest: a missing duration renders - (the placeholder the task list
already uses) instead of collapsing the column; a row names its owning
sub-agent ((by verifier) ...); the panel's shell: prefix is stripped;
a turn is still running becomes the session is still busy with the current turn because is_loading is set by paths with no turn behind them; /new
states the --force scope again (the shared tail names the exits that clear a
blocker, and --force is not one of them, so the one place that owns the flag
says what it does); /clear shows the same blocker list, since a defect fixed
in five of six commands is not fixed; and both facets document that the
projection is display-only and must not be parsed back as data.

Not taken: the review's nit on the resume call-sequence assertion (kept as
a cheap ordering guard) and its suggestion to restructure the facet to a typed
row (a ~120-line change for one consumer; the doc note covers the future
caller instead).

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes

Related Issues

No-Issue: observed in an operator session on 0.10.1 (Windows); a stuck
background task held the session and the refusal did not name it.

Attribution

🤖 Generated by SpikeBot 000(CodeWhale-LOCAL)

A session-switch command refused because runtime work is active reported only
that it was refused. In the session that prompted this, the real blocker was
one background script that had been spinning for five hours, and the refusal
never named it; `/new --force` was offered as if it were the way out though it
only discards draft or queued input.

session_transition_blockers() renders the same predicates as
session_transition_blocked() in operator terms, and the boolean now derives
from that list so the gate and its explanation cannot drift. Task rows carry
the id, status, duration and a bounded summary, at most five, then a remainder
line.

Both session facets gain transition_blockers(); the lifecycle adapter and the
control adapter forward to the App. The five lifecycle commands share one
renderer, transition_blocked_message(), so a new blocker kind changes one
place.

The gate's decision logic is unchanged.
The shared tail named /jobs cancel-all for every blocker, but that command
only kills shell processes: it maps to ShellJobAction::CancelAll ->
kill_running_for_session and answers "No running commands to cancel." for a
turn, dispatch, compaction, or cleanup blocker, so the message handed those
users a way out that does nothing (adversarial review of this PR, finding P2).

The tail now names both exits with their scope: Ctrl+C stops a running turn,
and /jobs cancel-all cancels running shell jobs. "Wait for them to finish"
stays as the advice that is always true.
@SparkofSpike

Copy link
Copy Markdown
Owner Author

结论

有必须修复项(1 个 major,其余为 minor/nit)。设计方向正确——用同一份谓词列表派生布尔值,确实是消灭漂移的正确做法——但新增的测试对「行对齐」这条核心承诺的断言是假的(它们无法失败),且 CLI 侧 --force 文案的删除把一条仍然成立的说明也一起删掉了。


发现清单

1. blocker 行的「对齐」承诺在 duration_ms == None 且多任务时必然破相 — major

  • 位置:crates/tui/src/tui/app.rs:4078-4118(transition_blocker_task_lines),常量在 app.rs:554-557

  • 理由:摘要被截到 60 字符(TRANSITION_BLOCKER_SUMMARY_CHARS),id 列宽取「最长 id」(真实值 shell_ + 8 hex = 14 字符),状态列宽 6。构造 5 条真实形态的行:

    shell_0000  queued     xxxxxxxxxx…(60)   → 行宽 82 字符
    
    • 命中该行的是 Note cell(apply.rs:1516 → HistoryCell::System → history.rs:394 走 render_message("Note", …) → markdown 渲染器),而不是 Error cell(render_error_message,history.rs:2665)。
    • 80 列终端下 content_width = 80 - (4+2) = 74;render_line_with_links_tagged(markdown_render.rs:1519)是词级折行,不会硬切(ww > width 不成立,因为每个 token 都短)。
    • 结果:…and N more 那行单独占一行,5 行 • 全都溢出右边缘。
    • 注意这一点无法靠「列宽按显示宽度算」修好——两个宽度本来就都是 0,是绝对预算太大。CJK 只让情况更糟(60 个汉字 = 120 列)。
  • 建议:给摘要预算改成「按最坏情况倒推」而不是常数 60,例如 TRANSITION_BLOCKER_SUMMARY_CHARS 取 ~34,或在 transition_blocked_message 里按终端宽度二次截断;另外把 …and N more 并入最后一条 • 行而不是另起一行。

2. 新增测试对「对齐」的断言恒真,在旧代码上也不会失败 — major(测试有效性)

  • 位置:crates/tui/src/tui/ui/tests.rs:20660-20693(transition_blockers_summarize_past_the_fifth_task_and_bound_each_summary)

  • 理由:断言是

    blockers[..5].iter().all(|row| row.chars().count() <= 100 && row.contains("..."))

    我按 truncate_text(rlm/turn.rs:313:max_chars.saturating_sub(3) 后接 "...")精确算了实际行宽:82 字符(10 字符 id + 2 + 6 字符状态 + 2 + 空 duration + 2 + 60 字符摘要)。<= 100 有 18 字符余量,contains("...") 在截断时必真。也就是说这条断言永远不可能失败,包括对齐被改坏时——它只钉住了「摘要被截断」这一实现细节,没有测「不破相」这个行为。

  • 建议:断言真实的显示宽度而不是字符数,并且阈值取一个能真正卡住的值(例如 unicode_width 下 row.width() <= 74),或者干脆改为断言「渲染后的行数 == 预期行数」。若保留字符数口径,<= 100 应改为 <= 80(即真行宽),这样上一条 major 会立刻被测试抓住。

3. 删掉 --force 澄清是「误删 + 误修」各一半 — minor

  • 位置:crates/tui/src/commands/groups/session/new.rs:63-66(旧文案在 git show FETCH_HEAD:…new.rs)
  • 理由:旧句「/new --force only discards draft or queued input」在新消息全文下依然成立且依然有用:新消息给的建议是「Wait for them to finish, or cancel them with /jobs cancel-all」——/jobs cancel-all 只杀 shell 作业,不解除 is_loading / dispatch_in_flight / is_compacting / is_purging(见 tools/shell.rs:3208 kill_running_for_session 只过滤 ShellStatus::Running)。所以一个「turn 在跑」的用户读完新消息,仍然会去试 --force,而 --force 依然没用。PR 描述把这条说成「被当成出路来提」是准确的,但删掉它并没有回答「那我能不能 --force」这个问题,只是把它从「被明确否掉」变成「用户自己去撞」。
  • 建议:在 transition_blocked_message 里保留一句等价澄清,但改成命令侧措辞(不要写进共享渲染器,因为 /load /branch 没有 --force):/new 分支在调用后追加 "/new --force only discards draft or queued input.",或者把 verb 参数扩展成 (&str, Option<&str>) 带上「本命令的 force 语义」。

4. 「cancel them」在只被 turn 挡住时没有指代对象 — minor

  • 位置:crates/tui/src/commands/groups/session/mod.rs:33-46
  • 理由:transition_blocked_message 结尾固定 "\n\nWait for them to finish, or cancel them with /jobs cancel-all."。但 blocker 列表里常见的两类——"a turn is still running" / "the runtime reports a turn in progress"——/jobs cancel-all 管不到。用户被指向一个不会生效的命令。
  • 建议:按 blocker 是否含任务行分流收尾。例如只在存在任务行时说 /jobs cancel-all,其余情况说 "Wait for it to finish, or press Esc to cancel the turn."(Esc 路径确实会清 is_loading:ui.rs:1076 mark_active_turn_cancelled_locally)。

5. transition_blockers() 的 Vec<String> 丢掉 owner_agent_name,与 Work 面板口径不一致 — minor

  • 位置:crates/tui/src/tui/app.rs:4078-4118
  • 理由:TaskPanelEntry 带 owner_agent_name(app/types.rs:383),shell 行确实会填(task_projection.rs:241,来自 ShellJobSnapshot.owner_agent_name)。而新行只渲染 id/status/duration/summary,丢弃了「这个 shell 是哪个子代理起的」。子代理起的后台 shell 正是「跑了五小时没人认领」的高发场景,认领信息恰恰是用户最需要的一列。
  • 建议:结构化数据里带上 owner,渲染为 shell_a3f2 running 5h 18m cw-leftovers.ps1 (by verifier)。

6. 新增测试在旧代码上会失败(这点没问题),但 resume 的序列断言已退化为实现钉 — nit

  • 位置:crates/tui/src/commands/groups/session/resume.rs:113-121
  • 理由:["transition_blocked", "transition_blockers"] 这条断言现在只是把「调用顺序」钉死,不再证明任何用户可见行为——两条消息断言已经覆盖了行为。它唯一的作用是防止未来有人把 gate 调用顺序颠倒,价值有限但不算错。
  • 建议:可保留,但把断言消息改成说明它防的是什么(「gate 先于 blocker 渲染,因为 blocker 只在被挡时才有意义」),或者删掉它并把「只调一次」的保证放到 transition_checks 计数上。

7. is_loading 文案在 --force 场景下不精确 — nit

  • 位置:crates/tui/src/tui/app.rs:4061("a turn is still running")
  • 理由:is_loading 为真不等于「有 turn 在跑」:commands/groups/core/core.rs:1045、groups/core/home.rs:136、groups/core/copy.rs:184 都把它置 true。届时消息会告诉用户「a turn is still running」,而实际上没有 turn。
  • 建议:改为 "the session is still busy with the current turn" 之类不承诺具体来源的措辞。

对 7 组问题的逐条作答

1. 单一事实来源:真的不可能漂移吗?

布尔侧不漂移,这半边成立。 session_transition_blocked() 现在就是 !self.session_transition_blockers().is_empty()(app.rs:4021-4022),旧的 8 个谓词全部原样搬进 session_transition_blockers()(app.rs:4034-4070),我逐条比对过:is_loading / dispatch_in_flight / suppress_stream_events_until_turn_complete / runtime_turn_status == "in_progress" / is_compacting / manual_compaction_queued / is_purging / task_panel 的 queued|running 过滤——一个不多一个不少。判定逻辑确实没改。

但「与 guard 语义重叠的直接字段读取」存在,且不止一处,列表如下(均已核对行号):

位置 读的字段 与 guard 的关系 是否该收敛
tui/control_socket.rs:448, 496, 520 is_loading + runtime_turn_status 同两个谓词,用于 busy / Interrupt 的 had_active_work 语义不同(那边问的是「有没有可中断的活」,不是「能不能切会话」)。不该收敛,但值得在注释里点明两者不是同一个问题
commands/contract/debug_operations.rs:513 is_loading + … turn_active 同上
tui/ui.rs:1113 (escape_cancel_request) is_compacting || manual_compaction_queued guard 的 2 个谓词子集 语义不同(取消 compaction)。不该收敛
tui/ui.rs:1124 is_loading || runtime_turn_status guard 的 2 个谓词子集 同上
commands/contract.rs:1291 is_loading || dispatch_in_flight guard 的 2 个谓词子集 这是 connecting 展示态,不是门。不该收敛
commands/groups/project/goal.rs:153 !goal.is_loading 另一个 struct 的同名字段 无关
commands/groups/config/config.rs:138, 1969 is_loading 「turn 中不许改 route」 语义不同

结论:没有发现「复制了 guard 的整套判定」的第二处,因此本 PR 不需要为收敛而扩大范围;但上表说明「is_loading 在 6+ 处被各自解释」,PR 描述里「同一组谓词」的说法只在本文件的守卫内成立,写进注释时值得限定范围。

2. 性能:每次调用构造 Vec<String> 是否落在热路径?

session_transition_blocked() 的全部调用点(grep 结果,已排除测试):

  • commands/contract.rs:317-318(SessionLifecycleAdapter::transition_blocked)
  • commands/contract.rs:1055-1056(SessionControlAdapter::transition_blocked)
  • commands/groups/core/core.rs:152(/clear)
  • commands/groups/core/home.rs:24(/home)
  • tui/ui/apply.rs:4283(apply_loaded_session_with_goal)

全部是命令派发 / 会话切换路径,没有一处在 render 循环里。 任务面板的渲染走 work_surface/model.rs 自己读 app.task_panel,不经过这个函数。所以「每帧构造 Vec」的担心不成立。

代价量化:最坏情况 5 条 format! + 最多 5 次 truncate_text(O(摘要长度),摘要本身已被上游截到 ≤240 字符,task_manager.rs:35,3842)+ 7 次 to_string。这是微秒级、每次用户动作一次的开销。可接受,不需要缓存。

唯一的注意点:SessionLifecycleAdapter::transition_blockers()(contract.rs:321-323)是 self.host.app.borrow(),而 command_contexts_with_config(contract.rs:4664-4693)用 Rc 共享同一个 host——transition_blocked() 和 transition_blockers() 是两次分离的 borrow(),不重叠,不会触发 RefCell 双借用 panic。无问题。

3. facet 契约:Vec<String> vs 结构化数据

我认为 Vec<String> 是一个可以接受的边界,但不是「好」的设计,而且 transition_blocked 的 doc 注释(facets.rs:1384-1388)已经把它锁成「只在被挡时读」,所以现在没有实际伤害。

未来代价(举一个具体调用方):假设要在 Work 面板里复用同一批数据——面板需要 duration_ms 来算进度条、需要 id 来做 /jobs cancel <id> 的行内按钮、需要 owner_agent_name 来分组、需要 status 来决定 icon。用现在的 facet,面板只有一条已经格式化好的字符串,它必须正则解析自己刚渲染出来的文本(shell_a3f2 running 5h 18m cw-leftovers.ps1——注意 running 和 shell_a3f2 之间的列宽是动态的,5h 18m 和 12s 之间宽度也不同),才能拿回 id。这是典型的「字符串当接口」。

改动量评估:把 trait 方法改成返回一个 struct TransitionBlocker { id: String, status: String, duration_ms: Option<u64>, summary: String, owner: Option<String> } 的 Vec,需要改:

  • command-contract/src/facets.rs(2 个 trait 方法签名)
  • command-contract/src/tests.rs(2 个 fake 的 Vec::new())
  • tui/src/commands/contract.rs(2 个 adapter)
  • commands/groups/session/{control_test_support,lifecycle_test_support}.rs(2 个 fake 的字段)
  • commands/groups/session/mod.rs(渲染器改为吃结构化类型)
  • app.rs(返回结构化列表 + 把渲染挪到 mod.rs 或保留一个 task_lines helper)

大约 8 个文件、~120 行。不建议在本 PR 做——本 PR 的目标是「让错误消息点名」,字符串边界对这一个用途是够的。但建议现在就在 facets.rs 的 doc 注释里写明「这是展示用投影,不是数据源;需要结构化数据时不要解析它,另开方法」,否则下一个调用方一定会去 parse 它。这条注释成本 3 行。

4. 命令侧行为

(a) 重复执行 gate / 可观测副作用

逐条核对 crates/tui/src/commands/groups/session/:

命令 transition_blocked() 调用次数 被挡时额外调用 问题
branch.rs:56 1 transition_blockers() ×1 无
fork.rs:69 与 fork.rs:86 2(互斥分支,同一次调用只走一条) transition_blockers() ×1 无重复
load.rs:58 1 ×1 无
new.rs:63 1 ×1 无
resume.rs:51 1 ×1 无

FakeControl::transition_blockers 会把 "transition_blockers" push 进 calls(control_test_support.rs:59-62),这正是 resume 那条序列断言的来源——它是测试夹具的计数器,不是生产审计日志。生产侧 transition_blockers() 只有一次 RefCell::borrow() + 一次列表构造,无副作用。

所以:没有发现重复执行 gate 或可观测副作用。 唯一「浪费」是「先问 bool 再问列表」= 谓词算了两遍(第一遍的 bool 是从列表派生的,第二遍又构造一次列表)。对 5 个命令、每次用户动作一次而言,无所谓;真要抠,可以在 facet 上只留 transition_blockers() 让命令自己判空——但那样每个命令都要多写一行,收益为负。保持现状是对的。

(b) /new 删掉 --force 澄清:误删还是修掉误导?

一半一半,净结果是退步。 判断依据是新消息全文:

Cannot start a new session while runtime work is active:
  • shell_a3f2  running  5h 18m  cw-leftovers.ps1

Wait for them to finish, or cancel them with /jobs cancel-all.
  • 说是「修掉误导」的部分成立:旧句把 --force 摆在一条通用拒绝消息里,读起来像是在暗示「加 --force 也许能过」。
  • 说是「误删」的部分也成立:新消息给出的唯一出路 /jobs cancel-all 只杀 shell(shell.rs:3208 只挑 ShellStatus::Running),对 is_loading / is_compacting / is_purging / dispatch_in_flight 完全无效。一个「turn 在跑」的用户在这条消息之后,唯一能想到的还是 --force,而它依然没用——只是现在消息不再提前告诉他这一点。
  • 结论:应该删掉「--force 是出路」的暗示,但不该删掉「--force 的语义边界」这条信息。 见发现 docs(i18n): complete the Tier-2 should-have docs for EPIC #5482 #3 的建议。

5. 消息渲染

(a) 只被 turn 挡住时的自洽性

不自洽。全文是:

Cannot load a session while runtime work is active:
  • a turn is still running

Wait for them to finish, or cancel them with /jobs cancel-all.

「them」指谁?只有一条 blocker,是个 turn;而 /jobs cancel-all 杀不了 turn。用户会照做、发现「No running commands to cancel.」(handlers.rs:1287),然后回到原点。

建议措辞(等价、更短、每条都对):把收尾做成 blocker 相关而非固定:

// 有任务行时
"\n\nWait for that work to finish, or stop it: `/jobs cancel-all` cancels every running shell."
// 只有 turn 类 blocker 时
"\n\nWait for it to finish, or press Esc to cancel the turn."

如果不想引入分支,最小改法是把收尾改成不承诺具体动作的中性句:"\n\nWait for it to finish, or cancel the work that holds it." —— 但这样就丢了「怎么取消」的线索,所以还是分支更好。

(b) 对齐在三种情况下是否破相

  1. 摘要含 CJK:破相,且是三种里最严重的。 format!("{id:<id_width$}") 用字符数补齐,而 CJK 字符的显示宽度是 2(markdown_render.rs 全程用 unicode_width)。id/status/duration 三列都是 ASCII(id 是 shell_ + 8 hex,见 tools/shell.rs:2674),所以补齐量本身是对的;问题在摘要长度预算是按字符数算的:60 个汉字 = 120 列,而 80 列终端给 Note cell 的正文宽度只有 74。代码注释说「Every column but the summary is ASCII, so padding by character count matches the rendered width」——这句话对「补齐」是对的,但它没有覆盖「摘要在 CJK 下会变成两倍宽」这一半。truncate_text 也是字符数语义(rlm/turn.rs:313)。
  2. id 超长:不破相。 id 列宽取 rows.iter().map(|r| r.0.chars().count()).max()(app.rs:4096-4101),长 id 自己把列撑开,所有行一起对齐。真实 id 是固定 14 字符(shell_ + 8),测试里用的 shell_{index:04} 也是 10 字符,都不会触发。唯一理论问题是某个 id 若含 CJK(当前不可能——subagent_routing.rs:882 的 summary.id 与 shell.rs:2674 都是 ASCII)会再次引入 fix: don't let Vim Normal mode swallow Space for thinking block expan… #1 的宽度错位。
  3. duration 为 None:不破相,但留下一个空洞。 unwrap_or_default()(app.rs:4081)给出空串,duration_width 可能是 0,于是行变成 id status summary(中间多两个空格,看起来像空列)。视觉上可读,但这一列在 None 时等于不存在,不同行之间会出现「有的行有这一列、有的行没有」的错位观感——因为列宽是取本批次最大值,如果 5 条里 4 条有 duration、1 条没有,那 1 条会被补齐(正确);如果全部为 None(queued 任务常见),整列消失(可接受)。format_task_list(subagent_routing.rs:929-932)在同场景下用 "-" 占位,这里用空串,两者不一致。建议跟 format_task_list 对齐,用 "-"。

6. 测试有效性

(a) 两个新测试能证明什么 / 旧代码上会失败吗

  • transition_blockers_follow_the_gate_and_name_the_blocking_task(ui/tests.rs:20629-20658):
    • 断言 1(初始 !blocked 且 blockers().is_empty()):在旧代码上不会失败——旧代码根本没有 session_transition_blockers(),这个测试编译不过。所以严格说它无法「在旧代码上失败」,它是新 API 的伴随测试。
    • 断言 2(推一条 running shell 后,blockers 含 shell_a3f2 / running / 5h 18m / cw-leftovers.ps1):测的是行为(错误消息里确实点名了任务),这是本 PR 的核心承诺,有效。contains 而非 assert_eq 是合理的(不钉死列宽)。
    • 断言 3(assert_eq!(app.session_transition_blocked(), !blockers.is_empty())):这是测实现,不是测行为。 因为 session_transition_blocked 的定义就是 !self.session_transition_blockers().is_empty()(app.rs:4021-4022),这条断言等价于 assert!(true)。它永远不可能失败,除非有人同时改掉定义——而那时这个测试的失败没有信息量(它只会说「布尔值和列表不一致」,而问题恰恰是「列表错了」)。如果它想证明「gate 与解释不漂移」,应该断言语义:例如「在 8 个谓词各自单独置位时,blocked()==true 且 blockers() 非空且至少一条提到该类工作」。现在这 8 个谓词里只有 1 个(task_panel)被覆盖到。
  • transition_blockers_summarize_past_the_fifth_task_and_bound_each_summary(ui/tests.rs:20660-20693):
    • 断言 blockers.len() == 6 和 blockers[5] == "…and 1 more":有效,测的是「最多五条 + 余量行」这个行为。
    • 断言 row.chars().count() <= 100 && row.contains("..."):无效,见发现 fix(compaction): survive a provider request-body 413 while making room #2。它既测不到「摘要被截到 60」(因为只断言了 <= 100,而 60 字符摘要的行是 82),也测不到「对齐」(字符数根本不对应显示宽度)。

(b) branch_composes_exact_baseline_messages / resume_transition_blocking_wins_before_any_route 的 fake 字符串是否忠实

  • branch(lifecycle_portable_tests.rs:55,62):手写 "shell_a3f2 running 5h 18m cw-leftovers.ps1"。与真实 App 会生成的行完全一致——我用 transition_blocker_task_lines 的算法手算过:id 列宽 10(只有一行,shell_a3f2 = 10)、状态列宽 7(running = 7)、duration 列宽 6(5h 18m = 6)、摘要 cw-leftovers.ps1,拼出来就是 shell_a3f2 running 5h 18m cw-leftovers.ps1。忠实。
  • new(lifecycle_portable_tests.rs:280):同一个字符串,但断言只用了 contains("shell_a3f2") 和 contains("/jobs cancel-all"),不依赖字符串精确性。无偏差可言。
  • resume(resume.rs:111,116):手写 "a turn is still running"。忠实——这正是 app.rs:4061 在 is_loading 时 push 的字面量。
  • 偏差来源评估:我没有发现偏差。 唯一的隐忧是这三处是手抄的,不是从 App::session_transition_blockers() 生成的;session_lifecycle_regression_tests.rs:470-487 那条走真实 App 的测试(new_session_force_cannot_detach_an_in_flight_turn)才是真正的端到端锚点,它断言了 "a turn is still running" 确实由真实 App 产出——这一条是本 PR 里最有价值的测试,它同时证明了 fake 字符串的忠实性。建议在 branch 那条测试的 fake 字符串旁加一行注释指向它,说明该字符串的权威来源是真实 App,避免将来 app.rs 改文案时这里静默漂移。

(c) resume 的 calls 序列断言是否还有意义

意义已大幅退化,但不是错的。["transition_blocked", "transition_blockers"] 现在证明的是「gate 先问、blame 后取」,这个顺序确实重要(若颠倒,会在未阻塞时也构造列表)。但上一条 assert_eq!(message(&result), …) 已经覆盖了用户可见行为,所以这条断言现在是实现钉。保留可以(成本为零),但按仓库「测试是选择性证据」的规矩,它属于「pin the implementation, not the defect」那一类,可以考虑合并进上一条断言的消息文本里。

7. 遗漏

同一根因下没有改用新消息的两处:

  1. /clear — commands/groups/core/core.rs:152-155:仍然返回 tr(MessageId::ClearConversationBusy),即 en locale 下的 "Nothing cleared — still busy. Try /clear again in a moment."(locales/en.json:805)。这条消息同样不点名是哪个任务挡住了。用户在本 PR 修复的同一场景里打 /clear,得到的还是那句无信息的话。
  2. restore-switch — tui/ui/apply.rs:4283-4287:仍然返回硬编码的 "runtime work is active; wait for the current turn, maintenance, and background tasks to finish, or cancel that specific work before switching sessions"。这是 /resume picker 选会话、/load 落到 AppAction::LoadSession 等路径最终汇入的地方(apply_loaded_session_with_goal),同样不点名。而且它的措辞("maintenance")与新的 "runtime work is active" 措辞已经不一致了。

另外两处相关的本地化问题:

  1. /home(commands/groups/core/home.rs:24-26)走 MessageId::HomeNavigationBusy("Finish or stop the current work before opening home."),也是无信息的。
  2. 新的 transition_blocked_message 与 5 个命令的 verb 字符串全部是硬编码英文。而这些命令原有的英文文案本来就是硬编码的,所以这不是本 PR 引入的回归——但如果 /clear / /home 将来要收敛到同一个渲染器,MessageId 与硬编码英文就会正面冲突。值得在 PR 描述或 mod.rs 注释里写一句「本渲染器暂不本地化,收敛 /clear /home 时需要一并决定」。

「留给后续」是否可接受? 分两半:

  • 可接受:/home 和 restore-switch 的错误面比 5 个生命周期命令窄得多,且本 PR 已经把「同一谓词 → 同一解释」的骨架搭好了,后续接入是纯增量(各 ~5 行)。PR 描述也明确说了「判定逻辑本身未改」,范围控制是清醒的。
  • 不可接受:/clear 应该在本次就改。理由是 PR 的问题陈述——「真正挡住的可能是一个跑了五小时的后台任务,但错误信息从不点名它」——/clear 在同一时刻、同一 task_panel 状态下会给出完全一样的无信息拒绝。只改 /new /load /fork /branch /resume 而不改 /clear,等于把同一个 bug 修了 5/6。而且 /clear 就在 commands/groups/core/core.rs,改动量与 /new 相当(一处 CommandResult::error 换成共享渲染器 + 一个 verb 字符串)。建议在本 PR 内补上;若坚持留后,请在 PR 描述里显式列出 /clear 与 restore-switch 两条,而不是只字未提。

事实与推测的边界

  • 已核实(读代码/算数值得出):app.rs 全部行号与谓词列表;is_loading 等字段的 6 处直接读取位置;5 个命令的 transition_blocked() 调用次数;--force 相关旧文案;format_duration/truncate_text 语义;Note cell 走 markdown 渲染器(apply.rs:1516 → history.rs:394)而非 Error cell;render_line_with_links_tagged 是词级折行;新测试行的实际字符宽 82(手算);kill_running_for_session 只杀 ShellStatus::Running;/jobs cancel-all 的实现;fake 字符串与真实算法输出一致(手算);transition_blocked 的 doc 约束。
  • 未能运行验证(本机无 Rust 工具链,cargo/rustc 不存在):没有实际跑 cargo test -p codewhale-tui,所以「新测试在旧代码上编译不过」是从「旧代码无此方法」推得的推断,不是执行结果。同理,渲染破相是通过复刻 wrap_plain_line/render_line_with_links_tagged 的算法手算的,不是实跑 ratatui 出来的。这两点建议由能编译的环境复核。
  • 推测(有依据但未证实):owner_agent_name 在真实场景里会非空——依据是 task_projection.rs:241 从 ShellJobSnapshot 拷贝且 shell.rs:4089-4102 会填充,但我没有找到一条端到端路径证据证明「子代理起的后台 shell」确实会带 owner 进入 task_panel。

署名

🤖 由 SpikeBot 003(ClaudeCode-JP) 生成

… owns

Adversarial review of this PR (SpikeBot 003) left seven findings. The two
majors are the row budget and the test that could not fail:

- A row was bounded by a 60-character summary, and the summary counted
  characters. Rows render into a Note body that is 74 columns wide in an
  80-column terminal, so a CJK summary (60 ideographs = 120 columns) doubled
  the row and five of them wrapped the whole message. The budget is now
  derived from the assembled columns and truncation goes through
  truncate_line_to_width, the existing display-width-aware helper.
- The "stays bounded" assertion allowed 100 characters, eighteen more than a
  row can occupy, and so passed even when the alignment it claimed to protect
  was broken. It now measures display width against the real 74-column body,
  and the summary is CJK in the test so a character-count budget cannot pass.

The remaining findings, also addressed:

- A missing duration renders `-`, the placeholder the task list already uses,
  instead of collapsing the column.
- A row names its owning sub-agent (`(by verifier) ...`): an unclaimed
  background command is the case an operator most needs to identify.
- The panel's `shell: ` prefix is stripped from the command.
- `a turn is still running` becomes `the session is still busy with the
  current turn`: `is_loading` is set by paths with no turn behind them.
- `/new` states the `--force` scope again. The shared tail named the exits
  that clear a blocker, and `--force` is not one of them, so the one place
  that owns the flag has to say what it does.
- `/clear` shows the same blocker list: it consults the same gate, and a
  defect fixed in five of six commands is not fixed.
- Both facets document that the projection is display-only and must not be
  parsed back as data.
@SparkofSpike SparkofSpike reopened this Oct 9, 2026
CI's rustfmt (stable 1.99.0) keeps the four-element chain on one line; the
local 1.98.1 split it, so `cargo fmt --all -- --check` failed on the pushed
head while passing locally. The check follows the same rule everywhere else
in the file, so this is the whole difference.

Not run locally: the divergence is between rustfmt versions, so re-formatting
here would only reproduce the 1.98 shape. CI's formatter is the authority.
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.

1 participant