spec(1.21): T-3 — BlockToolResult + ToolResult + DIAGNOSE.* errcode

[sqlrush]

spec-1.21 T-3 implementation. spec design FROZEN @ APPROVED R2.1 + § 12.1 errata (gate 解锁) in opendbrb.

T-3 范围

• domain/llm.go (D-1): BlockToolResult 兑现 + llm.ToolResult{ToolUseID,Content,IsError} + ContentBlock nil-guard 契约 + NewToolUseBlock/NewToolResultBlock 构造器
• app/diagnose/errors.go (D-5): 5 DIAGNOSE.* errcode (终止类 MAX_TURNS/TOTAL_TIMEOUT/TOOL_UNKNOWN/UNEXPECTED_PAUSE + 回填类 TOOL_TIMEOUT)
• platform/errcode frozen manifest 原子追加 5 行
• 8 新单测 (ordinals 稳序 / nil-guard / IsError flag / 5 errcode 三件套 / 唯一码)

后续

T-4 (anthropic 多轮 toMessages) / T-5 (ToolExecutor + clock/echo) / T-6 (diagnose.Loop) / T-7..T-9 后续 PR 接续。

Spec: spec-1.21-diagnose-loop.md
Spec branch (opendbrb): spec-1.21-diagnose-loop @ c2bfbe2 (§ 12.1 errata)

[sqlrush]

T-10a Path 1/3 — claude-code-reviewer

Reviewer: claude-code-reviewer (independent run)
Scope: spec-1.21 T-3..T-9 cumulative — commits d08e100..aba621a on opendbx PR #81
(internal/{domain/llm,app/diagnose,app/cli/llmapp,bootstrap,platform/errcode}/ +
tests/integration/uitest/diagnoseloop/)
Artifact: this PR comment

Findings:
CRIT: 0
HIGH: 0
MED: 1
LOW: 1
NIT: 3

CRIT

n/a

HIGH

n/a

MED

MED-1: FinishToolUse with empty toolUses commits a degenerate user turn
File: internal/app/diagnose/loop.go:244–304

When a provider emits FinishToolUse but c.ToolUses is empty (a provider protocol anomaly — the Anthropic stream only produces this if toolAcc is empty when message_delta fires, which decodeTools() handles by returning (nil, nil) instead of an error), toolUses remains nil throughout the FinishToolUse branch. The code:

1. Skips validation (zero-iteration range loop → no TOOL_UNKNOWN terminal).
2. Builds asstContent with only a BlockText block (if text was present) and zero BlockToolUse entries.
3. Produces an empty results slice.
4. Commits both turns atomically: an assistant message with no ToolUse blocks and a user message with zero Content blocks.

A user message with an empty Content slice will cause Anthropic to 400 on the very next Stream call. ValidateRequest does not catch this because it only checks len(req.Messages) == 0, not per-message block counts. The OpenAI provider explicitly defends against nil ToolUses on FinishToolUse at the stream level (stream.go:188-192), but the loop itself has no defensive gate.

Fix — add an explicit guard at the top of the FinishToolUse case before building asstContent:

case llm.FinishToolUse:
    if len(toolUses) == 0 {
        _ = emit(ctx, Event{Kind: EventFinish, Turn: turn, Finish: llm.FinishError, Err: llm.ErrDecodeFailed})
        return finalize(result, msgs, llm.FinishError, "", llm.ErrDecodeFailed), llm.ErrDecodeFailed
    }

LOW

LOW-1: handleControl mutates toolUseNames map through shallow copy — documented exception to immutability rule
File: internal/app/cli/llmapp/model.go:281–290

next := *m is a shallow copy; next.toolUseNames holds the same map pointer as m.toolUseNames. No correctness bug today (single-in-flight-submit guarantee), but flagging for consistency. If a future spec relaxes the single-submit constraint this becomes a race.

NIT

NIT-1: doc.go references retired streamStartCmd and stale ToolUses field name
File: internal/app/cli/llmapp/doc.go:13,17-18 — streamStartCmd deleted in T-8 (now loopStartCmd); ToolUses field replaced by ToolUse/ToolResult union.

NIT-2: model.go submit() godoc references retired streamStartCmd
File: internal/app/cli/llmapp/model.go:217 — should say loopStartCmd.

NIT-3: Cleanup() godoc references retired consumeStream goroutine name
File: internal/app/cli/llmapp/model.go:431 — goroutine is now the anonymous one in loopStartCmd.

---

Invariant checklist (all VERIFIED)

1. 9-arm exhaustive FinishReason switch ✓ (no default; spec锁 enum 数=9)
2. Paired-commit atomicity ✓ (TOOL_UNKNOWN / cancel / total-timeout / emitErr 全部不提交)
3. ctx 三层分清楚 ✓ (totalCtx/turnCtx/toolCtx 各自 cancel,classify 看 totalCtx.Err)
4. TOOL_TIMEOUT 回填 vs 终止类 ✓
5. errcode 三件套 (规则 7) ✓ — 5 DIAGNOSE.* + manifest atomic
6. consumeStream/streamStartCmd 死代码删净 ✓
7. toolUsePlaceholder 删净 + integration test 守门 ✓
8. bootstrap registry 注入 (T-9 user 边界) ✓ — happy + cfg-fail 双路径
9. Anthropic/OpenAI 多轮 adapter ✓ (SDK 签名顺序 / nil-guard / OpenAI N-ToolMessage 展开 / JSON args / IsError 前缀 / role mismatch reject)
10. fake script-exhaust → REQUEST_INVALID ✓
11. 规则 13 英文注释 ✓ (新 T-8/T-9 添加无中文;遗留 spec-1.20.2 carry-forward 不动)
12. 规则 21 BlockToolResult switch 覆盖 ✓ (anthropic + openai 全有 default reject)
13. diagnose 包 boundary ✓ (无 UI/block/render/llmapp imports)
14. ScrollbackTypesForTest 测试 seam 仅 integration test 用,godoc 明示 ✓
15. spec-1.20 单 turn regression ✓

---

Verdict: APPROVE-WITH-FIXES

Follow-up (must-fix before merge):

• MED-1: loop.go FinishToolUse case 顶加 len(toolUses) == 0 防御 + 单测.
• NIT-1/2/3: 批量修 doc.go + model.go stale godoc 引用.

Optional (deferred OK):

• LOW-1: Model struct godoc 加 toolUseNames shallow-copy 例外说明.

[sqlrush]

T-10a path 2/3 — codex review on ca5fcc8

External codex session: 019e72e5-7a1c-78f0-bd8b-1c142f5c51eb

Verdict: REQUEST-CHANGES

Summary: core loop and UI wiring work for simple cases, and go test ./... passed during review. codex found 2 P2 findings in the new tool-call UI path.

Findings

1. P2 — ToolUse stays Running after ToolResult arrives

   • File: internal/app/cli/llmapp/model.go:302-303
   • Impact: handleControl appends block.ToolResult, but the earlier block.ToolUse node remains StateRunning. A normal echo/clock round trip leaves a permanent running indicator beside a completed result, misrepresenting completed tools.
   • Suggested fix: when processing EventToolResult, update the matching ToolUse render node to StateResolved or StateError, or render ToolUse in a non-stale state once its result is known.

2. P2 — Multi-tool turn renders results before all tool calls are shown

   • File: internal/app/diagnose/loop.go:305-306
   • Impact: for multiple tool calls in one assistant turn, the loop emits call1/result1/call2/result2. The provider transcript commits all assistant tool_use blocks first, then user tool_result blocks. UI order should reflect that protocol shape; current order is visually/protocol-inconsistent.
   • Suggested fix: emit all EventToolCall events first, then execute tools and emit results in order.

Verification Performed

• Reviewed diff against main.
• Ran go test ./... successfully.
• Checked provider mapping and block reuse paths for Anthropic/OpenAI, diagnose loop, llmapp adapter, bootstrap registry injection, and package boundaries.

No CRIT found. These two P2s should be absorbed before path 3/3 final approval because they affect the user-visible tool-call loop UI.

[sqlrush]

T-10a Path 2/3 R-fix absorb — codex 2 P2 已修

Source: codex review trace #issuecomment-4572770510 — REQUEST-CHANGES, 0 CRIT, 2 P2.
Fix commit: 11470c7 fix(app/diagnose): T-10a R-fix — codex P2-1 + P2-2
CI: ✅ 全 8 required job 绿

P2-1 absorb — ToolUse 完成后 UI 仍 StateRunning

File: internal/app/cli/llmapp/model.go (handleControl ToolResult 分支)

handleControl 原仅 append block.ToolResult,前面的 block.ToolUse 留在 StateRunning — UI 同时显示"工具运行中"和"工具结果",违反 spec-1.9 caller-owns-transition 契约 + spec-1.21 D-6 EventToolResult → 转 Resolved/Error hand-off。

修法:加 transitionToolUseState(sb, id, state) []block.RenderNode 不可变 helper(tail-walk 找 matching ID + 替换值 + 返新 slice 保 shallow-copy backing array 不污染)。handleControl ToolResult 分支先 transition 再 append。IsError → StateError,否则 StateResolved。

Tests(新 2 单测):

• TestModel_LoopTransitionsToolUseState — 成功路径 → StateResolved
• TestModel_LoopTransitionsToolUseToError — IsError=true → StateError

P2-2 absorb — 多工具同 turn UI emit 顺序

File: internal/app/diagnose/loop.go (FinishToolUse case)

原 emit:call1, exec1, result1, call2, exec2, result2 — UI 看起来像 assistant 跨多个想象 turn 一次一工具,不符 wire protocol shape。

正确 shape:

• assistant turn 一次性给所有 tool_use → UI 应先 emit 全部 EventToolCall(Phase 1)
• user turn 一次性回所有 tool_result → UI 应按序 emit EventToolResult(Phase 2)

修法:FinishToolUse case 重组为 2-phase loop。Phase 1 仅 emit call(validation 已先前完成);Phase 2 execute + emit result。cancel / total-timeout 仍在 Phase 2 内 eager terminate,paired-commit 跳过 → 无 orphan tool_use 入 transcript。

Test(新 1 单测):

• TestRun_MultipleTools_EmitPhaseOrder — 2 tools(clock + echo)→ 验事件序为 [call:c1, call:c2, result:c1, result:c2]

兼容性 / 回归

• 1.20 单 turn no-tool 路径:无 tool_use 时 phase 全跳过 — 字节等价不变。
• 已存在 TestRun_ToolRoundTrip_HappyAndPairedCommit / 集成 TestDiagnoseLoop_FullSmoke / 1.20 regression 全过(paired-commit + msgs shape 不变,仅 emit 序变)。
• make gate 全绿;race detector 全绿。

待办

Path 3/3 用户 final verdict 待启。

[sqlrush]

T-10a Path 3/3 R-fix absorb — user HIGH-1 已修

Source: user Path 3/3 verdict — REQUEST-CHANGES, 0 CRIT, 1 HIGH.
Fix commit: 4c91996 fix(config): T-10a R-fix — user HIGH-1 DiagnoseConfig wiring
CI: ✅ 全 8 required job 绿

HIGH-1 absorb — spec-vs-impl 偏离:DiagnoseConfig 未落地

问题证据(用户路径):

• spec-1.21 D-6 明确要求 DiagnoseConfig{MaxTurns, ToolTimeout, TotalTimeout} 全 5-tag,且 per-turn LLM 复用 LLMConfig.RequestTimeout。
• config.Config 旧版无 Diagnose 段(config.go:32)
• newChatModel() 构造 llmapp.Options 只传 Registry,未传 4 timeouts(tui_launcher.go:194)
• diagnose.Loop 因此全走内部默认,ReqTimeout=60s 与 cfg.LLM.RequestTimeout 默认 30s 脱钩(loop.go:46)

影响:用户 llm.request_timeout 配置对 diagnose loop 每轮 LLM 请求无效;diagnose.max_turns/tool_timeout/total_timeout 根本无法配置;DoD § DiagnoseConfig 全 5-tag + 跨字段 validate 未满足。

修法

config:

• internal/platform/config/config.go:新 DiagnoseConfig{MaxTurns int, ToolTimeout time.Duration, TotalTimeout time.Duration},全 5-tag(yaml / json / env / validate;redact 不适用);Config 加 Diagnose DiagnoseConfig 段。
• internal/platform/config/defaults.go:spec-1.21 D-6 默认 16 / 30s / 10min。
• internal/platform/config/validation.go:加 validateCrossField helper + 单一 invariant — ToolTimeout ≤ TotalTimeout(spec D-6 跨字段;per-field tag grammar 不能表达,独立 hook 比扩 walker 干净)。

bootstrap:newChatModel 4 字段全接:

MaxTurns:     cfg.Diagnose.MaxTurns,
ToolTimeout:  cfg.Diagnose.ToolTimeout,
TotalTimeout: cfg.Diagnose.TotalTimeout,
ReqTimeout:   cfg.LLM.RequestTimeout,  // 复用 LLMConfig 不重复字段

Tests(5 新)

测试	守门
TestDefault_DiagnoseValues	defaults 16/30s/10min pin
TestValidate_Diagnose_MaxTurnsBounds	table-driven 0/1/100/101 边界
TestValidate_Diagnose_CrossFieldTimeouts	ToolTimeout > TotalTimeout → reject;== → accept(≤ 契约)
TestApplyENV_DiagnoseOverrides	3 env var(int + 2 duration)走 reflective env walker
TestNewChatModel_PropagatesDiagnoseConfig	bootstrap wiring smoke under env-overridden Diagnose + RequestTimeout

回归

• EnvMap_HasExpectedEntries 加 3 entry。
• 既有 Validate(Default()) 通过(defaults 全 valid + cross-field)。
• 既有 bootstrap / llmapp / diagnose / fake 全 pass。

当前状态

T-10a 三路全闭合:

• Path 1/3 claude → APPROVE-WITH-FIXES → R-fix ca5fcc8 修 MED-1 + NIT-1/2/3
• Path 2/3 codex → REQUEST-CHANGES → R-fix 11470c7 修 P2-1 + P2-2
• Path 3/3 user → REQUEST-CHANGES → R-fix 4c91996 修 HIGH-1 DiagnoseConfig

等用户 final APPROVE → T-10b(spec FROZEN + 双仓 tag v0.39.0-stage1.39 + PR squash merge) + T-10c(roadmap ✅ + CHANGELOG)。