feat(workflows): write a subagent transcript for every dispatch - #8839
feat(workflows): write a subagent transcript for every dispatch#8839qqqys wants to merge 8 commits into
Conversation
Workflow subagents were the only agents in the product that left no record on disk. `attachJsonlTranscriptWriter` had exactly one caller — the Agent tool, foreground and background — while workflow dispatch goes straight to `AgentHeadless.create` / `createAgentHeadless`, so nothing ever landed in `<projectDir>/subagents/<sessionId>/`. Anything that reads that directory to answer "what did this agent actually do" was therefore blind on the workflow path: post-mortem of a failed run, cost accounting, and any check built on tool calls rather than on the agent's own prose. That last distinction is the load-bearing one — an agent that did nothing still writes plausible, specific-sounding text, so its prose cannot tell you it worked; only its tool calls can. Attach the existing writer in the dispatch wrapper. One attach point covers both dispatch paths because `runStallResilient` already builds one `AgentEventEmitter` per attempt and hands the same instance to the fast path and the override path alike. The agent id moves up to that wrapper so the writer can be attached before any agent event can fire, and so the transcript is named for the same id the dispatch reports in its terminal error. Per attempt, not per `agent()` call: a stall retry writes its own transcript rather than appending to the abandoned one, so a retry reads as two agent runs instead of one that behaved strangely. Best-effort by construction — a transcript is audit metadata, so neither the attach nor the cleanup can fail a dispatch that would otherwise have succeeded. The writer opens its fd lazily, so a dispatch that produces no record materializes no file. No new format: this is the same ChatRecord-shaped JSONL the rest of the product already reads, so a future structured execution ledger is additive rather than a second, competing evidence shape. Tests pin the record shape a consumer needs, not merely that a file appears: the launch prompt in the first record, functionCall / functionResponse pairable by callId with args intact, one file per retry attempt, and a dispatch that still succeeds when the transcript cannot be written. Part of QwenLM#8769. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e8ad1e6 to
5ff4836
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@qqqyc Stopping at the template gate before code review — the prose is thorough, but the body follows a different structure than the PR template, and three required sections are missing:
## Reviewer Test Plan— the## Testscontent belongs here.### How to verify: the four record-shape cases plus the suite commands you already list.### Evidence (Before & After): the natural evidence is a workflow run before/after showingsubagents/<sessionId>/agent-<id>.jsonlappearing with the launch prompt and pairable tool calls; if you'd rather treat this as non-user-visible, the template acceptsN/Athere.### Tested on: the body says the suites passed locally but not on which OS — fill in the matrix.## Risk & Scope— main risk or tradeoff (a file per attempt, attach/cleanup swallowing errors by design, etc.), what was not validated, breaking changes (presumably none).## Linked Issues— "Part of #8769" is currently in the prose; give it the dedicated section so the linkage is explicit.
Also rename ## What → ## What this PR does and ## Why → ## Why it's needed to match the template's headings; the extra sections (How, No new format) can stay. This is a restructure, not a rewrite — the content is already there.
Once the body follows the template, re-run with @qwen-code /triage and this picks up at code review.
中文说明
@qqqyc 在进入代码审查前先停在模板门禁——正文写得很详尽,但结构与 PR 模板不一致,缺少三个必需小节:
## Reviewer Test Plan——## Tests的内容应放在这里。### How to verify:你那四个记录形状用例加上已列出的套件命令。### Evidence (Before & After):最自然的证据是一次 workflow 运行的前后对比——subagents/<sessionId>/agent-<id>.jsonl从不出现到出现,且带有 launch prompt 与可按 callId 配对的工具调用;如果你认为这属于非用户可见改动,模板也接受在那里写N/A。### Tested on:正文说套件在本地通过,但没说是哪个操作系统——请填平台矩阵。## Risk & Scope—— 主要风险/取舍(每次 attempt 一个文件、挂载/清理按设计吞错等)、未验证的部分、破坏性变更(应该没有)。## Linked Issues—— "Part of #8769" 目前写在正文里,请放进专门小节,让关联显式化。
另外把 ## What 改为 ## What this PR does、## Why 改为 ## Why it's needed,与模板标题一致;其余小节(How、No new format)可以保留。这只是结构调整,不是重写——内容都已经有了。
正文按模板补齐后,用 @qwen-code /triage 重跑,即可进入代码审查。
— Qwen Code · qwen3.8-max
|
已修复 + 验证证据:PR 描述已按模板补齐 Reviewer Test Plan、Risk & Scope、Linked Issues;保留原有变更说明,无代码改动。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: I did not fully read runOverridePath's schema-mode early-return block (workflow-orchestrator.ts around lines 910-1000) — but detach happens in the attempt callb…; You are review agent reverse-audit — Reverse audit agen...: did not read workflow-orchestrator.ts ~910–1000 (schema-mode early-return block inside runOverridePath ); detach runs in the attempt callback's finally , whic….
中文说明
已审查。 建议见行内评论。 未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:I did not fully read runOverridePath's schema-mode early-return block (workflow-orchestrator.ts around lines 910-1000) — but detach happens in the attempt callb…;You are review agent reverse-audit — Reverse audit agen...:did not read workflow-orchestrator.ts ~910–1000 (schema-mode early-return block inside runOverridePath ); detach runs in the attempt callback's finally , whic…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| * Workflow subagents were the only agents in the product that left no record. | ||
| * `attachJsonlTranscriptWriter` has exactly one other caller — the Agent tool | ||
| * (foreground and background) — while workflow dispatch goes straight to |
There was a problem hiding this comment.
[Suggestion] The new doc comment's caller census for attachJsonlTranscriptWriter is inaccurate: there are three other production call sites, not one — tools/agent/agent.ts:3263 (background), tools/agent/agent.ts:4043 (foreground), and agents/background-agent-resume.ts:1032 (resume re-attach). The omitted caller is the load-bearing one: it is the only consumer of the writer's fragile resume options (appendToExisting: true, initialParentUuid), which continue a transcript UUID chain for transcript-first resume. — Failure scenario: a maintainer changing the writer's options/append semantics trusts this census, audits only agent.ts, and misses background-agent-resume.ts → transcript-chain continuation on the resume path breaks silently, with no compiler or test signal at the writer itself.
| * Workflow subagents were the only agents in the product that left no record. | |
| * `attachJsonlTranscriptWriter` has exactly one other caller — the Agent tool | |
| * (foreground and background) — while workflow dispatch goes straight to | |
| * Workflow subagents were the only agents in the product that left no record. | |
| * `attachJsonlTranscriptWriter` has three other callers — the Agent tool | |
| * (foreground and background) and the background-agent resume path — while | |
| * workflow dispatch goes straight to |
中文说明
新增文档注释中对 attachJsonlTranscriptWriter 调用方的清点不准确:除本处外实际还有三个生产调用点 —— tools/agent/agent.ts:3263(后台)、tools/agent/agent.ts:4043(前台)与 agents/background-agent-resume.ts:1032(resume 重挂)。被漏掉的这个最关键:它是 writer 脆弱的 resume 选项(appendToExisting: true、initialParentUuid,用于接续 transcript UUID 链)的唯一消费者。失败场景:未来修改 writer 选项/追加语义的维护者信任这段清点、只审计 agent.ts 而漏掉 background-agent-resume.ts → 悄悄破坏 resume 的转录链接续,且编译器与测试都不会报警。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| agentId: workflowAgentId, | ||
| agentName: label || agentType || 'workflow-agent', | ||
| sessionId, |
There was a problem hiding this comment.
[Suggestion] Of the three agentName fallback branches, only the label branch is pinned by the new tests — the precedence-swap mutant (agentType || label || 'workflow-agent') survives all four (verified by a mutation run). All four tests dispatch with label only, and every override-path test uses a config lacking getProjectRoot (attach throws there and is swallowed), so the fallback chain is never exercised on that path either. — Failure scenario: a future refactor swapping the precedence ships green → agent('x', {label: 'reviewer', agentType: 'Explore'}) records agentName: 'Explore' instead of 'reviewer', breaking the transcript↔script-line name matching the adjacent comment says this ordering exists to guarantee.
Suggested fix (new test case, not a change to this line): dispatch with BOTH options set — await dispatch('…', { label: 'reviewer', agentType: 'Explore' }) — and assert the first record's agentName === 'reviewer'; this is the case that kills the mutant. A neither-set case pinning 'workflow-agent' is also worth adding. (Verified: the assertion fails against the mutant and passes on the current code.)
中文说明
agentName 的三个回退分支中只有 label 分支被新测试钉住 —— 互换优先级的突变体(agentType || label || 'workflow-agent')能让四条测试全部通过(已实际跑突变体验证)。四条测试都只带 label;而所有 override 路径测试用的 config 缺少 getProjectRoot(挂载在那里抛错被吞),因此那条链也从未被真正执行。失败场景:未来互换优先级的重构可以绿灯上线 → agent('x', {label: 'reviewer', agentType: 'Explore'}) 记下 agentName: 'Explore' 而不是 'reviewer',破坏相邻注释声称此顺序所要保证的 transcript↔脚本行名称对应。建议修复(新增测试,而非改这一行):同时设置两个选项派发 —— await dispatch('…', { label: 'reviewer', agentType: 'Explore' }) —— 并断言首条记录的 agentName === 'reviewer';这正是能杀死该突变体的用例。也值得补一个两者都未设置、钉住 'workflow-agent' 的用例。(已验证:该断言对突变体失败、对当前代码通过。)
— qwen3.8-max via Qwen Code /review (v0.21.8)
| cwd: projectRoot, | ||
| version: config.getCliVersion() || 'unknown', | ||
| gitBranch: getCachedGitBranch(projectRoot), |
There was a problem hiding this comment.
[Suggestion] No test pins the cwd, version, or gitBranch annotations — deleting the cwd and version lines is a surviving mutant (verified). — Failure scenario: a refactor of attachDispatchTranscript (e.g. rebuilding the options object) drops these fields → every workflow transcript silently loses its resume context (cwd), compatibility-tracking version, and branch attribution — the audit metadata this PR's own doc comment justifies the feature with — and nothing goes red.
Suggested fix: in the first transcript test add expect(first['cwd']).toBe(projectDir) and expect(first['version']).toBe('0.0.0-test') — both values are already fixed by transcriptConfig() (probe-verified: fails against the deletion mutant, passes on the current code). Pinning gitBranch would additionally need a repo fixture or a mocked getGitBranch, since the mkdtemp fixture is not a git repo.
中文说明
没有任何测试钉住 cwd、version、gitBranch 标注 —— 删掉 cwd 与 version 两行后突变体依然存活(已验证)。失败场景:对 attachDispatchTranscript 的重构(例如重建 options 对象)丢掉这些字段 → 每份 workflow transcript 都会悄悄失去恢复上下文(cwd)、兼容性跟踪版本与分支归属 —— 也就是本 PR 文档注释用来论证该功能价值的审计元数据 —— 而测试套件一片绿灯。建议修复:在第一条 transcript 测试中补 expect(first['cwd']).toBe(projectDir) 与 expect(first['version']).toBe('0.0.0-test') —— 两个值都已由 transcriptConfig() 固定(探针已验证:对删除突变体失败、对当前代码通过)。钉住 gitBranch 还需要仓库 fixture 或 mock 掉 getGitBranch,因为 mkdtemp fixture 不是 git 仓库。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| * One attach point covers both dispatch paths because `runStallResilient` | ||
| * builds one `AgentEventEmitter` per attempt and hands the same instance to | ||
| * the fast path and the override path alike. |
There was a problem hiding this comment.
[Suggestion] The claim that one attach point covers both dispatch paths is untested: all four transcript tests take the fast path, and in every override-path test attachDispatchTranscript throws at config.getProjectRoot() (absent from fakeConfigWithMgr) and is swallowed — so no override-path dispatch ever attaches the writer in any test. The current code is correct (the emitter is forwarded into createAgentHeadless); this is a regression-protection gap, not a live bug. — Failure scenario: a future change that stops forwarding the wrapper's emitter on the non-schema override path, or makes schema mode construct its own emitter → transcripts for agent({agentType/model/schema/isolation}) silently degrade to a prompt-only record with no tool calls — exactly the blindness this PR exists to remove — while the suite stays green.
Suggested fix: add one transcript test whose config merges the subagent-manager stub with the transcript methods (getSessionId/getProjectRoot/storage.getProjectDir/getCliVersion), dispatching { agentType: 'Explore', label: 'e' } and asserting the file contains the launch-prompt user record plus a functionCall/tool_result pair emitted through the passed emitter.
中文说明
「一个挂载点覆盖两条派发路径」这一说法没有测试支撑:四条 transcript 测试全部走快路径,而所有 override 路径测试里 attachDispatchTranscript 都在 config.getProjectRoot() 处抛错(fakeConfigWithMgr 没有该方法)并被吞掉 —— 因此没有任何测试中 override 路径的 dispatch 真正挂上过 writer。当前代码是正确的(emitter 确实被转发进 createAgentHeadless);这里缺的是回归保护。失败场景:未来某个改动不再在非 schema override 路径上转发包装层的 emitter,或让 schema 模式自造 emitter → agent({agentType/model/schema/isolation}) 的 transcript 会悄悄退化为只有 launch prompt、没有工具调用的记录 —— 恰是本 PR 要消除的盲区 —— 而套件依旧绿灯。建议修复:补一条 transcript 测试,config 合并 subagent-manager stub 与 transcript 方法(getSessionId/getProjectRoot/storage.getProjectDir/getCliVersion),以 { agentType: 'Explore', label: 'e' } 派发,并断言文件中包含 launch prompt 的 user 记录以及经由所传 emitter 发出的可配对 functionCall/tool_result。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // Minted here rather than inside the dispatch so this attempt's | ||
| // transcript is named for the same id the dispatch reports in its | ||
| // terminal error — and so the writer is attached to the emitter | ||
| // BEFORE any agent event can fire on it. | ||
| const workflowAgentId = `workflow-agent-${randomBytes(8).toString('hex')}`; |
There was a problem hiding this comment.
[Suggestion] The id linkage this comment promises — "the transcript is named for the same id the dispatch reports in its terminal error" — does not exist for the stall-abandoned outcome, the one failure mode that produces multiple transcripts. runStallResilient swallows each attempt's id-carrying Workflow subagent <id> did not complete error and finally throws agent "<label>" stalled on all N attempts… with no id and no cause (probe-verified: zero ids reachable from the terminal error chain; three unpairable files left behind). — Failure scenario: two concurrent unlabeled dispatches (label omitted → both fall back to 'workflow-agent') that both stall out → six files and two byte-identical terminal errors, with nothing to pair an error with its records short of opening every file.
Suggested fix: collect the per-attempt ids (e.g. append to a closure-captured array inside the attempt fn) and include them — or the session transcript directory path — in the abandoned error. A minimal { cause: err } on the runStallResilient throw surfaces only the last attempt's id (probe-verified), so collecting all ids is the complete fix.
中文说明
这段注释承诺的 id 关联 ——「transcript 以 dispatch 在其终态错误中报出的同一个 id 命名」—— 在 stall 放弃这一结局上并不存在,而那恰恰是唯一会产生多份 transcript 的失败模式。runStallResilient 会吞掉每次 attempt 携带 id 的 Workflow subagent <id> did not complete 错误,最终抛出 agent "<label>" stalled on all N attempts… —— 既无 id 也无 cause(已用探针验证:终态错误链上可达的 id 数为零,三份文件无法与错误配对)。失败场景:两个并发且未带 label 的 dispatch(省略 label → 都回退为 'workflow-agent')同时 stall 到底 → 会话目录里六份文件、两条字节相同的终态错误,除了逐个打开文件外没有任何线索能把错误与其记录配对。建议修复:收集每次 attempt 的 id(例如在 attempt 函数内向闭包捕获的数组追加),并在放弃错误中带上它们 —— 或会话 transcript 目录路径。给 runStallResilient 的抛出加最小 { cause: err } 只能带出最后一次 attempt 的 id(已验证),收集全部 id 才是完整修复。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| ), | ||
| { | ||
| agentId: workflowAgentId, | ||
| agentName: label || agentType || 'workflow-agent', |
There was a problem hiding this comment.
[Suggestion] When label is absent, this records the raw model-authored opts.agentType string, but subagent resolution is case-insensitive (verified through findSubagentByName → loadSubagent lowercased matching at every level, including builtins) — so the recorded agentName can diverge from the canonical subagent name. The Agent tool records the canonical subagentConfig.name at both of its attach sites; the fast path is unaffected. — Failure scenario: agent('review X', {agentType: 'explore'}) legitimately resolves to the builtin whose canonical name is 'Explore'; the workflow transcript carries agentName: 'explore' while an Agent-tool run of the identical definition records 'Explore' in the same directory → a post-mortem or cost-accounting reader that joins or groups by agentName silently splits or misses the workflow runs.
Suggested fix: record the canonical name — thread the resolved SubagentConfig's name back from runOverridePath to the transcript attach, or normalize agentType via subagent resolution before recording, falling back to the raw string only when resolution fails.
中文说明
当 label 缺省时,这里记录的是模型原始写下的 opts.agentType 字符串,但 subagent 解析是大小写不敏感的(已沿 findSubagentByName → loadSubagent 的逐层小写匹配验证,含内置 agent)—— 因此记录下的 agentName 可能与规范化 subagent 名不一致。Agent 工具在其两个挂载点记录的都是规范的 subagentConfig.name;快路径不受影响。失败场景:agent('review X', {agentType: 'explore'}) 能正常解析到规范名为 'Explore' 的内置 agent;workflow transcript 记下 agentName: 'explore',而同一定义的 Agent 工具运行在同一目录记下 'Explore' → 按 agentName 关联或分组的事后排查/成本统计会悄悄把 workflow 运行拆散或漏掉。建议修复:记录规范名 —— 把 runOverridePath 中解析出的 SubagentConfig 名回传给 transcript 挂载,或在记录前经 subagent 解析规范化 agentType,仅在解析失败时回退为原始字符串。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
…pe (QwenLM#8839) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Round summary — PR #8839 review feedbackCommit: Feedback dispositions1. CHANGES_REQUESTED review — PR body does not follow the template (template gate). 2. 3. 4. 5. 6. 7. Also checked, no action neededThe wrapper review's reverse-audit note said the schema-mode early-return block in Changed files
VerificationCommands actually run (after the final code state):
No settings source changed ( 中文说明本轮总结 — PR #8839 审查反馈提交: 反馈处理结果1. CHANGES_REQUESTED 审查 — PR 正文不符合模板(模板门禁)。 2. 3. 4. 5. 6. 7. 另外核查过、无需处理的外层审查的 reverse-audit 备注说未完整阅读 变更文件
验证实际执行的命令(针对最终代码状态):
未改动 settings 源(无需 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Not explored to full depth (tool budget reached): Context: this PR attaches the harness's existing per-suba...: could not execute the test suite — the review worktree has no node_modules and I did not run a full install; the mutation analysis above is from code reading,…; Context: this PR attaches the harness's existing per-suba...: (none).
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 未探索到全部深度(达到工具调用预算):Context: this PR attaches the harness's existing per-suba...:could not execute the test suite — the review worktree has no node_modules and I did not run a full install; the mutation analysis above is from code reading,…;Context: this PR attaches the harness's existing per-suba...:(none)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const agentName = label | ||
| ? label | ||
| : agentType | ||
| ? await canonicalSubagentName(config, agentType) | ||
| : 'workflow-agent'; |
There was a problem hiding this comment.
[Suggestion] R2-1: Duplicate subagent-name resolution per attempt — canonicalSubagentName (called here for unlabeled agentType dispatches) runs findSubagentByName(agentType), and runOverridePath re-runs the same lookup later in the same attempt (~line 759). Each lookup does a fresh readdir of the project and user agent-definition directories plus a read+frontmatter-parse of every .md file — subagentsCache only serves the list-all flow, not lookup-by-name (probe-verified: editing a definition between two calls changed the second result; each of two consecutive calls paid 2 readdirs + 2 file parses). — Failure scenario: every unlabeled-agentType agent() call pays two full definition-directory scans per attempt, up to 6 across stall retries, scaling with workflow fan-out and subagent count — exactly the per-launch duplicate-I/O pattern the PR's own getCachedGitBranch export comment says it exists to avoid. — Suggested fix: resolve once per attempt and share the result — thread the resolved SubagentConfig into runOverridePath (keeping its not-found throw), or memoize canonical-name resolution per process like gitBranchCache.
中文说明
每次 attempt 重复解析子 agent 名称——canonicalSubagentName(在此处为无 label 的 agentType dispatch 调用)会执行 findSubagentByName(agentType),而 runOverridePath 在同一次 attempt 稍后(约第 759 行)会再执行一次相同的查找。每次查找都会对 project 与 user 两级 agent 定义目录做一次全新的 readdir,并读取+解析每个 .md 文件的 frontmatter——subagentsCache 只服务于"列出全部"流程,不服务于按名查找(已用探针验证:在两次调用之间修改定义文件会改变第二次结果;连续两次调用各自付出 2 次 readdir + 2 次文件解析)。— 失败场景:每个无 label 的 agentType agent() 调用在每次 attempt 都要付出两次完整的定义目录扫描,stall 重试时最多 6 次,且随 workflow 扇出规模与子 agent 数量增长——这正是本 PR 自己的 getCachedGitBranch 导出注释声称要避免的"每次启动重复 I/O"模式。— 建议修复:每次 attempt 只解析一次并共享结果——把解析得到的 SubagentConfig 传入 runOverridePath(保留其未找到时抛错),或像 gitBranchCache 一样对规范化名称解析做进程级记忆化。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const label = typeof opts.label === 'string' ? opts.label.trim() : ''; | ||
| const agentType = | ||
| typeof opts.agentType === 'string' ? opts.agentType.trim() : ''; |
There was a problem hiding this comment.
[Suggestion] R2-2: One-sided agentType trim — this function trims agentType before canonical-name resolution, but runOverridePath (~line 762) resolves the raw untrimmed opts.agentType, and findSubagentByNameAtLevel matches by toLowerCase() only — no trim at any level of the chain. — Failure scenario (probe-verified): model-authored agent('…', { agentType: ' Explore ' }) — the attach trims to 'Explore', resolves, and materializes this attempt's transcript with agentName: 'Explore' plus the launch prompt (attach runs before runSingleDispatch); runOverridePath then resolves the raw ' Explore ', gets null, and throws agent type ' Explore ' not found — the agent never launched, but the audit directory carries a record that reads as a canonical Explore run, and any consumer joining transcripts on agentName misattributes it. A whitespace-only ' ' agentType orphans a record the same way. — Suggested fix: trim consistently at both resolution sites (also in runOverridePath) so transcript identity and launch outcome never diverge — dropping the trim here only removes the canonical-name misattribution; it still orphans a transcript for the never-launched dispatch.
中文说明
单侧 agentType trim——此函数在规范化名称解析前先 trim agentType,但 runOverridePath(约第 762 行)解析的是未 trim 的原始 opts.agentType,而 findSubagentByNameAtLevel 只做 toLowerCase() 匹配——整条解析链上任何一级都没有 trim。— 失败场景(已用探针验证):模型生成的 agent('…', { agentType: ' Explore ' })——attach 侧 trim 成 'Explore',解析成功,并在 runSingleDispatch 之前就物化了本次 attempt 的 transcript(agentName: 'Explore' + launch prompt);随后 runOverridePath 解析原始的 ' Explore ',得到 null,抛出 agent type ' Explore ' not found——agent 从未启动,但审计目录里却多出一条读起来像 Explore 正常运行过的记录,任何按 agentName 关联 transcript 的消费者都会误归因。纯空白的 ' ' agentType 同样会留下孤儿记录。— 建议修复:在两处解析点一致地 trim(runOverridePath 也 trim),使 transcript 身份与启动结果永不背离——只删掉这里的 trim 仅能消除规范化名称的误归因,仍会为从未启动的 dispatch 留下孤儿 transcript。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| .findSubagentByName(agentType); | ||
| return resolved?.name || agentType; |
There was a problem hiding this comment.
[Suggestion] R2-3: Both fallback branches of canonicalSubagentName are untested — only the resolution-success path is exercised. Mutants verified to survive the full 142-test suite: resolved?.name || agentType → resolved?.name || 'workflow-agent', and deleting the try/catch entirely. — Failure scenario: a dispatch whose agentType is not a registered subagent (a model-authored typo or custom name) should still record the raw agentType as the transcript's agentName — the first mutant ships green and would silently record the wrong name; deleting the try/catch ships green too, and would let the manager getter throwing escalate into the outer attach catch, swallowing the whole transcript for that dispatch. — Suggested fix: add two cases to the new describe — dispatch { agentType: 'NoSuchAgent' } against overrideTranscriptConfig (resolveName → null) asserting records[0].agentName === 'NoSuchAgent'; and dispatch with an agentType against the manager-less transcriptConfig() asserting a transcript still lands with the raw name.
中文说明
canonicalSubagentName 的两个回退分支都没有测试——只有解析成功路径被覆盖。已验证两个变异体在整个 142 用例套件下存活:resolved?.name || agentType → resolved?.name || 'workflow-agent',以及整体删除 try/catch。— 失败场景:agentType 不是已注册子 agent 的 dispatch(模型生成的拼写错误或自定义名称)本应仍把原始 agentType 记为 transcript 的 agentName——第一个变异体可以绿着上线,会悄悄记录错误的名称;删除 try/catch 同样能绿着上线,会让 manager getter 抛错升级为外层 attach catch,从而吞掉该 dispatch 的整份 transcript。— 建议修复:在新的 describe 中补两个用例——用 overrideTranscriptConfig(resolveName → null)dispatch { agentType: 'NoSuchAgent' },断言 records[0].agentName === 'NoSuchAgent';以及用无 manager 的 transcriptConfig() dispatch 一个 agentType,断言 transcript 仍以原始名称落盘。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| } finally { | ||
| detachTranscript(); | ||
| cleanupApprovalBridge?.(); |
There was a problem hiding this comment.
[Suggestion] R2-4: detachTranscript() is unpinned — removing this line makes no test fail (verified by mutation run): all test writes go through synchronous fs.writeSync before the assertion reads the file, and no test emits STREAM_TEXT. — Failure scenario: cleanup (returned by attachJsonlTranscriptWriter) is the only code path that closes the writer's append-mode fd and removes the <file>.stream sidecar. If this call regresses, every materialized transcript leaks one fd until process exit — workflow dispatches are numerous and CLI/daemon processes are long-lived — and streaming dispatches leave stale .stream files, with no red test. — Suggested fix: assert an observable detach effect: have nextExecuteHook emit a STREAM_TEXT event, then after dispatch resolves assert the .stream sidecar is gone; or capture the emitter in the hook, emit TOOL_CALL after dispatch returns, and assert the transcript did not grow.
中文说明
detachTranscript() 未被测试钉住——删除这一行不会让任何测试失败(已用变异运行验证):所有测试写入都是在断言读文件之前经由同步 fs.writeSync 完成的,且没有测试发出 STREAM_TEXT。— 失败场景:cleanup(attachJsonlTranscriptWriter 的返回值)是唯一关闭 writer append 模式 fd 并删除 <file>.stream 附属文件的代码路径。如果该调用回归,每个已物化的 transcript 都会泄漏一个 fd 直到进程退出——workflow dispatch 数量多,而 CLI/daemon 进程寿命长——流式 dispatch 还会留下陈旧的 .stream 文件,且没有任何测试变红。— 建议修复:断言一个可观察的 detach 效果:让 nextExecuteHook 发出一个 STREAM_TEXT 事件,在 dispatch 完成后断言 .stream 附属文件已消失;或在 hook 中捕获 emitter,在 dispatch 返回后再发出 TOOL_CALL,断言 transcript 没有增长。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // Minted here rather than inside the dispatch so this attempt's | ||
| // transcript is named for the same id the dispatch reports in its | ||
| // terminal error — when the stall wrapper abandons, its error names |
There was a problem hiding this comment.
[Suggestion] R2-5: The id linkage this comment promises does not hold for schema-mode content failures — both schema terminal errors (~line 1014, 'subagent completed without calling StructuredOutput (after 2 in-conversation nudges).', and ~1018-1019, the plain-text variant) carry no attempt id, label, or transcript path, even though workflowAgentId is in scope. Every sibling terminal error names its id(s): fast-path ~646, schema TIMEOUT/MAX_TURNS/ERROR ~1000, override non-GOAL ~1027, and the stall-abandoned error names all attempt ids. — Failure scenario: a parallel()/pipeline() of schema-mode dispatches fails content validation; runStallResilient re-throws non-stall errors raw and WorkflowExecutionError carries only the message, so nothing identifies the failed attempt; with one transcript per dispatch in the session directory, an operator cannot pair the failure to its record except by mtime guesswork — ambiguous for concurrent dispatches landing in the same second. — Suggested fix: the error text is deliberately upstream-verbatim for user-visible parity, so keep it; emit debugLogger.warn naming workflowAgentId and the transcript path immediately before those two throws, or soften this comment to state which terminal errors carry the id.
中文说明
这条注释承诺的 id 关联在 schema 模式内容失败上不成立——两个 schema 终态错误(约第 1014 行 'subagent completed without calling StructuredOutput (after 2 in-conversation nudges).' 与约第 1018-1019 行的纯文本变体)都不携带 attempt id、label 或 transcript 路径,尽管 workflowAgentId 就在作用域内。其余所有同级终态错误都带有 id:fast-path 约 646 行、schema TIMEOUT/MAX_TURNS/ERROR 约 1000 行、override 非 GOAL 约 1027 行,stall-abandoned 错误还会列出全部 attempt id。— 失败场景:parallel()/pipeline() 中的一组 schema 模式 dispatch 内容校验失败;runStallResilient 原样重抛非 stall 错误,WorkflowExecutionError 只携带消息文本,因此没有任何信息能标识失败的 attempt;会话目录里每个 dispatch 各有一份 transcript,运维者只能靠 mtime 猜测来配对失败与记录——同一秒内并发 dispatch 时无法区分。— 建议修复:错误文本是为了与上游保持用户可见的一致性而刻意逐字保留的,保持不变即可;在那两处 throw 之前紧邻着发一条 debugLogger.warn,写明 workflowAgentId 与 transcript 路径,或者把这条注释弱化为只声明哪些终态错误携带 id。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| label: typeof opts.label === 'string' ? opts.label : undefined, | ||
| abandonedDetail: () => `Attempt ids: ${attemptIds.join(', ')}.`, |
There was a problem hiding this comment.
[Suggestion] R2-6: The attempt-id pairing this PR adds only fires on the all-stall abandoned error — a mixed retry leaves an orphan. Attempt 1 stalls (watchdog aborts it; its transcript is written and detached), attempt 2 fails non-stall (MAX_TURNS/TIMEOUT/ERROR, or agent type '…' not found): runStallResilient reaches the bare throw err (~workflow-stall.ts:272), which rethrows the raw error naming only attempt 2's id; abandonedDetail is never consulted. — Failure scenario (probe-verified): after stall-then-MAX_TURNS, two transcripts exist on disk and the terminal error names only one — attempt 1's file is an unpairable orphan. This also falsifies the new test comment's claim that the stall-abandoned outcome is "the one failure mode that leaves multiple transcripts behind". The parent-abort rethrow above the bare rethrow can orphan earlier attempts the same way. — Suggested fix: append the same detail on the non-stall rethrow when more than one attempt ran (and on the parent-abort rethrow); add a stall-then-MAX_TURNS test asserting every attempt id is named.
中文说明
本 PR 新增的 attempt-id 关联只在"全部 stall 后 abandoned"这一种错误上生效——混合重试会留下孤儿记录。attempt 1 stall(watchdog 中止它,其 transcript 已写入并 detach),attempt 2 以非 stall 方式失败(MAX_TURNS/TIMEOUT/ERROR,或 agent type '…' not found):runStallResilient 走到裸的 throw err(约 workflow-stall.ts:272),原样重抛的错误只带 attempt 2 的 id;abandonedDetail 根本不会被查询。— 失败场景(已用探针验证):先 stall 再 MAX_TURNS 之后,磁盘上有两份 transcript,而终态错误只点名其中一份——attempt 1 的文件成为无法配对的孤儿。这也证伪了新测试注释中"stall-abandoned 是唯一会留下多份 transcript 的失败模式"的说法。裸重抛上方的 parent-abort 重抛同样会让先前 attempt 的记录成为孤儿。— 建议修复:在运行过多次 attempt 时的非 stall 重抛上也追加同样的明细(parent-abort 重抛同理);补一个 stall 后接 MAX_TURNS 的测试,断言每个 attempt id 都被点名。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| * writer itself opens its fd lazily, so a dispatch that produces no record | ||
| * materializes no file. |
There was a problem hiding this comment.
[Suggestion] R2-7: This sentence is false for every practical dispatch at this call site — initialUserPrompt is always passed, and attachJsonlTranscriptWriter seeds it synchronously at attach time (recordUserMessage → append → ensureOpen → fs.openSync), so the file exists with one record before any agent exists. The PR description makes the same claim ("an attempt that produces no records leaves no empty file"). Only an empty-string prompt would make the sentence true. — Failure scenario: a maintainer reasoning about the seed-only orphan records this PR creates for pre-launch failures (the agent type not found class) reads "a dispatch that produces no record materializes no file" and concludes that class cannot exist — the exact inverse of the behavior the PR's own first test pins ("writes one transcript per dispatch, opening with the launch prompt"); the stall-retry test's per-attempt files contain only the seeded record. — Suggested fix:
| * writer itself opens its fd lazily, so a dispatch that produces no record | |
| * materializes no file. | |
| * writer itself opens its fd lazily, but the seeded launch-prompt record is | |
| * written at attach time, so every dispatched prompt materializes its file | |
| * immediately — including dispatches that fail before the agent launches. |
中文说明
这句话在此调用点对任何实际 dispatch 都不成立——这里总是传入 initialUserPrompt,而 attachJsonlTranscriptWriter 在 attach 时同步写入这条种子记录(recordUserMessage → append → ensureOpen → fs.openSync),因此在任何 agent 存在之前文件就已带一条记录落盘。PR 描述也有同样的说法("没有产生记录的 attempt 不会留下空文件")。只有空字符串 prompt 才能让这句话成立。— 失败场景:维护者在推断本 PR 为启动前失败(agent type not found 一类)造成的"仅含种子记录的孤儿文件"时,读到"不产生记录的 dispatch 不会物化文件",会得出这类文件不可能存在的结论——与本 PR 自己的第一个测试所钉住的行为("每次 dispatch 写一份 transcript,以 launch prompt 开头")恰好相反;stall 重试测试里每个 attempt 的文件就只含种子记录。— 建议修复:见上方 suggestion 代码块。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // The audit annotations the feature exists to carry — un-pinned, a | ||
| // rebuild of the attach options could drop them without a red test. |
There was a problem hiding this comment.
[Suggestion] R1-3: Still stands for gitBranch (round-1 finding; cwd and version were pinned at this block since). No test exercises gitBranch with a value: every transcript test runs in a non-git mkdtemp dir under os.tmpdir(), so git rev-parse --abbrev-ref HEAD fails, getCachedGitBranch returns undefined, and JSON.stringify omits the key. Deleting the gitBranch: option line (workflow-orchestrator.ts:513) survives the full suite (verified by mutation run). — Failure scenario: real workflow dispatches run inside git checkouts, where baseFields writes gitBranch onto every record; if a future change drops the option line or re-privatizes the cache, every workflow transcript silently loses the branch annotation and any consumer joining on gitBranch reads undefined — the suite stays green. — Suggested fix: make the temp projectDir a repo with one commit (git init alone is not enough — rev-parse --abbrev-ref HEAD fails on an unborn HEAD), then assert first['gitBranch'] equals the fixture branch; unique mkdtemp paths keep the module-level gitBranchCache from cross-contaminating tests.
中文说明
对 gitBranch 仍然成立(第 1 轮发现;cwd 与 version 此后已在此代码块钉住)。没有测试以真实值验证 gitBranch:所有 transcript 测试都运行在 os.tmpdir() 下的非 git mkdtemp 目录里,git rev-parse --abbrev-ref HEAD 会失败,getCachedGitBranch 返回 undefined,JSON.stringify 直接省略该键。删除 gitBranch: 选项行(workflow-orchestrator.ts:513)在整个套件下依然存活(已用变异运行验证)。— 失败场景:真实 workflow dispatch 运行在 git 检出内,baseFields 会把 gitBranch 写进每条记录;若未来改动删掉该选项行或把缓存重新私有化,所有 workflow transcript 都会悄悄丢失分支标注,任何按 gitBranch 关联的消费者读到 undefined——套件仍是绿的。— 建议修复:把临时 projectDir 变成带一个 commit 的仓库(仅 git init 不够——未诞生 HEAD 时 rev-parse --abbrev-ref HEAD 会失败),然后断言 first['gitBranch'] 等于夹具分支;唯一的 mkdtemp 路径可避免模块级 gitBranchCache 在测试间交叉污染。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31391895896 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31394772225 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. 3 of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those. A human should investigate the sandbox image and runner docker daemon, then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31397491013 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停:本 PR 达到轮次上限(100/100)。评论 |
|
@qwen-code /takeover stop |
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
…entType resolution (QwenLM#8839)
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8839 (round 2 findings)All eight inline findings from the automated reviewer were verified against the code and addressed in code. Commit: Findings and dispositionsR2-1 — Duplicate subagent-name resolution per attempt (workflow-orchestrator.ts:499) — FIXEDVerified: the transcript attach resolved the agentType through R2-2 — One-sided
|
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Autofix round outcome: failed CI check attributed to a repo-wide runner instability wave — no PR defect found, no change committedFeedback item
What the evidence showsVia the public GitHub API (step-level job data, no credentials needed):
Run log: https://github.com/QwenLM/qwen-code/actions/runs/31493849475 🧠 Handled by Qwen Code · model/模型 |
Superseded by head 6b7bc90; no current unresolved Critical remains. Clearing stale CHANGES_REQUESTED state at the PR author request.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): Context: PR #8839 attaches the harness's existing per-sub...: none — all planned checks completed (tests executed: workflow-stall.test.ts + workflow-orchestrator.test.ts , 173 passed).; Context: PR #8839 attaches the harness's existing per-sub...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above were completed within budget.; Context: PR #8839 attaches the harness's existing per-sub...: none — all checks I started were completed within budget.; Context: PR #8839 attaches the harness's existing per-sub...: did not run the new vitest suites (analysis-only review; findings are code-path arguments, not test outcomes)., and 1 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):Context: PR #8839 attaches the harness's existing per-sub...:none — all planned checks completed (tests executed: workflow-stall.test.ts + workflow-orchestrator.test.ts , 173 passed).;Context: PR #8839 attaches the harness's existing per-sub...:none — all checks above completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above were completed within budget.;Context: PR #8839 attaches the harness's existing per-sub...:none — all checks I started were completed within budget.;Context: PR #8839 attaches the harness's existing per-sub...:did not run the new vitest suites (analysis-only review; findings are code-path arguments, not test outcomes).,另有 1 条。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| * written at attach time, so every dispatched prompt materializes its file | ||
| * immediately — including dispatches that fail before the agent launches. |
There was a problem hiding this comment.
[Critical] The transcripts this PR deliberately materializes land in <projectDir>/subagents/<sessionId>/, where the /review coverage gate (packages/cli/src/commands/review/lib/coverage.ts) treats every agent-*.jsonl as an agent it launched itself. Its idle classification (successfulToolCalls === 0 → idleAgents, coverage.ts:573–576) runs BEFORE the "agents this review did not launch" escape (if (!given) continue;, :584), and ok requires idleAgents.length === 0 (:954). This diff creates zero-tool-call file producers that did not exist on the workflow path: pre-launch failures (unknown agentType, isolation:'remote' — both thrown AFTER the seed record is written), abandoned stall attempts (one seed-only file per attempt), and workflow agents that make zero tool calls. Probed against the real coverageFromTranscripts: adding one PR-shaped seed-only record flips a compliant 2-chunk review from {"ok":true} to {"ok":false,"idle":["review the diff"]} and check-coverage exits 3 demanding a relaunch of an "agent" the review never launched (named for the workflow prompt's first line); no prescribed remediation clears it, since superseded() requires a verbatim CLI-built-prompt match. Rarer variant: a workflow prompt containing literal chunk N of M text is adopted as that chunk's agent and can donate coverage credit no review agent certified. The gate's over-inclusion pre-exists, but this diff makes it newly reachable, and the provenance half of the fix can only come from this PR. — Failure scenario: during a /review session (same project dir + session id) the model runs the workflow tool; a dispatch fails pre-launch or an attempt is abandoned → a seed-only transcript lands newer than the plan → the gate fails with exit 3 and the review cannot converge. Probe: BASELINE ok:true → WITH-WF ok:false → WITHOUT-WF ok:true. — Suggested fix: give workflow transcripts a provenance readers can filter on (a distinguishing record field), and/or gate-side classify only records whose launch prompt matches a CLI-built prompt; also move the not-launched escape ahead of the idle classification.
中文说明
本 PR 有意物化的 transcript 落在 <projectDir>/subagents/<sessionId>/,而 /review 的覆盖门禁(packages/cli/src/commands/review/lib/coverage.ts)把该目录下每个 agent-*.jsonl 都当作它自己启动的 agent。其 idle 分类(successfulToolCalls === 0 → idleAgents,coverage.ts:573–576)先于“非本次审查启动”豁免分支(if (!given) continue;,:584)执行,且 ok 要求 idleAgents.length === 0(:954)。此 diff 在 workflow 路径上引入了此前不存在的零工具调用文件产生者:启动前失败(未知 agentType、isolation:'remote'——都在种子记录写入之后才抛错)、被放弃的 stall 重试(每个 attempt 一个仅含种子的文件)、以及零工具调用的 workflow agent。对真实 coverageFromTranscripts 的 probe:仅加入一个本 PR 形状的只含种子记录的 transcript,就会让一个合规的双块审查从 {"ok":true} 翻转为 {"ok":false,"idle":["review the diff"]},check-coverage 以 exit 3 失败并要求重新启动一个审查从未启动过的“agent”(以 workflow prompt 首行命名);没有任何规定的补救手段能清除它(superseded() 要求与 CLI 构建的 prompt 逐字匹配)。更罕见的变体:包含字面 chunk N of M 文本的 workflow prompt 会被当作该 chunk 的 agent,捐献没有任何审查 agent 认证过的覆盖功劳。门禁侧的过度纳入是预先存在的,但此 diff 使其新可达,而修复的溯源那一半只能由本 PR 提供。— 失败场景:/review 会话期间(同一项目目录 + 会话 id)模型运行 workflow 工具;dispatch 在启动前失败或 attempt 被放弃 → 比 plan 更新的种子 transcript 落盘 → 门禁以 exit 3 失败,审查无法收敛。Probe:BASELINE ok:true → WITH-WF ok:false → WITHOUT-WF ok:true。— 建议修复:给 workflow transcript 加上读者可过滤的溯源信息(可区分的记录字段),和/或门禁侧只分类 launch prompt 与 CLI 构建 prompt 匹配的记录;同时将未启动豁免分支移到 idle 分类之前。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| function appendAttemptDetail(err: unknown, detail: string | undefined): void { | ||
| if (!detail || !(err instanceof Error)) return; | ||
| if (!Object.getOwnPropertyDescriptor(err, 'message')?.writable) return; |
There was a problem hiding this comment.
[Suggestion] appendAttemptDetail silently drops abandonedDetail when the terminating error's message is not a writable own property — DOMException('Workflow aborted.', 'AbortError') (thrown by runOverridePath's schema-mode abort at workflow-orchestrator.ts:1029) is dropped by the writable guard in Node 22 — and any non-Error rejection is dropped by the instanceof Error guard. No fallback log exists, so a multi-attempt run's transcripts are left unpaired with the terminal error — the exact outcome abandonedDetail was added to prevent (its JSDoc: "stay pairable with the error"). The guard itself is correct — forcing the assignment would throw a TypeError and replace the AbortError the sandbox classifies on; the gap is that the skip is silent. — Failure scenario: attempt 1 of a schema-mode dispatch stalls (transcript 1 written); attempt 2 is parent-aborted → the DOMException's guards reject the append (probe: caught.name AbortError, message unchanged, contains attempt ids: false) → two transcripts on disk, error names no attempt id. The new test masks this by throwing a plain Error. — Suggested fix: when the detail cannot be attached, emit the pairing via debugLogger.warn (mirroring warnSchemaContentFailure), or attach it via a non-message property (e.g. err.cause) that preserves name === 'AbortError'.
中文说明
当终止错误的 message 不是可写的自有属性时,appendAttemptDetail 会静默丢弃 abandonedDetail——DOMException('Workflow aborted.', 'AbortError')(由 workflow-orchestrator.ts:1029 的 schema 模式 abort 抛出)在 Node 22 中被 writable 守卫拦下——任何非 Error 的 rejection 则被 instanceof Error 守卫拦下。由于没有兜底日志,多次 attempt 运行留下的 transcript 无法与终止错误配对——这正是 abandonedDetail 要防止的结果(其 JSDoc:“stay pairable with the error”)。守卫本身是正确的——强行赋值会抛出 TypeError 并替换掉 sandbox 赖以分类的 AbortError;问题在于这种跳过是静默的。— 失败场景:schema 模式 dispatch 的 attempt 1 stall(transcript 1 已写入);attempt 2 被父级 abort → DOMException 被守卫拒绝追加(probe:caught.name 为 AbortError,message 不变,不含 attempt id)→ 磁盘上有两份 transcript,错误却不指名任何 attempt id。新测试因抛出普通 Error 而掩盖了这一点。— 建议修复:无法附加 detail 时,通过 debugLogger.warn 输出配对信息(仿照 warnSchemaContentFailure),或附加到非 message 属性(如 err.cause)上,以保持 name === 'AbortError'。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // BEFORE any agent event can fire on it. Terminal errors name this | ||
| // id directly except the two upstream-verbatim schema content | ||
| // failures, which log it instead (see runOverridePath); when the |
There was a problem hiding this comment.
[Suggestion] Two upstream-verbatim terminal errors — agent({agentType}): agent type '<name>' not found. (~L817) and agent({isolation:'remote'}) is not available in this build. (~L789) — now occur AFTER the transcript attach has seeded a file, but neither names the attempt id nor logs a pairing, unlike the two schema-content failures which this PR deliberately pairs via warnSchemaContentFailure. Both are deterministic single-attempt failures, so abandonedDetail never applies. This comment's enumeration ("except the two upstream-verbatim schema content failures") asserts exhaustive coverage and is wrong — these two neither name nor log the id. — Failure scenario: agent('x', {agentType: 'Typo'}) seeds agent-workflow-agent-<hex>.jsonl (pinned by this PR's own test), then rejects with the unpaired not-found error; an operator post-morteming sees N transcript files and an error naming no id — the gap this PR exists to close. — Suggested fix: generalize warnSchemaContentFailure (e.g. warnTranscriptPairing(config, workflowAgentId, reason)) and call it before these two throws; correct the comment's enumeration.
中文说明
两个 upstream 逐字 terminal 错误——agent({agentType}): agent type '<name>' not found.(约 L817)与 agent({isolation:'remote'}) is not available in this build.(约 L789)——现在都发生在 transcript attach 已种下文件之后,但两者既不在错误中指名 attempt id,也不记录配对日志,而不像本 PR 特意为两个 schema 内容失败通过 warnSchemaContentFailure 做的那样。两者都是确定性的单次 attempt 失败,因此 abandonedDetail 永远不会生效。此注释的枚举(“except the two upstream-verbatim schema content failures”)声称穷尽覆盖,这是不对的——这两个错误既不指名也不记录该 id。— 失败场景:agent('x', {agentType: 'Typo'}) 种下 agent-workflow-agent-<hex>.jsonl(本 PR 自己的测试钉住了这一点),然后以未配对的 not-found 错误拒绝;事后排查的操作者看到 N 个 transcript 文件和一个不指名 id 的错误——正是本 PR 要弥合的缺口。— 建议修复:泛化 warnSchemaContentFailure(如 warnTranscriptPairing(config, workflowAgentId, reason)),在这两处 throw 之前调用;并修正注释的枚举。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| appendAttemptDetail(err, mixedRetryDetail); | ||
| throw err; | ||
| } finally { |
There was a problem hiding this comment.
[Suggestion] On a mixed retry whose attempt 2 ends in one of the two upstream-verbatim schema-content failures, appendAttemptDetail appends ' Attempt ids: ...' to the message — contradicting the throw-site comment and the warnSchemaContentFailure doc, which keep those texts verbatim "so scripts authored against either runtime see the same error text" (the pairing is warn-logged precisely to keep the text intact). — Failure scenario: attempt 1 stalls; attempt 2 throws the verbatim 'subagent completed without calling StructuredOutput (after 2 in-conversation nudges).' → probe: caught.message gains ' Attempt ids: a1, a2.', verbatim-equality preserved: false. A workflow script that branches on the exact upstream string (the documented reason the string is kept verbatim) stops matching after any stall-retry; prefix/includes() matching still works. — Suggested fix: exempt the two verbatim schema-content errors from appendAttemptDetail (e.g. a symbol/flag set at the throw site that appendAttemptDetail checks), relying on the existing warnSchemaContentFailure log for pairing.
中文说明
当混合重试的 attempt 2 以两个 upstream 逐字 schema 内容失败之一结束时,appendAttemptDetail 会向消息追加 ' Attempt ids: ...'——与 throw 处注释及 warnSchemaContentFailure 的文档相矛盾:后者刻意保持这些文本逐字不变,“使针对任一运行时编写的脚本看到相同的错误文本”(配对正是通过 warn 日志来保持文本完整)。— 失败场景:attempt 1 stall;attempt 2 抛出逐字的 'subagent completed without calling StructuredOutput (after 2 in-conversation nudges).' → probe:caught.message 被追加 ' Attempt ids: a1, a2.',逐字相等不再成立。按精确 upstream 字符串分支的 workflow 脚本(这正是保留逐字文本的书面理由)在任何 stall 重试后将不再匹配;前缀/includes() 匹配仍可工作。— 建议修复:让两个逐字 schema 内容错误豁免于 appendAttemptDetail(例如在 throw 处设置 symbol/flag 供 appendAttemptDetail 检查),配对继续由现有的 warnSchemaContentFailure 日志承担。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| function overrideTranscriptConfig( | ||
| resolveName: (name: string) => string | null, | ||
| ): Config { |
There was a problem hiding this comment.
[Suggestion] This new overrideTranscriptConfig factory re-implements the stub subagent surface fakeConfigWithMgr (~L3045–3192) already provides: createAgentHeadless → {subagent: {execute, getFinalText, getTerminateMode, getExecutionSummary}, dispose} — the exact minimal surface the override path consumes; both even read the shared nextTerminateMode state. That surface has already grown once under review pressure (getExecutionSummary added per fakeConfigWithMgr's own "R1 (#1)" comment), so the next surface change must update both stubs in lockstep. The irreducible difference is the wrapper config's transcript accessors (storage.getProjectDir/getProjectRoot/getCliVersion); the createAgentHeadless body itself need not be a second copy — fakeConfigWithMgr's onCreate.runWithEmitter hook can express the hardcoded emission. — Failure scenario: if the override path reads a new method from the subagent and only fakeConfigWithMgr is updated, the transcript-suite tests fail for an unrelated reason or pass vacuously. — Suggested fix: extract the stub-subagent builder into a module-scope helper parameterized by execute-time behavior, consumed by both fixtures (hoisting fakeConfigWithMgr out of its describe block).
中文说明
这个新的 overrideTranscriptConfig 工厂重新实现了 fakeConfigWithMgr(约 L3045–3192)已经提供的 stub subagent 表面:createAgentHeadless → {subagent: {execute, getFinalText, getTerminateMode, getExecutionSummary}, dispose}——正是 override 路径消费的最小表面;两者甚至共享 nextTerminateMode 状态。该表面已在评审压力下增长过一次(getExecutionSummary 是 fakeConfigWithMgr 自己的 “R1 (#1)” 注释记录的增补),下一次表面变更必须同步更新两个 stub。不可约的差异只是包装配置的 transcript 访问器(storage.getProjectDir/getProjectRoot/getCliVersion);createAgentHeadless 主体本身不必成为第二份拷贝——fakeConfigWithMgr 的 onCreate.runWithEmitter 钩子足以表达硬编码的事件发射。— 失败场景:若 override 路径从 subagent 读取一个新方法而只更新了 fakeConfigWithMgr,transcript 套件的测试会因无关原因失败或空转通过。— 建议修复:把 stub-subagent 构建器提取为模块级辅助函数,以执行期行为为参数,供两个 fixture 共用(需将 fakeConfigWithMgr 提升出所在 describe 块)。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| warnSchemaContentFailure(config, workflowAgentId); | ||
| throw new Error( | ||
| 'subagent completed without calling StructuredOutput (after 2 in-conversation nudges).', |
There was a problem hiding this comment.
[Suggestion] The new id↔transcript pairing log for the two upstream-verbatim schema content failures has zero assertions — both call sites execute under the two existing schema tests (workflow-orchestrator.test.ts:3437, :3466) but only into the (path unavailable) catch branch, because those tests' fakeConfigWithMgr config has no storage.getProjectDir/getProjectRoot accessors, so the getAgentJsonlPath construction never executes under test. Deleting either call site, or breaking the path construction inside warnSchemaContentFailure, ships green. — Failure scenario: a refactor deletes either warn call site (or breaks the path join) and every suite stays green — an operator hitting a schema content failure gets an id-less error and no log pointing at the record. — Suggested fix: in one of the two existing schema-failure tests, use a transcript-capable config and assert debugLogger.warn (via vi.spyOn) was called with a message containing the attempt id and its .jsonl path.
中文说明
两个 upstream 逐字 schema 内容失败的新 id↔transcript 配对日志没有任何断言——两个调用点在现有的两个 schema 测试(workflow-orchestrator.test.ts:3437、:3466)下都会执行,但只会走进 (path unavailable) 的 catch 分支,因为那些测试的 fakeConfigWithMgr 配置没有 storage.getProjectDir/getProjectRoot 访问器,getAgentJsonlPath 的构造在测试中从未执行。删除任一调用点,或破坏 warnSchemaContentFailure 内部的路径构造,都能带着全绿合入。— 失败场景:一次重构删除了任一 warn 调用点(或破坏了路径拼接),所有套件仍为绿——操作者遇到 schema 内容失败时只有不指名 id 的错误,也没有指向记录的日志。— 建议修复:在两个现有 schema 失败测试之一中使用具备 transcript 能力的配置,并断言 debugLogger.warn(通过 vi.spyOn)被调用且消息包含 attempt id 及其 .jsonl 路径。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // model-authored agentType must resolve the same definition in both | ||
| // places, or the transcript records a canonical launch that the raw | ||
| // string then fails to find. | ||
| const agentType = opts.agentType.trim(); |
There was a problem hiding this comment.
[Suggestion] The new agentType trimming (attach ~L512 and here) has no test — no case passes a padded agentType. The trim is load-bearing, not an equivalent mutant: findSubagentByNameAtLevel compares lowercased names without trimming, so ' explore ' does not resolve untrimmed. Mutation probes: deleting the trim here makes agent('x', {label, agentType: ' Explore '}) reject with agent type ' Explore ' not found while the transcript had recorded agentName: 'Explore' (a transcript claiming a launch the run says never happened); deleting the attach-side trim records ' Explore ' while the dispatch succeeds canonically. All 147 existing tests stay green under either mutant. Note the trim here only bites when a label is set (resolvedSubagent short-circuits the raw lookup otherwise), so the test set needs a {label, agentType: ' Explore '} case alongside an agentType-only one. — Suggested fix: add transcript tests dispatching with padded agentType: ' Explore ' — one agentType-only and one {label, agentType} case — asserting the dispatch resolves and the first record's agentName is 'Explore'.
中文说明
新的 agentType trim(attach 约 L512 与此处)没有任何测试——没有用例传入带空白的 agentType。该 trim 是承重改动而非等价变异:findSubagentByNameAtLevel 按小写化但不 trim 的名字比较,因此 ' explore ' 不 trim 就无法解析。变异 probe:删除此处 trim 会使 agent('x', {label, agentType: ' Explore '}) 以 agent type ' Explore ' not found 拒绝,而 transcript 已记录 agentName: 'Explore'(transcript 声称的启动在运行中并不存在);删除 attach 侧 trim 则记录 ' Explore ' 而 dispatch 以规范化定义成功。两种变异下现有 147 个测试全部保持绿。注意此处 trim 只在设置了 label 时才起作用(否则 resolvedSubagent 会短路原始查找),因此测试集除了仅 agentType 的用例外还需要一个 {label, agentType: ' Explore '} 用例。— 建议修复:新增 transcript 测试,以带空白的 agentType: ' Explore ' dispatch——一个仅 agentType、一个 {label, agentType}——断言 dispatch 成功解析且首条记录的 agentName 为 'Explore'。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| // shows in progress output, so a reader matching a transcript to a line | ||
| // of the script has the same name in both places. `agentType` is the | ||
| // fallback that still says something; the constant is the last resort. | ||
| const label = typeof opts.label === 'string' ? opts.label.trim() : ''; |
There was a problem hiding this comment.
[Suggestion] agentName is recorded from a trimmed label, but every other surface uses opts.label untrimmed — the fast path launches AgentHeadless.create(opts.label ?? 'workflow-agent', …) (~L627), the override path's ephemeral config uses the untrimmed name (~L827), and the stall/abandoned error renders the untrimmed option (workflow-stall.ts:277). A padded label makes the transcript name disagree with the run's own displayed name — the exact join the attach's own comment ("a reader matching a transcript to a line of the script has the same name in both places") promises. The diff already applies this consistency argument to agentType but not to label. — Failure scenario (probed): {label: ' reviewer '} → the run shows ' reviewer ' while the transcript records 'reviewer'; {label: ' '} → the run shows ' ' while the transcript records 'workflow-agent'. An operator matching transcripts to run output by name gets no match. — Suggested fix: trim once at the top of the dispatch (const label = typeof opts.label === 'string' ? opts.label.trim() : undefined, mapping empty-trim to undefined) and pass the trimmed value to the stall options, AgentHeadless.create, and the ephemeral config. Fix verified by probe: 149/149 green with convergence assertions.
中文说明
agentName 记录的是 trim 后的 label,但其他所有表面都使用未 trim 的 opts.label——fast path 以 AgentHeadless.create(opts.label ?? 'workflow-agent', …) 启动(约 L627),override 路径的临时配置使用未 trim 的名字(约 L827),stall/abandoned 错误也渲染未 trim 的选项(workflow-stall.ts:277)。带空白的 label 会使 transcript 名字与运行自身显示的名字不一致——恰恰破坏了 attach 注释自己承诺的关联(“把 transcript 与脚本某行对应的读者在两处看到同一个名字”)。diff 已把这一一致性论证用在 agentType 上,却没有用在 label 上。— 失败场景(已 probe):{label: ' reviewer '} → 运行显示 ' reviewer ' 而 transcript 记录 'reviewer';{label: ' '} → 运行显示 ' ' 而 transcript 记录 'workflow-agent'。按名字把 transcript 与运行输出匹配的操作者将一无所获。— 建议修复:在 dispatch 顶部 trim 一次(const label = typeof opts.label === 'string' ? opts.label.trim() : undefined,空串 trim 映射为 undefined),并把 trim 后的值传给 stall 选项、AgentHeadless.create 与临时配置。修复已经过 probe 验证:含收敛断言 149/149 全绿。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| function warnSchemaContentFailure( | ||
| config: Config, | ||
| workflowAgentId: string, | ||
| ): void { |
There was a problem hiding this comment.
[Suggestion] The only pairing between the two upstream-verbatim schema content errors and their transcripts is this debugLogger.warn call — writeLog returns early unless isDebugLogFileEnabled() (debugLogger.ts:105–107), which requires QWEN_DEBUG_LOG_FILE, set only in debug mode. In a default run the pairing this function exists to provide is never written anywhere: the error texts are verbatim (no id), the run-log mirror flattens to String(hostErr.message) (no id), and abandonedDetail only appends when attempt > 1. The PR's stated purpose ("no record is left unpairable") holds for these two failure classes only under an opt-in debug flag. In-repo convention triangulates the fix: extension/corruptFile.ts:28–31 comments that debugLogger.warn is gated and deliberately surfaces on stderr instead. — Failure scenario: with default settings, a schema content failure leaves the error naming no id and every transcript under subagents/<sessionId>/ equally plausible as the culprit — the pairing promise holds only under --debug. — Suggested fix: surface the pairing on a default-enabled channel (the workflow run log's dispatch-failure path), or attach the id to the error as a non-message property the workflow layer can render.
中文说明
两个 upstream 逐字 schema 内容错误与其 transcript 之间唯一的配对就是这个 debugLogger.warn 调用——除非 isDebugLogFileEnabled()(debugLogger.ts:105–107)为真,writeLog 会提前返回,而这需要 QWEN_DEBUG_LOG_FILE,只在 debug 模式下设置。默认运行中,这个函数存在所要提供的配对不会被写到任何地方:错误文本是逐字的(无 id),运行日志镜像被压平为 String(hostErr.message)(无 id),而 abandonedDetail 只在 attempt > 1 时追加。PR 声明的目的(“no record is left unpairable”)对这两类失败只在选择性开启的 debug 标志下成立。仓库内惯例印证了修法:extension/corruptFile.ts:28–31 注释说明 debugLogger.warn 受此门槛限制,因此刻意改走 stderr。— 失败场景:默认设置下,schema 内容失败让错误不指名 id,subagents/<sessionId>/ 下每个 transcript 都同样可能是当事记录——配对承诺只在 --debug 下成立。— 建议修复:把配对信息放到默认开启的通道(workflow 运行日志的 dispatch 失败路径),或把 id 附加到错误的非 message 属性上供 workflow 层渲染。
— qwen3.8-max via Qwen Code /review (v0.21.9)
There was a problem hiding this comment.
Declined — both proposed channels were checked against the code and neither one lands the pairing on a default-enabled surface today:
- Non-message error property: currently a dead switch. Every surface that shows a dispatch failure flattens to the message —
mapDispatchErrorinworkflow-sandbox.tsbuilds the vm error fromString(hostErr.message), the runner settlement usesextractErrorMessage, and theagentCompleted(label, error)event string is discarded byworkflow-runner.ts. A property no renderer reads adds nothing by this repo's own dead-switch rule. - Workflow run-log dispatch-failure path: that mirror lives inside the injected sandbox bridge JS (
mapDispatchError/observeDispatchinworkflow-sandbox.ts). Extending security-sensitive host-boundary code so an audit-trail id can ride the mirror is disproportionate risk for this PR.
What landed instead narrows the gap as far as it goes without those surfaces: the R3-2 fallback warn now covers exactly this finding's mixed-retry case (asserted by the new DOMException and verbatim-across-stall-retry tests), the throw-site pairing warn is asserted end-to-end with a transcript-capable config (R3-9), and the two pre-launch rejections are paired too (R3-3). The remaining gap is narrow: a default (non---debug) run whose mixed retry ends in a verbatim schema-content failure — and even there the transcript file name carries the attempt id and its contents show the failed structured_output calls. Happy to revisit if a default-enabled dispatch-failure diagnostic surface is added later.
中文说明
不予采纳——两个建议通道都对照代码核实过,目前都无法把配对信息送到默认开启的展示面上:
- 错误对象的非 message 属性:目前是死开关。所有展示 dispatch 失败的出口都会把错误拍平成 message——
workflow-sandbox.ts里的mapDispatchError用String(hostErr.message)构造 vm 错误,runner 结算用extractErrorMessage,而agentCompleted(label, error)事件的字符串在workflow-runner.ts里被直接丢弃。没有任何渲染方会读这个属性,按本仓库自己的死开关规则,加了也没有意义。 - workflow 运行日志的 dispatch 失败路径:那个镜像逻辑在注入的沙箱桥接 JS 里(
workflow-sandbox.ts的mapDispatchError/observeDispatch)。为了让一条审计用的 id 能搭上镜像,去扩展安全敏感的主机边界代码,对这个 PR 来说风险与收益不成比例。
本轮落地的改动在不触碰这些表面的前提下尽量收窄了缺口:R3-2 的回退 warn 现在正好覆盖了本条发现的混合重试场景(由新增的 DOMException 测试和 stall 重试保持 verbatim 测试断言),throw 处的配对 warn 已在具备 transcript 能力的 config 下做了端到端断言(R3-9),两个启动前拒绝错误也补上了配对(R3-3)。剩余缺口很窄:默认(非 --debug)运行、且混合重试以 verbatim 的 schema 内容失败收尾——即便如此,transcript 文件名里带着 attempt id,文件内容也能看到失败的 structured_output 调用。如果以后新增了默认开启的 dispatch 失败诊断面,很乐意重新考虑。
| // Seeds the first `user` record, written before the model has said | ||
| // anything — so the transcript states what the agent was asked to do | ||
| // without a reader needing the script that asked it. | ||
| initialUserPrompt: prompt, |
There was a problem hiding this comment.
[Suggestion] The attempt-id pairing in terminal errors and the doc's "every dispatched prompt materializes its file immediately" guarantee are unconditional, but materialization is conditional: an empty prompt seeds nothing (recordUserMessage's if (!text) return; the writer's own test pins "skips an empty initialUserPrompt"), the fd opens lazily, ROUND_START is not a recorded event, and an unwritable transcript dir is swallowed invisibly (ensureOpen sets openFailed, attach never learns). Probed through the real createProductionDispatch + runStallResilient: empty prompt → the all-stall error names 3 attempt ids with zero files on disk; unwritable dir → the same dangling ids, dispatch proceeds file-less. Flip check: removing this PR's abandonedDetail wiring removes the ids — the dangling pointers are produced exactly by this wiring. — Failure scenario: agent('', {...}) stalls on all 3 attempts before any recordable event → terminal error reads 'Attempt ids: <id1>, <id2>, <id3>.' while zero transcript files exist — every id is a dangling pointer, contradicting the doc's universal claim. Same shape for any prompt when the transcript directory is unwritable. — Suggested fix: have attachDispatchTranscript report whether the seed record actually materialized, and either list only ids that have a record on disk or phrase the detail defensively; at minimum, correct the doc comment's universal claim for the empty-prompt case.
中文说明
terminal 错误中的 attempt id 配对与文档里“每个 dispatched prompt 立即物化其文件”的保证都是无条件的,但物化是有条件的:空 prompt 不会种下任何记录(recordUserMessage 的 if (!text) return;writer 自己的测试钉住了“skips an empty initialUserPrompt”),fd 是惰性打开的,ROUND_START 不是被记录的事件,transcript 目录不可写时会被无声吞掉(ensureOpen 置 openFailed,attach 无从得知)。对真实 createProductionDispatch + runStallResilient 的 probe:空 prompt → all-stall 错误指名 3 个 attempt id 而磁盘上零文件;目录不可写 → 同样的悬空 id,dispatch 无文件继续。翻转检查:移除本 PR 的 abandonedDetail 接线后 id 消失——悬空指针正是由这段接线产生。— 失败场景:agent('', {...}) 在任何可记录事件之前连续 3 次 stall → terminal 错误写着 'Attempt ids: <id1>, <id2>, <id3>.' 而一个 transcript 文件都不存在——每个 id 都是悬空指针,与文档的普遍性声明矛盾。transcript 目录不可写时对任何 prompt 都是同样形态。— 建议修复:让 attachDispatchTranscript 报告种子记录是否真正物化,只列出磁盘上有记录的 id,或以防御性措辞表达 detail;至少修正文档注释对空 prompt 情形的普遍性声明。
— qwen3.8-max via Qwen Code /review (v0.21.9)
… and pairing honest (QwenLM#8839) - Stamp agentKind:'workflow' on every workflow transcript record and filter it out of the /review coverage gate, so a seed-only workflow record can no longer be classified as an idled review agent; move the not-launched escape ahead of the idle check for the untagged foreign records. - Move the attempt-id error decoration from the generic retry wrapper to the production dispatch, skip upstream-verbatim schema errors, and log the pairing instead of appending when the message cannot be mutated (DOMException) or must stay verbatim. - Pair the two pre-launch rejections with their transcripts via the same warn, normalize label once per dispatch, and resolve agentType raw for Agent-tool parity.
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Round 4 — review feedback addressedCommit: Findings and dispositionsCritical
Suggestions
Review-level
|
已被后续 commit 取代,当前 head 需重新 review
doudouOUC
left a comment
There was a problem hiding this comment.
Code Review — PR #8839 (round 1, model: deepseek-v4-flash)
Overall the change is sound: all 5 diff chunks were reviewed by 10 parallel review agents, the previous blocker (coverage-gate issue, comment 3759856442) was re-checked and confirmed fixed by this diff, all workspaces build cleanly, and every pre-existing test failure is outside the diff files. Two Suggestion-level items were found:
Findings
[Suggestion] verificationGaps workflow filter has no test coverage
packages/cli/src/commands/review/lib/coverage.ts:1346
The agentKind !== 'workflow' filter added in verificationGaps (.filter((rec) => rec.agentKind !== 'workflow')) is not exercised by any test. The identical filter in coverageFromTranscripts is covered, but there is no equivalent case for verificationGaps. If the filter were accidentally removed, workflow dispatches that open a diff file would be counted as verifiers, potentially masking a gap in verification coverage.
[Suggestion] workflow-stall.test.ts is inert for this PR's change
packages/core/src/agents/runtime/workflow-stall.test.ts:221
The test-efficacy check found that all 24 tests in this suite pass even when the source change is reverted — the suite does not actively guard the new behavior (the only change in this file is an explanatory comment pointing to the real coverage in workflow-orchestrator.test.ts). A regression in workflow-stall behavior introduced via this PR's dispatch path would only be caught if it happens to be covered by the orchestrator transcript suite.
Notes
- Attempt-id pairing, per-attempt transcripts, and best-effort cleanup behavior are covered by the new
dispatch transcriptsuite inworkflow-orchestrator.test.ts(137 orchestrator cases + 328 related cases verified locally per the PR description). - This comment was posted manually because the automated review session completed the analysis but terminated before its own comment-posting step.
…nLM#8839) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/100 轮)。改动内容与我反驳保留之处如下: Autofix review-handling summary — PR #8839 (round 1 feedback)Two Suggestion-level findings from the automated review. One addressed with a new regression test; one declined with evidence. Findings and dispositions1.
|
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x) and Integration Tests (CLI, No Sandbox) were skipped in CI; the suites ran on ubuntu CI and locally (Linux) only.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — no check left unfinished.; You are review agent reverse-audit — Reverse audit agen...: none — finished within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above were completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
已审查。 建议见行内评论。
未审查:build-and-test — Test (macos-latest, Node 22.x), Test (windows-latest, Node 22.x) and Integration Tests (CLI, No Sandbox) were skipped in CI; the suites ran on ubuntu CI and locally (Linux) only。
未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — no check left unfinished.;You are review agent reverse-audit — Reverse audit agen...:none — finished within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above were completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above completed within budget.。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| warnTranscriptPairing( | ||
| config, | ||
| workflowAgentId, | ||
| 'pre-launch failure (remote isolation unavailable)', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Neither pre-launch warnTranscriptPairing call site (this one and the agent-type-not-found site at ~L850) is exercised by any test — a deletion probe removed both calls and all 154 tests stayed green. For a single-attempt run this warn is the ONLY id↔transcript pairing of a pre-launch failure: the verbatim message carries no id by design, and the seeded transcript is written at attach time, before the rejection. — Failure scenario: deleting or breaking either warn call ships green → an operator staring at agent({isolation:'remote'}) is not available in this build. has no log pairing the error to the seeded agent-workflow-agent-<hex>.jsonl, which becomes orphaned — the exact post-mortem gap this feature exists to close. Fix: mirror pairs the verbatim schema content failure with its transcript in the warn log — dispatch { isolation: 'remote' } (or an unregistered agentType) under a transcript-capable config, assert a warn matching /pre-launch failure .* for (workflow-agent-[0-9a-f]{16}); transcript: /, and that the named file exists.
中文说明
两个 pre-launch 的 warnTranscriptPairing 调用点(此处与 ~L850 的 agent-type-not-found 处)均无任何测试覆盖——删除探针移除了这两个调用,全部 154 个测试仍然全绿。对单次 attempt 运行而言,这条 warn 是 pre-launch 失败唯一的 id↔transcript 配对:按设计 verbatim 消息不含 id,而种子 transcript 在 attach 时(拒绝发生之前)就已写入。— 失败场景:删除或破坏任一 warn 调用都能带着全绿合入 → 操作者面对 agent({isolation:'remote'}) is not available in this build. 时没有任何日志把错误与种下的 agent-workflow-agent-<hex>.jsonl 配对,该 transcript 成为孤儿——正是本功能要弥合的事后排查缺口。修复:仿照 pairs the verbatim schema content failure with its transcript in the warn log——在具备 transcript 能力的配置下 dispatch { isolation: 'remote' }(或未注册的 agentType),断言 warn 匹配 /pre-launch failure .* for (workflow-agent-[0-9a-f]{16}); transcript: /,且所指文件存在。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const warns = debugLogRecorder.warn.mock.calls.map((c) => String(c[0])); | ||
| expect(warns.some((w) => w.includes('Attempt ids:'))).toBe(true); |
There was a problem hiding this comment.
[Suggestion] This assertion only pins the literal 'Attempt ids:' prefix, not the ids themselves — weaker than the comment's promise above ('The multi-attempt pairing was not dropped by the skip — it was logged instead') and weaker than the sibling tests in this describe block ('names every attempt id in the stall-abandoned error', 'names every attempt id when a parent abort ends a mixed retry'), which derive each attempt id from the transcript file names and assert containment. — Failure scenario: a refactor of appendAttemptDetail's fallback branch (~L1249) that degrades the warn payload (drops the detail, joins an empty/wrong id list, passes the wrong attempt id) leaves this test green while a DOMException terminal failure — the error class that cannot be rewrapped because the sandbox classifies it by name === 'AbortError' — silently loses its only pairing to its transcripts. Fix: derive each attempt id from the two transcript file names on disk and assert the pairing warn contains each one, matching the sibling tests:
const files = transcriptFiles();
expect(files).toHaveLength(2);
const pairingWarn = warns.find((w) => w.includes('Attempt ids:'));
expect(pairingWarn).toBeDefined();
for (const file of files) {
const id = path.basename(file, '.jsonl').slice('agent-'.length);
expect(pairingWarn).toContain(id);
}中文说明
该断言只钉住了字面前缀 'Attempt ids:',没有钉住 id 本身——弱于上方注释的承诺('The multi-attempt pairing was not dropped by the skip — it was logged instead'),也弱于同一 describe 块中的兄弟测试('names every attempt id in the stall-abandoned error'、'names every attempt id when a parent abort ends a mixed retry'):它们从 transcript 文件名推导出每个 attempt id 并断言包含。— 失败场景:appendAttemptDetail 的回退分支(~L1249)被重构而 warn 载荷退化(丢弃 detail、拼接空/错误的 id 列表、传错 attempt id)时,本测试仍然保持绿色,而 DOMException 终止失败——因 sandbox 按 name === 'AbortError' 分类而不能被重新包装的错误类——会悄然失去与 transcript 的唯一配对。修复:从磁盘上的两个 transcript 文件名推导每个 attempt id,断言配对 warn 包含每一个,与兄弟测试的写法一致(见上方代码)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if (attemptIds.length > 1) { | ||
| appendAttemptDetail(config, err, attemptIds); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Single-attempt terminal errors rethrown raw from the agent loop (e.g. a model-client API auth/network failure mid-loop; AgentHeadless.execute sets terminateMode=ERROR and rethrows) neither name the attempt id nor log the pairing: with attemptIds.length === 1 this catch skips appendAttemptDetail, and unlike the four verbatim failures no warnTranscriptPairing is called — while a fully populated transcript exists on disk. The comment above asserts 'A single-attempt failure already names its own id — or logs it, for the verbatim errors', which is false for this class (probe-confirmed: the surfaced error carried no id, zero warns were logged, and the transcript sat orphaned). The same class covers unexpected throws inside runOverridePath before the id-named terminals (AgentHeadless.create, worktree provisioning). — Failure scenario: agent('do X') runs one attempt (no stall); the model client throws mid-loop → the error reaching the script/operator carries no workflow-agent-<hex> id and nothing logs it, orphaning the transcript for exactly the failed-run post-mortem this feature's doc block exists for. Fix: also log the pairing when one attempt ran and the error does not already carry its id — add to this catch:
} else if (attemptIds.length === 1) {
warnTranscriptPairing(
config,
attemptIds[0]!,
'single-attempt terminal failure',
);
}(or unconditionally log the last attempt's pairing on every failed run), and correct the comment's invariant.
中文说明
从 agent 循环中原样重抛的单次 attempt 终止错误(例如循环中途的模型客户端 API 鉴权/网络失败;AgentHeadless.execute 置 terminateMode=ERROR 并重抛)既不在错误中指名 attempt id,也不记录配对日志:attemptIds.length === 1 时这个 catch 跳过 appendAttemptDetail,而且与四个 verbatim 失败不同,也没有调用 warnTranscriptPairing——尽管磁盘上已有一份内容完整的 transcript。上方注释声称 'A single-attempt failure already names its own id — or logs it, for the verbatim errors',对这一类错误并不成立(探针确认:浮现的错误不含 id、零条 warn 被记录、transcript 成为孤儿)。同一类还包括 runOverridePath 中在指名 id 的终止错误之前的意外抛出(AgentHeadless.create、worktree 准备)。— 失败场景:agent('do X') 只跑一次 attempt(无 stall),模型客户端在中途抛错 → 到达脚本/操作者的错误不含 workflow-agent-<hex> id,也没有任何日志记录它,transcript 就此孤儿化——恰恰是本功能文档块存在意义所在的失败运行事后排查。修复:当只跑了一次 attempt 且错误尚未携带其 id 时,也在此 catch 中记录配对(见上方代码),或无条件地在每次失败运行时记录最后一个 attempt 的配对;同时修正注释中的不变量表述。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| debugLogger.warn( | ||
| `[workflow] ${reason} for ${workflowAgentId}; transcript: ${transcript}`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R3-12 (round 3, still standing): this warn is the sole fallback carrier of the id↔transcript pairing for errors that cannot carry attempt ids, but debugLogger.warn writes nothing unless QWEN_DEBUG_LOG_FILE is set — the CLI sets it only under --debug (cli config.ts:1563). Verified at this commit by the repo's own debugLogger.test.ts (31/31, including 'does not write debug log by default'). The doc invariants — 'The pairing then rides the warn log instead, so no transcript is left orphaned by the skip' (~L1232) and the catch comment's 'or logs it, for the verbatim errors' (~L456) — hold only under --debug. The round-3 autofix decline conceded the mechanism and rejected only the two proposed fixes. — Failure scenario: default session (no --debug), a multi-attempt run ends in a DOMException abort / verbatim-marked content failure / non-Error rejection → the skip branch warns, writeLog early-returns, nothing is written anywhere → the operator sees an error that by design names no attempt id, with no record pairing it to either attempt's transcript; under parallel() dispatches, reconstructing the pairing by mtime degrades further. Fix (minimum): correct the two over-promising comments (~L456-459, ~L1231-1233) to state the actual guarantee ('rides the warn log when debug file logging is enabled'). Substantive option: land the pairing on a default-enabled surface — the round-3 reply said this is worth revisiting if one is added.
中文说明
R3-12(第 3 轮提出,依然存在):这条 warn 是「无法携带 attempt id 的错误」与其 transcript 配对的唯一兜底载体,但 debugLogger.warn 在未设置 QWEN_DEBUG_LOG_FILE 时什么都不写——CLI 只在 --debug 下才设置该变量(cli config.ts:1563)。已在本提交上由仓库自带的 debugLogger.test.ts 验证(31/31 通过,包含 'does not write debug log by default')。文档中的不变量——'The pairing then rides the warn log instead, so no transcript is left orphaned by the skip'(~L1232)以及 catch 注释里的 'or logs it, for the verbatim errors'(~L456)——只在 --debug 下成立。第 3 轮 autofix 的 decline 承认了该机制('The remaining gap is narrow...'),只是否决了两个具体修复提案。— 失败场景:默认会话(无 --debug)下,多次 attempt 运行以 DOMException abort / verbatim 标记的内容失败 / 非 Error 拒绝终结 → skip 分支发出 warn,writeLog 提前返回,任何地方都没有写入 → 操作者看到一个按设计不含 attempt id 的错误,也没有任何记录能把它与任一 attempt 的 transcript 配对;在 parallel() dispatch 下按 mtime 重建配对更加困难。修复(最低限度):修正两处过度承诺的注释(~L456-459、~L1231-1233),改为陈述真实保证('rides the warn log when debug file logging is enabled')。实质性选项:让配对落在默认启用的表面上——第 3 轮回复表示若将来增加这样的表面,值得重议。
— qwen3.8-max via Qwen Code /review (v0.21.10)
…enLM#8839) - Log the id↔transcript pairing in the dispatch catch when a single-attempt run ends in an error that neither names the attempt id nor logged it at the throw site — model-client rethrows, create and worktree-provisioning throws previously left the transcript orphaned. - Mark the two pre-launch rejections in verbatimTerminalErrors alongside the schema content failures so the catch recognises their throw-site pairing and never logs it twice. - Pin both pre-launch pairing warns under a transcript-capable config (deletion probes previously shipped green) and strengthen the DOMException pairing assertion to name every attempt id. - State the real guarantee in the pairing comments: the warns ride the debug log file and land on disk only when debug file logging is enabled. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/100 轮)。改动内容与我反驳保留之处如下: Autofix round 4 summary — PR #8839All four inline findings from the automated reviewer were addressed in one commit: Findings and dispositions
Files changed
VerificationCommands actually run this round (all from the repository root unless noted):
Not run (with reason): integration tests after 中文说明Autofix 第 4 轮总结 — PR #8839自动审查者的四条行内发现已在一次提交中全部处理: 发现与处置
变更文件
验证本轮实际执行的验证命令(除注明外均在仓库根目录执行):
未执行的命令(附原因): Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the suites ran on ubuntu CI and locally (Linux) only.
Not explored to full depth (tool budget reached): Context: PR #8839 attaches the harness's per-subagent tra...: none — all checks above completed within budget.; Context: PR #8839 attaches the harness's per-subagent tra...: none — all checks above completed within budget.; Context: PR #8839 attaches the harness's per-subagent tra...: none — all checks above completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
已审查。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the suites ran on ubuntu CI and locally (Linux) only。
未探索到全部深度(达到工具调用预算):Context: PR #8839 attaches the harness's per-subagent tra...:none — all checks above completed within budget.;Context: PR #8839 attaches the harness's per-subagent tra...:none — all checks above completed within budget.;Context: PR #8839 attaches the harness's per-subagent tra...:none — all checks above completed within budget.。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review-address round: no action needed This round found no actionable feedback on PR #8839:
The PR is in Critical-only mode after 5 change-producing rounds. The single deferred item (an automated-reviewer review) is non-Critical feedback excluded from this round's actionable scope; it remains open for human follow-up and was intentionally left untouched per the Critical-only rules. No code changes were made this round, so no verification commands were run. 中文说明Autofix review-address 本轮:无需处理 本轮在 PR #8839 上未发现任何可处理的反馈:
该 PR 在经历 5 个产生改动的轮次后已进入仅处理 Critical 的模式。唯一一条被延后的条目(一次自动审查机器人的 review)属于本轮可处理范围之外被排除的非 Critical 反馈;按 Critical-only 规则,它保持开放状态留待人工跟进,本轮刻意未做任何改动。 本轮未做任何代码改动,因此也未运行任何验证命令。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Closing for now — parking this rather than abandoning it. The branch is preserved and this can be reopened as-is. |
…ript Supersedes QwenLM#8846, whose handoff layer did not work: it wrote the script to an arbitrary `--out` directory and told the caller to pass `args: <path>`. Neither is a form the Workflow tool accepts. `readWorkflowFileSecurely` realpaths `scriptPath` and refuses anything outside the saved-workflow directories, and `args` is inline JSON with no path form. Both were verified against the tool's own contract this time, not just the sandbox's. The sandbox has no filesystem — its globals are agent/parallel/pipeline/ phase/log/console/args/budget/workflow and nothing that opens a file — so a roster carried in `args` is a roster the model has to retype into its tool call. That is the failure this change exists to remove, reintroduced one layer up. The roster is therefore baked into the generated script, and the model's call carries one path and no payload. `--roster` and `emit-workflow` still build the same prompts from the same plan through the same `buildLaunch`. What differs is who launches them: `--roster` asks the orchestrator to copy ~13 blocks into agent calls, in one response, unedited; this writes them into a file the runtime reads. The generated file is a fixed body plus one JSON literal. No logic is generated, so the part that can be wrong is the part the tests execute — and they execute the real output of the generator, including a roster with backticks, `${`, backslashes and newlines in the prompts. Not a one-variable change, and the difference is now handled rather than claimed away: workflow dispatch substitutes its own terse subagent persona unless an agentType is given, so the script passes `agentType: 'general-purpose'` — the subagent type SKILL.md requires of the hand-launched path. Otherwise the two paths would run different agents over identical prompts and an A/B between them would not mean anything. Also from the QwenLM#8846 review: - Prompts are recorded. `check-coverage` compares each launch against what the CLI recorded handing out, so without this the whole roster read as unlaunched. - The workflows gate is checked before anything is written, so a run that cannot execute what this emits leaves no script and no prompt records implying it did. - A fan-out where every agent failed throws instead of returning a value. Returned, it would let the caller aggregate over a diff no agent read. - Cleanup sweeps the generated script. It has to live in the user's saved-workflow dir, where every file is a `/<name>` slash command, so a review that left one behind would hand the user a permanent command for a diff that no longer exists. - The dead `label` / `version` / `mode` / `plan` fields are gone with the args file, and the unreachable chunk guard with them. - `readPlanReport` is one definition in lib/report.ts rather than a sixth copy of the try/parse/rethrow block; moving the existing five onto it is mechanical and left to its own change. Still refused, unchanged: a territory fan-out (3B), whose chunk agents carry a per-chunk contract this script does not express, and a worktree review, which needs every agent pinned to the PR worktree. Draft: this depends on QwenLM#8839 for coverage evidence — workflow-dispatched agents write no subagent transcript without it — and nothing routes through this yet, so review behaviour is unchanged. Part of QwenLM#8769. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What this PR does
Attach the harness's existing per-subagent transcript writer to workflow dispatch, so every
agent()call leaves a record in<projectDir>/subagents/<sessionId>/agent-<id>.jsonl— the same place, and the same format, the Agent tool already writes.Why it's needed
Workflow dispatch bypassed the transcript attachment used by the Agent tool, so workflow subagents were the only agents in the product that left no record on disk.
Everything that reads that directory to answer "what did this agent actually do" was blind on the workflow path — post-mortem of a failed run, cost accounting, and any check built on tool calls rather than on the agent's own prose. An agent that did nothing can still write plausible text; only the recorded tool calls show what it actually did.
How
The existing writer is attached once in the dispatch wrapper, covering both dispatch paths because they share one event emitter per attempt. The agent id is allocated before attachment, so events cannot race the writer and terminal errors point to the matching transcript.
Each retry attempt receives its own id and transcript. Attachment and cleanup are best-effort audit metadata: failures are logged but do not fail an otherwise successful dispatch, and an attempt that produces no records leaves no empty file.
The existing cached Git branch lookup is reused so workflow dispatch does not add a per-launch Git subprocess.
No new format
This reuses the ChatRecord-shaped JSONL already consumed elsewhere. A future structured execution ledger for timing, tokens, and terminal results remains a separate additive artifact.
Reviewer Test Plan
How to verify
agentId,agentName, andsessionId.callId, with tool arguments intact.Evidence (Before & After)
N/A — this is a non-UI persistence path. The focused tests assert that the transcript is absent without attachment and present afterward with the launch prompt and pairable tool-call records.
Tested on
Windows and Linux were not tested locally and remain covered by CI.
Environment (optional)
Local macOS workspace with the repository's package-scoped TypeScript, ESLint, and Vitest commands.
Risk & Scope
Linked Issues
Part of #8769.
中文
做了什么
把 harness 现有的 per-subagent transcript writer 挂到 workflow dispatch 上,让每次
agent()调用都在<projectDir>/subagents/<sessionId>/agent-<id>.jsonl留下记录,与 Agent 工具使用相同的位置和格式。为什么需要
workflow dispatch 绕过了 Agent 工具使用的 transcript 挂载,因此 workflow 子 agent 是产品中唯一不留下磁盘记录的 agent。
所有依赖该目录回答“agent 实际做了什么”的能力在 workflow 路径上都不可用,包括失败运行的事后排查、成本统计,以及基于工具调用而非 agent 自述文本的检查。一个没有实际工作的 agent 仍可能生成可信的文字,只有记录下来的工具调用能证明其真实行为。
如何实现
在 dispatch 包装层统一挂载现有 writer。两条 dispatch 路径每次 attempt 共用同一个 event emitter,因此一个挂载点即可覆盖两条路径。agent id 在挂载前生成,避免事件先于 writer 到达,并让终态错误能直接指向对应 transcript。
每次重试都有独立 id 和 transcript。挂载与清理属于 best-effort 审计元数据:失败会记录 debug 日志,但不会让原本成功的 dispatch 失败;没有产生记录的 attempt 不会留下空文件。
复用现有的 Git 分支缓存查询,避免 workflow dispatch 为每次启动新增 Git 子进程。
不引入新格式
继续复用现有消费者已经读取的 ChatRecord JSONL。未来包含耗时、token 与终态结果的结构化 execution ledger 仍可作为独立的增量产物。
Reviewer 测试计划
验证方式
agentId、agentName与sessionId。callId配对,并保留完整工具参数。前后证据
N/A——这是非 UI 的持久化路径。聚焦测试断言挂载前 transcript 不存在,挂载后文件包含 launch prompt 与可配对的工具调用记录。
本地验证平台
Windows 与 Linux 未在本地执行,由 CI 覆盖。
环境
本地 macOS workspace,使用仓库 package 级 TypeScript、ESLint 与 Vitest 命令。
风险与范围
关联 Issue
Part of #8769.