refactor(core): hold Goal sends to the caller's recursion budget, and say why the other carve-outs stay - #10259
refactor(core): hold Goal sends to the caller's recursion budget, and say why the other carve-outs stay#10259qqqys wants to merge 4 commits into
Conversation
… say why the other carve-outs stay Once a Goal had its own autonomous spend bound (QwenLM#9891), the places where client.ts carved runtime Goal turns out of the session ceilings were due for a second look. Read one at a time they are not one thing. Removed: a Goal-type send ignored the caller's `turns` budget and always started from MAX_TURNS. Every runtime continuation is a fresh top-level send that starts from MAX_TURNS on its own, and no production caller passes `turns` for a Goal send, so the carve-out only ever mattered for a caller handing over an exhausted budget -- and for that caller a Goal should be refused like every other message type. It now is, and the interrupted-exit path pauses the Goal instead of leaving it running with a permit nobody will finish. Retained, with the reason written at the site: - sessionTurnCount / maxSessionTurns. The counter increments on every model call the user's prompts drive, tool continuations included, so counting Goal turns would let a user cap of N kill a healthy Goal after ~N/4 continuations with no resume path in headless. The Goal is bounded by its own token budget, and the headless host excludes runtime Goal turns from the same cap, so counting here would split the two ceilings. - The steer-input check on the same count, for the same reason. - The legacy stop-hook Goal chain's undecremented recursion budget. That chain recurses inside one sendMessageStream call, so the runtime's token budget never meters it; it is bounded by MAX_GOAL_ITERATIONS and stopHookBlockingCap, and a 50-iteration chain with steer and next-speaker continues would exhaust MAX_TURNS before its own cap. The 75-turn runtime test no longer passes 0 as the budget; it was only passing because of the removed carve-out. Mutation probe: re-adding the carve-out fails exactly the new test (22 others green).
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
|
Thanks for the PR — a tidy follow-up to the Goal token-budget work in PR 9891. Template looks good ✓ Problem: Not a user-facing bug — this is recorded-debt cleanup. PR 7895 carved runtime Goal turns out of several session ceilings, and the series plan said those exemptions would be re-audited once a Goal had its own spend bound (PR 9891, merged 2026-08-26). The exemptions are real: I verified all four sites exist in Direction: Aligned. The audit keeps what still earns its place and removes only the recursion-budget exemption. The "no behavioral change for production callers" claim checks out: the TUI ( Size: 31 production lines ( Approach: Scope feels exactly right — one behavioral line, three rewritten comments stating why the other carve-outs stay, and a pinning test per site. Nothing I would cut. Risk: No elevated risk signals — no high-risk paths matched the revert-history patterns. One process note up front: this is a fork Moving on to code review. 🔍 中文说明感谢贡献——这是继 PR 9891（Goal token 预算）之后的干净续作。 模板完整 ✓ 问题：不是用户可见的 bug，而是记录在案的债务清理。PR 7895 把运行时 Goal 轮次从多个会话上限中豁免出来，系列计划说好等 Goal 有了自己的消费边界（PR 9891，已于 2026-08-26 合并）后重新审计这些豁免。豁免确实存在： 方向：对齐。审计保留了仍有存在理由的豁免，只撤掉递归预算这一处。「对生产调用方无行为变化」的说法成立：TUI（ 规模：31 行生产代码（ 方案：范围恰到好处——一行行为改动、三处重写注释（说明其他豁免为何保留）、每个位置一个固定测试。没有可砍的。 风险：无升级风险信号——未命中 revert 历史高风险路径。一点流程预告：这是 fork 的 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewBefore reading the diff, my independent take on "audit the Goal ceiling exemptions now that a budget exists" was: remove only the recursion-budget exemption (each runtime continuation is a fresh top-level send, so a Goal never needs to outlive one turn's allowance), keep the session-cap exclusion (counting Goal turns would let a user-set cap kill a healthy Goal mid-run), keep steer admission consistent with it, and keep the legacy hook chain's undecremented budget because the token budget cannot see recursion inside a single send. The PR lands exactly there — and goes one step further by pinning each kept carve-out with a test. Claims I verified against the base tree at the reviewed commit:
Tests: the three new cases pin the removed exemption, steer admission at a hit cap (mock cap is 1, count is forced to 1, and No critical issues, no blockers. TestingEvidence in this section is the PR's own CI, read through the GitHub API — this run does not execute PR code. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The unit suite was still running at the time of this pass; the table updates in place once CI settles. Real-scenario (tmux) coverage: 中文说明代码审查读 diff 之前，我对「预算落地后审计 Goal 上限豁免」的独立方案是：只撤递归预算豁免（每次运行时续跑都是全新的顶层发送，Goal 没有理由超越单轮的递归额度）、保留会话上限排除（计入 Goal 轮次会让用户设定的上限在约 N/4 次续跑后杀死健康 Goal）、steer 准入与之保持一致、旧版 hook 链不递减预算（token 预算看不到单次发送内部的递归）。PR 的落点与此完全一致——并更进一步，为每处保留的豁免补了固定测试。 基于评审提交点的基线代码核实过的说法：
测试：三个新用例分别固定了被撤的豁免、上限触顶时的 steer 准入（mock 上限为 1、计数强制为 1， 无严重问题，无阻塞项。 测试本节的证据是 PR 自己的 CI（通过 GitHub API 读取）——本次运行不执行 PR 代码。评审时单元套件仍在运行，CI 落定后表格会就地更新。真实场景（tmux）覆盖： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review across every stage, but the fork-refactor guardrail needs a maintainer's sign-off; the score reflects the policy cap, not doubt about the change. Stepping back: this is the kind of refactor where the write-up is most of the value, and the write-up survives contact with the code. Every load-bearing claim checked out against the base tree — the four carve-out sites exist as described, no production caller passes an explicit budget on a Goal send, the permit-release in the Why this run does not approve: the PR is cross-repository with a ⏸️ Deferring to @wenshao — review found no blockers; the fork-refactor approval guardrail is the only thing standing between this PR and a merge. Needs a maintainer's sign-off (or another core maintainer's) on 中文说明置信度：3/5——各阶段审查都很干净，但 fork 重构护栏需要 maintainer 签核；这个分数反映的是政策上限，而不是对改动本身的疑虑。 退后一步看：这类重构的价值大半在说明文字里，而这份说明文字经得起与代码对照。所有承重说法都在基线代码上核实过——四处豁免现场与描述一致、没有任何生产调用方给 Goal 发送传显式预算、 本次不批准的原因：该 PR 来自 fork 且标题为 ⏸️ 转交 @wenshao——审查未发现阻塞项；fork 重构审批护栏是这个 PR 与合并之间唯一的障碍。待 CI 变绿后，需要 maintainer（或其他核心维护者）在 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.2)
qwen-code-ci-bot
left a comment
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.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…oal chain The comment said stopHookBlockingCap and MAX_GOAL_ITERATIONS were the only bounds on the legacy stop-hook chain. A user-set maxSessionTurns is a third: each hook hop is a plain send with no Goal permit, so it counts as a session turn, and the cap truncates the chain before either named bound can fire. Measured by the reviewer on this PR's own fixture: with maxSessionTurns=2 the chain dies at hop 2; with the cap off it runs all 7.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action neededNo code changes were made this round: the only actionable finding was already fixed at the current HEAD, and every other feedback item is informational. Feedback triage
No failed checks, no still-red checks, no base conflicts ( Re-verification of R1-1 at HEAD (
|
| 条目 | 来源 | 处置 |
|---|---|---|
rc:3870137543 — [Suggestion] R1-1:注释过度声称 stopHookBlockingCap / MAX_GOAL_ITERATIONS「是这条路径上唯一的边界」,遗漏了用户设置的 maxSessionTurns 同样会截断旧版 hook Goal 链 |
自动审查器 | 已在代码中解决 — 由维护者在提交 cbb04770a4(当前 HEAD)中修复;本轮已重新验证,见下文 |
rc:3870252882 — 「已在 cbb04770a4 中修复 — 采用了你的措辞……」 |
@qqqys(维护者在 R1-1 线程中的回复) | 不是发现 — 是对修复的确认;无需回应 |
rv:5038882639 — 第一轮审查,「无阻断问题,建议见行内」 |
自动审查器 | 信息性内容;其唯一建议即 rc:3870137543 |
rv:5038898247 — 因 CI 仍在运行而从批准降级为评论 |
自动审查器 | 审查器自身覆盖缺口的披露;无可修复项 |
rv:5039013824 — 空审查正文 |
@qqqys | 上述行内回复的审查外壳;无可处理项 |
rv:5039528733 — 在修复后的 HEAD 上进行第二轮审查,零发现 |
自动审查器 | 信息性内容;确认修复后的状态重新审查通过 |
无失败检查,无持续变红的检查,无与基础分支的冲突(--conflict false)。
在 HEAD(cbb04770a4)上对 R1-1 的重新验证
- 注释措辞与被接受的建议一致。
packages/core/src/core/client.ts(约第 4171 行)现在写道:「……链本身由 stopHookBlockingCap / MAX_GOAL_ITERATIONS 约束。这些是这条路径上唯一的 Goal 专属边界(用户设置的 maxSessionTurns 仍会截断这条链:每一跳 hook 都是不带 Goal 许可的普通发送,因此会计入一次会话轮次):旧版 hook Goal 在一次 sendMessageStream 调用内递归……」。不准确的绝对化表述已被移除,并按建议要求点名了第三个边界及其原因。 - 注释的事实性陈述在代码中成立。 旧版链上的每次续发都是一次
type: SendMessageType.Hook、不带goalPermit/goalOrigin的递归sendMessageStream发送(client.ts约 4229–4251 行),因此isGoalRuntimeTurn为 false(约 3109 行),每一跳都会递增sessionTurnCount(约 3269/3279 行),maxSessionTurns拒绝逻辑(约 3301–3315 行)可以截断这条链 —— 与审查器在本 PR 自带的client-goal.test.ts旧版链测试夹具上运行探针得到的数据一致(maxSessionTurns=2时链在第 2 跳带MaxSessionTurns事件终止;关闭上限时跑满全部 7 跳)。
验证
本轮未改动任何源码,因此未运行 build / typecheck / lint / 测试命令 —— 没有新的差异需要验证。当前 HEAD cbb04770a4 已被第二轮自动审查和 CI 覆盖;工作树干净。
resolved-comments.txt 中列出了 rc:3870137543,以便解析该已修复的线程;没有任何行内发现被拒绝、推迟或升级,因此不写 comment-replies.json。
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。
🧠 Handled by Qwen Code · model/模型 qwen3.8-max
3 participants
What this PR does
Audits the places where
client.tscarves runtime-scheduled Goal turns out of the session ceilings, now that a Goal has its own autonomous spend bound (#9891), and acts on each by its own merits. One exemption goes: a Goal-type send no longer ignores the caller's recursion budget (turns) —boundedTurnsisMath.min(turns, MAX_TURNS)for every message type. Three stay, with their comments rewritten to say the real reason they stay rather than the reason that stopped being true when the token budget landed.boundedTurns = Goal ? MAX_TURNS : min(turns, MAX_TURNS)MAX_TURNS= 100 nested steer / next-speaker / hook continuations)MAX_TURNSon its own; no production caller passesturnsfor a Goal send (TUI and headless both use the default; ACP does not go throughclient.sendMessageStreamat all). The carve-out only ever mattered for a caller handing over an exhausted budget, and for that caller a Goal should be refused like everything else — which it now is, and the interrupted-exit path pauses the Goal instead of leaving it running with a permit nobody will finish.if (messageType !== Retry && !isGoalRuntimeTurn)—sessionTurnCount++and themax_session_turnscheckmaxSessionTurns(user-set, default unlimited)sessionTurnCountincrements on every model call the user's prompts drive, tool continuations included. Counting Goal turns would let a user cap of N kill a healthy Goal after roughly N/4 continuations, with no resume path in headless (exit 53). The Goal's spend is bounded by its own token budget, re-armed only by an explicit resume or edit; and the headless host already excludes runtime Goal turns from the same cap (enforceSessionTurnLimit(isRuntimeGoalTurn)), so counting them here would split the two ceilings.takeSteerInput:!isGoalRuntimeTurn && sessionTurnCount >= maxSessionTurnshookTurnBudget = activeGoal ? boundedTurns : boundedTurns - 1sendMessageStreamcall, so the runtime's token budget — which meters continuations the Goal runtime schedules — never sees it. Its bounds areMAX_GOAL_ITERATIONS(50) andstopHookBlockingCap(8, pauses the Goal); a 50-iteration chain with steer and next-speaker continues would exhaustMAX_TURNSbefore its own cap, so decrementing here would trade one runaway guard for a new way to kill a legitimate run.Two further goal-runtime carve-outs in this file are not ceilings and are untouched: the hook-re-entry microcompaction checkpoint and the managed auto-memory background tasks are skipped for runtime turns. Neither has anything to do with budget; they are noted here so the next reader does not mistake them for the four above.
Why it's needed
The series plan recorded these as "the
client.tsgoal-turn ceiling exemptions to remove once a budget exists" (#9891 is that budget). Read one by one they are not one thing: one is a recursion budget the Goal had no reason to ignore, two are a user-set session cap whose semantics (every model call counts) would make Goals unusable under it, and one guards a legacy path the token budget cannot reach. Removing all four on the strength of "there is a budget now" would have regressed the two user-visible ones. The value of this PR is as much the recorded reasoning as the one-line removal: every remaining exemption now carries the reason it actually stays.Reviewer Test Plan
How to verify
cd packages/core && npx vitest run src/core/client.test.ts src/core/client-goal.test.ts src/goals/— 18 files, 824 tests. New case:holds a runtime Goal turn to the caller recursion budget like any other send(a Goal send withturns = 0does not run the model, pauses the Goal through the interrupted-exit path, and finishes the permit). The existing 75-turn runtime test no longer passes0as the budget — it was only ever passing because of the carve-out this PR removes; with the default budget it still runs all 75 turns and still leavessessionTurnCountat 0, which pins the retained session-turn exclusion.messageType === SendMessageType.Goal ? MAX_TURNS : …fails exactly the new test (22 others green); restored, 23/23.npx tsc --noEmitinpackages/core: 32 errors with and without the diff (pre-existing dependency skew outside this change, verified by exporting the diff, checking out the pristine files, and re-applying — nogit stash). prettier + eslint clean on both files.Evidence (Before & After)
N/A (no user-visible change for any production caller; the only behavioral change is for a caller passing an exhausted
turnsbudget to a Goal send, which none does).Tested on
Environment (optional)
N/A (unit tests only).
Risk & Scope
maxSessionTurnsexclusion means a user who sets a session-turn cap still sees Goals run past it. That is deliberate and now documented at the site: the Goal is bounded by its own token budget, and the headless host applies the same exclusion, so the alternative is two ceilings that disagree. If the project later wants Goals under the session cap, the right shape is a per-spend-window count reset on resume, not this exclusion's removal.enforceSessionTurnLimitexclusion (mirrors the retained one; untouched). The attribution-snapshot and auto-memory carve-outs for runtime turns (not ceilings; noted above). ACP, which drivesGeminiChatdirectly and never passes through these checks.Linked Issues
Supersedes refactor(core): hold Goal sends to the caller's recursion budget, and say why the other carve-outs stay #10126 — same two commits, new PR number. That PR's two automatic review runs both computed Approve, 0 Critical, 0 Suggestion and both failed to post: a stale empty PENDING review by the review bot on that PR made every
review submitreturn 422 ("one pending review per user per pull request"), and the pending object is bound to the PR number, so only a fresh PR gets a clean slate.Depends on feat(goal): stop autonomous continuation at a token budget the user re-arms #9891 (the token budget that makes the audit possible).
中文说明
这个 PR 做了什么
审计
client.ts中把运行时调度的 Goal 轮次排除在会话上限之外的各处豁免——现在 Goal 已经有了自己的自主消费边界(#9891)——并逐一按各自的理由处理。撤掉一处:Goal 类型的发送不再忽略调用方给的递归预算(turns),boundedTurns对所有消息类型统一为Math.min(turns, MAX_TURNS)。保留三处,并重写注释,写明它们真正保留的理由,而不是那个在 token 预算落地后已不再成立的理由。boundedTurns = Goal ? MAX_TURNS : min(turns, MAX_TURNS)MAX_TURNS= 100 层嵌套的 steer / next-speaker / hook 续跑)MAX_TURNS起步;生产中没有任何调用方给 Goal 发送传turns(TUI 和 headless 都用默认值;ACP 根本不经过client.sendMessageStream)。这个豁免只对「调用方递交一个已耗尽预算」的情形有意义,而这种情形下 Goal 应该和其他类型一样被拒绝——现在正是如此,并且中断退出路径会暂停 Goal,而不是让它带着一个无人完成的 permit 继续「运行」。if (messageType !== Retry && !isGoalRuntimeTurn)——sessionTurnCount++与max_session_turns检查maxSessionTurns(用户设定,默认不限)sessionTurnCount对用户提示驱动的每一次模型调用递增,含工具续跑。若把 Goal 轮次计入,用户设的上限 N 会让一个健康的 Goal 在大约 N/4 次续跑后中途死亡,而 headless 中没有恢复路径(退出码 53)。Goal 的消费由它自己的 token 预算约束,且只能由显式的 resume 或 edit 重新武装;headless host 已经把运行时 Goal 轮次排除在同一上限之外(enforceSessionTurnLimit(isRuntimeGoalTurn)),在这里计入会让两处上限分裂。takeSteerInput:!isGoalRuntimeTurn && sessionTurnCount >= maxSessionTurnshookTurnBudget = activeGoal ? boundedTurns : boundedTurns - 1sendMessageStream调用内递归,而 runtime 的 token 预算只计量 Goal runtime 调度的续跑,永远看不到它。它的边界是MAX_GOAL_ITERATIONS(50)和stopHookBlockingCap(8,会暂停 Goal);一条 50 次迭代、带 steer 与 next-speaker 续跑的链会在触及自己的上限之前先耗尽MAX_TURNS,所以在这里递减等于用一种新的杀死合法运行的方式换掉一个失控防护。本文件中还有两处针对 goal-runtime 的豁免不是上限,未改动:hook 重入时的 microcompaction 检查点,以及托管自动记忆的后台任务,对运行时轮次都会跳过。二者与预算无关;在此注明,以免下一位读者把它们误认为上述四处之一。
为什么需要
系列计划把这些记录为「预算落地后要撤掉的
client.tsgoal 轮次上限豁免」(#9891 就是那个预算)。逐一读过之后它们并不是同一件事:一处是 Goal 没有理由忽略的递归预算,两处是一个用户设定的会话上限、其语义(每次模型调用都计数)会让 Goal 在其下无法使用,还有一处守护着 token 预算触及不到的旧路径。仅凭「现在有预算了」就全部撤掉,会让其中两处用户可见的行为倒退。本 PR 的价值既在于那一行删除,更在于记录下来的推理:每一处保留的豁免现在都写明了它真正保留的原因。评审验证计划
如何验证
cd packages/core && npx vitest run src/core/client.test.ts src/core/client-goal.test.ts src/goals/——18 个文件,824 个测试。新增用例:holds a runtime Goal turn to the caller recursion budget like any other send(turns = 0的 Goal 发送不会调用模型,通过中断退出路径暂停 Goal,并完成 permit)。原有的 75 轮运行时测试不再传0作为预算——它此前之所以能过,只是因为本 PR 撤掉的这个豁免;改用默认预算后它仍跑完全部 75 轮,sessionTurnCount仍为 0,这固定住了保留下来的会话轮次排除。messageType === SendMessageType.Goal ? MAX_TURNS : …,恰好挂新测试(其余 22 绿);恢复后 23/23。packages/core的npx tsc --noEmit:有无 diff 均为 32 个错误(本变更之外的已有依赖偏差,通过导出 diff、检出原始文件、再重新应用来验证——未使用git stash)。两个文件 prettier + eslint 干净。证据(前后对比)
N/A(对任何生产调用方都没有用户可见的变化;唯一的行为变化针对给 Goal 发送传入已耗尽
turns预算的调用方,而现实中没有这样的调用方)。已测试平台
Linux ✅;macOS / Windows⚠️ (CI 覆盖)。
环境(可选)
N/A(仅单元测试)。
风险与范围
maxSessionTurns排除意味着设置了会话轮次上限的用户仍会看到 Goal 超出该上限运行。这是刻意的并已在现场注明:Goal 由自己的 token 预算约束,headless host 也采用同样的排除,否则就是两个互相矛盾的上限。如果项目日后希望 Goal 受会话上限约束,正确的形态是按消费窗口计数并在 resume 时重置,而不是撤掉这个排除。enforceSessionTurnLimit排除(与保留的那处一致;未改动)。运行时轮次的 attribution snapshot 与自动记忆豁免(不是上限;上文已注明)。ACP 直接驱动GeminiChat,从不经过这些检查。关联 Issue
取代 refactor(core): hold Goal sends to the caller's recursion budget, and say why the other carve-outs stay #10126——同样的两个 commit,换一个 PR 号。原 PR 的两轮自动评审均判定 Approve、0 Critical、0 Suggestion,但都未能发出:评审机器人在该 PR 上残留了一个空的 PENDING review,使每次
review submit都返回 422(每用户每 PR 只允许一个 pending review),而该对象绑定在 PR 号上,只有新 PR 才能重新开始。依赖 feat(goal): stop autonomous continuation at a token budget the user re-arms #9891(使本次审计成为可能的 token 预算)。