fix(sdk-java): correct TIMEOUT_30_MINUTES to actually be 30 minutes - #7188
Conversation
The constant was defined as new Timeout(60L, TimeUnit.MINUTES) in both
the client and qwencode modules, i.e. 60 minutes, contradicting its name
and Javadoc ("A timeout of 30 minutes."). It is the fallback turn timeout
in ProcessTransport and the timeout used by QwenCodeCli.simpleQuery, so
callers waited twice the documented default. The 30-minute intent is
corroborated by TransportOptionsAdapter.DEFAULT_TURN_TIMEOUT (30 min).
Change the literal to 30L in both modules.
|
Thanks for the PR! Template looks good ✓ — all required sections present with bilingual translation. Problem: observed bug with clear evidence. Direction: straightforward bug fix — correcting a constant to match its documented intent. Aligned. Size: not applicable — 2 production lines in Approach: minimal and focused. Two files, one literal each ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ — 所有必需章节齐全，含双语翻译。 问题：已观测到的 bug，证据明确。 方向：简单的 bug 修复——将常量修正为其文档所述的意图。对齐。 规模：不适用—— 方案：最小化且聚焦。两个文件，各改一个字面量（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewIndependent proposal: change Diff matches proposal exactly. Two files, one literal each, nothing else. No critical issues found. The change is a VerificationBoth modules compile cleanly ( Client module unit tests pass (9/9): The qwencode module's tests are integration tests that spawn a live Qwen Code CLI process — they time out without a running daemon, which is expected and unrelated to this change. tmux testing: not applicable for this PR. This is a Java SDK constant-value fix with no interactive CLI behavior to capture. The build verification above is the appropriate test. No test pins the old value — 中文说明代码审查独立方案： 将两个 Diff 与方案完全一致。 两个文件，各一个字节面量，无其他改动。 未发现关键问题。改动为既有 验证两个模块均编译通过（ client 模块单元测试通过（9/9）： qwencode 模块的测试为集成测试，需要运行中的 Qwen Code CLI 进程——无守护进程时会超时，这是预期行为，与本改动无关。 tmux 测试： 本 PR 不适用。这是 Java SDK 常量值修正，无可捕获的交互式 CLI 行为。上述构建验证即为合适的测试方式。 无测试锁定旧值 — 两个模块中 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 This is about as clear-cut as it gets. A constant named The only risk noted in the PR — callers unknowingly relying on the 60-minute actual value — is real but low-probability: the constant's name, docs, and sibling default all advertise 30 minutes, and any caller needing more can pass an explicit LGTM, approving. ✅ 中文说明Confidence: 5/5 这个 PR 非常清晰。名为 PR 中提到的唯一风险——调用方无意中依赖了实际的 60 分钟值——确实存在但概率很低：常量的名称、文档和兄弟默认值都声明 30 分钟，任何需要更长时间的调用方均可显式传入 LGTM, 批准。✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: Agent 0: Issue fidelity & root-cause ownership — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 1a: Line-by-line correctness — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 2: Security — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 3: Code quality — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 4: Performance & efficiency — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 5: Test coverage — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 6a: Undirected audit — attacker mindset — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 6b: Undirected audit — 3 AM oncall mindset — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 6c: Undirected audit — six-months-later maintainer — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 1b: Removed-behavior audit — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 1c: Cross-file tracer — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 0: Issue fidelity & root-cause ownership — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 1a: Line-by-line correctness — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 2: Security — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 3: Code quality — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 4: Performance & efficiency — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 5: Test coverage — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 6a: Undirected audit — attacker mindset — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 6b: Undirected audit — 3 AM oncall mindset — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 6c: Undirected audit — six-months-later maintainer — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 1b: Removed-behavior audit — its prompt was built, but no agent on record was launched with it. Not reviewed: Agent 1c: Cross-file tracer — its prompt was built, but no agent on record was launched with it.
— qwen3.7-max via Qwen Code /review
…coverage-side collapse
Review follow-through on three confirmed Criticals, which converge on
one design:
- The all-built-none-launched collapse misfired on the exhibit it was
built for: candidatesOf is also all-empty when every agent ran on a
REWRITTEN prompt, so the aggregate claimed "the run stopped at the
prompt builder" beside forty-three rewritten-launch disclosures that
said otherwise. Removed -- and it also discarded the per-role subjects
before the caller's echoes could dedupe against them.
- The last-em-dash subject/reason split reparsed rendered prose, and a
reason is free-form: labels carry a dash for an invariant's file,
error interpolations carry anything. Coverage now hands compose the
entries as {subject, reason} pairs (a new `disclosures` field beside
the prose twins the stderr formatting keeps); the caller's entries are
never parsed at all -- they dedupe by prefix against known coverage
subjects, exactly as the chunk list already did, and render verbatim
otherwise.
- With subjects structural and per-role entries surviving to the dedup,
the same-reason grouping does the collapsing for every shape at once:
the #7188 wall, the #7166 wall, and the caller-echo variant all render
as one sentence per cause -- with each subject under its most precise
cause only.
…se an all-built-none-launched roster (QwenLM#7190) * fix(review): one disclosure per subject — dedupe the Not-reviewed list, collapse an all-built-none-launched roster A public review body on QwenLM#7188 was a wall of twenty-two "Not reviewed" clauses for eleven roles: the run built every roster prompt, launched not one agent, and every disclosure appeared twice. Two defects, one wall: - compose-review's `unreviewed` list fills from two sides -- the caller's `unreviewedDimensions` and the coverage recomputation -- and a run that pastes the gate's own gap lines into its input posts every one of them twice. The chunk list has deduped by its `chunk <id>` prefix since that exact bug shipped there; the role list now dedupes by label too (everything before the entry's last em-dash segment, because an invariant agent's label legitimately carries one). When both sides name the same subject, the coverage-derived text wins -- it is the evidence-bounded register the body is written in. - coverage collapses "nobody BUILT anything" to one line, but the launched-side twin -- every prompt built, none launched -- was still one line per dimension, burying the single fact that explains all of them: the run stopped at the builder. Collapsed the same way, one line for the run; the per-role selectors survive for the repair. * fix(review): group Not-reviewed disclosures by cause, prefer the precise text on collision Second exhibit, same wall: the review posted on QwenLM#7166 was ninety-nine disclosure paragraphs over four causes -- forty-three chunks all launched with rewritten prompts, fifty-five roles built and never launched -- with the six real findings buried beneath. Neither QwenLM#7190 mechanism touched it: nothing was duplicated, and the roster was not all-unlaunched (the chunk agents ran). - Explained disclosures now group by their reason text, the same last em-dash segment the dedup keys on: same cause, one sentence, every subject on it -- exactly as the bare names already grouped under the shared whiff sentence. A reason embedding per-subject detail (an unread brief's path) differs per entry and keeps its own line. - The dedup now prefers coverage-derived text over the caller's AND the earliest coverage category on a within-coverage collision: a rewritten chunk is also, to the roster, a requirement with no verbatim launch, and the later roster text told the author "no agent was launched" about an agent that demonstrably ran. * fix(review): make the discarded-suggestions clause self-contained in the posted body "see the terminal output" pointed the PR author at a terminal only the operator has -- a dead reference that eight hours of real bot reviews carried onto five different pull requests. Say what happened and that nothing is owed, in the body's own register. * fix(review): carry disclosures structurally -- no reparsed prose, no coverage-side collapse Review follow-through on three confirmed Criticals, which converge on one design: - The all-built-none-launched collapse misfired on the exhibit it was built for: candidatesOf is also all-empty when every agent ran on a REWRITTEN prompt, so the aggregate claimed "the run stopped at the prompt builder" beside forty-three rewritten-launch disclosures that said otherwise. Removed -- and it also discarded the per-role subjects before the caller's echoes could dedupe against them. - The last-em-dash subject/reason split reparsed rendered prose, and a reason is free-form: labels carry a dash for an invariant's file, error interpolations carry anything. Coverage now hands compose the entries as {subject, reason} pairs (a new `disclosures` field beside the prose twins the stderr formatting keeps); the caller's entries are never parsed at all -- they dedupe by prefix against known coverage subjects, exactly as the chunk list already did, and render verbatim otherwise. - With subjects structural and per-role entries surviving to the dedup, the same-reason grouping does the collapsing for every shape at once: the QwenLM#7188 wall, the QwenLM#7166 wall, and the caller-echo variant all render as one sentence per cause -- with each subject under its most precise cause only. * refactor(review): single-source each disclosure's prose from its structural entry Review follow-through: every reason lived twice -- the prose push that feeds check-coverage's stderr and the disclose() that feeds the posted body -- and an edit to one and not the other would silently diverge what the operator reads from what the author is told. disclose() now returns the joined prose and the stderr arrays consume it, so there is one statement of each sentence. Plus the two nits: the needless covEntries copy, and a -1 guard on the verification-gap split (safe today by construction, latent tomorrow).
…n relays Naming idle and unopened agents by their brief's publicLabel put those labels into coverage subjects — the very register the orchestrator spells its scoped whiff relays in. The caller-echo filter then prefix-matched `reverse audit — chunk 2's auditor returned nothing substantive twice` against an idle `reverse audit` entry and dropped the disclosure the skill promises renders verbatim, in exactly the runs where a whiffed auditor makes such a relay likeliest. Entries named by a role's publicLabel — the budget stop's, idle/unopened agents', and the Step 4/5 floor's — now never prefix-match, the way the budget-stop entry already did not; a bare subject echo still dedups, and chunk and internal subjects keep the #7188 dedup. Three suggestions from the same round: a file-scoped launch keeps its file (`the invariant check (...) on src/a.ts`), resolved through the roster requirement rather than a key split — a split would misread `verify--<digest>` and `reverse-audit--chunk-N` as file-scoped and leak a digest onto the PR page; the role label is now resolved once at the single name-derivation point instead of wrapped at three push sites; and the bracket-strip gains a positive survival case, so a drop-if-wrapped regression can no longer pass the suite green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
3 participants
What this PR does
Corrects the value of
Timeout.TIMEOUT_30_MINUTESin the Java SDK, which wasnew Timeout(60L, TimeUnit.MINUTES)— i.e. 60 minutes, twice what its name and Javadoc declare. It changes the literal to30Lin both copies of the constant:packages/sdk-java/qwencode/.../cli/utils/Timeout.javapackages/sdk-java/client/.../acp/sdk/utils/Timeout.javaWhy it's needed
The constant is named
TIMEOUT_30_MINUTESand its Javadoc says "A timeout of 30 minutes.", but the value is 60 minutes. It is not a cosmetic mislabel — it is the effective default in two behavioral paths:ProcessTransportuses it as the fallback turn timeout when the caller does not set one:orElse(Timeout.TIMEOUT_30_MINUTES).QwenCodeCli.simpleQuery(...)passes it directly as the turn timeout.Neither caller supplies an explicit value, so both wait 60 minutes where the API contract promises 30. That 30-minute intent is corroborated independently by
TransportOptionsAdapter.DEFAULT_TURN_TIMEOUT = new Timeout(1000 * 60 * 30L, TimeUnit.MILLISECONDS)(= 30 minutes), the sibling default for the same turn-timeout setting. The name, the Javadoc, and the parallel adapter default all agree on 30; only this literal disagrees. Fixing the literal (rather than renaming toTIMEOUT_60_MINUTES) aligns the value with all three.Reviewer Test Plan
How to verify
Timeout.TIMEOUT_30_MINUTES.getValue()is now30LwithgetUnit() == TimeUnit.MINUTESin both modules.packages/sdk-java/client/.../transport/process/ProcessTransport.java—orElse(Timeout.TIMEOUT_30_MINUTES)packages/sdk-java/qwencode/.../cli/QwenCodeCli.java—simpleQuery(..., Timeout.TIMEOUT_30_MINUTES)grep -rn "TIMEOUT_30_MINUTES" */src/testreturns nothing).Evidence (Before & After)
Timeout.java(both modules):Before:
public static final Timeout TIMEOUT_30_MINUTES = new Timeout(60L, TimeUnit.MINUTES);→ resolves to 60 minutes; callers wait 2× the documented default.After:
public static final Timeout TIMEOUT_30_MINUTES = new Timeout(30L, TimeUnit.MINUTES);→ matches the name, the Javadoc, andDEFAULT_TURN_TIMEOUT(30 min).Tested on
macOS: static review only — no Java toolchain in the review environment. The change is a single
longliteral (60L→30L) inside an existingnew Timeout(long, TimeUnit)call, so it cannot alter compilation or types; and no test pins the old value. The behavioral effect (fallback turn timeout /simpleQuerytimeout) is 30 minutes instead of 60, as documented.Environment (optional)
@qwen-code/sdk-java(clientandqwencodeMaven modules).Risk & Scope
Timeout.Linked Issues
None.
中文说明
本 PR 的作用
修正 Java SDK 中
Timeout.TIMEOUT_30_MINUTES的取值。该常量原为new Timeout(60L, TimeUnit.MINUTES)，即 60 分钟，是其名称与 Javadoc 所声明时长的两倍。本 PR 在该常量的两份副本中把字面量改为30L：packages/sdk-java/qwencode/.../cli/utils/Timeout.javapackages/sdk-java/client/.../acp/sdk/utils/Timeout.java为什么需要
该常量名为
TIMEOUT_30_MINUTES，Javadoc 写明"A timeout of 30 minutes."，但其值为 60 分钟。这并非单纯的命名错误——它是两条行为路径上实际生效的默认值：ProcessTransport在调用方未设置时，用它作为**轮次超时（turn timeout）**的兜底值：orElse(Timeout.TIMEOUT_30_MINUTES)。QwenCodeCli.simpleQuery(...)直接把它作为轮次超时传入。两处调用方都未显式传值，因此在 API 约定为 30 分钟之处实际等待 60 分钟。30 分钟的意图另有佐证：
TransportOptionsAdapter.DEFAULT_TURN_TIMEOUT = new Timeout(1000 * 60 * 30L, TimeUnit.MILLISECONDS)（= 30 分钟），即同一轮次超时设置的另一处默认值。名称、Javadoc、以及并行的 adapter 默认值三者都指向 30，唯有此字面量不一致。修正字面量（而非改名为TIMEOUT_60_MINUTES）使取值与三者一致。复核测试计划
如何验证
Timeout.TIMEOUT_30_MINUTES.getValue()现为30L，getUnit() == TimeUnit.MINUTES。packages/sdk-java/client/.../transport/process/ProcessTransport.java—orElse(Timeout.TIMEOUT_30_MINUTES)packages/sdk-java/qwencode/.../cli/QwenCodeCli.java—simpleQuery(..., Timeout.TIMEOUT_30_MINUTES)grep -rn "TIMEOUT_30_MINUTES" */src/test无结果）。证据（修复前后对比）
Timeout.java（两个模块）：修复前：
public static final Timeout TIMEOUT_30_MINUTES = new Timeout(60L, TimeUnit.MINUTES);→ 解析为 60 分钟；调用方等待文档默认值的两倍。修复后：
public static final Timeout TIMEOUT_30_MINUTES = new Timeout(30L, TimeUnit.MINUTES);→ 与名称、Javadoc 及DEFAULT_TURN_TIMEOUT（30 分钟）一致。测试环境
macOS：仅做静态复核——复核环境无 Java 工具链。改动仅为既有
new Timeout(long, TimeUnit)调用内的一个long字面量（60L→30L），不会影响编译或类型；且无测试锁定旧值。行为影响（兜底轮次超时 /simpleQuery超时）由 60 分钟变为文档所述的 30 分钟。运行环境（可选）
@qwen-code/sdk-java（client与qwencode两个 Maven 模块）。风险与影响范围
Timeout。关联 Issue
无。