Add stop hook blocking cap by qqqys · Pull Request #4208 · QwenLM/qwen-code

qqqys · 2026-05-16T13:02:42Z

Summary

What changed: Added a configurable consecutive Stop/SubagentStop hook blocking cap, wired it through core config, CLI settings, ACP, and subagent stop loops.
Why it changed: A blocking Stop hook can otherwise keep a turn continuing for too long before broader turn budgets stop it. The cap gives users a faster recovery path while keeping the existing goal iteration guard as a secondary safety net.
Reviewer focus: Main Stop hook continuation, ACP parity, SubagentStop parity, and the new settings/env override behavior.

Validation

Commands run:

cd packages/core && npx vitest run src/hooks/stopHookCap.test.ts src/core/client.test.ts src/tools/agent/agent.test.ts
npm run build
npm run typecheck
git diff --check

Prompts / inputs used: N/A, focused unit coverage for the loop guard and config parsing.
Expected result: Stop/SubagentStop hook loops stop once the configured blocking cap is reached, and normal hook pass-through remains unchanged.
Observed result: 205 focused tests passed; build, typecheck, and diff whitespace check passed.

Quickest reviewer verification path:

cd packages/core && npx vitest run src/hooks/stopHookCap.test.ts src/core/client.test.ts src/tools/agent/agent.test.ts

Evidence: Local command output showed 3 test files passed, 205 tests passed.

Scope / Risk

Main risk or tradeoff: Users with intentionally long-running blocking Stop hooks may need to raise stopHookBlockingCap or QWEN_CODE_STOP_HOOK_BLOCK_CAP.
Not covered / not validated: Full interactive terminal E2E was not run.
Breaking changes / migration notes: Default cap is 8 consecutive blocking Stop/SubagentStop decisions.

Testing Matrix

	🍏	🪟	🐧
npm run	✅	⚠️	⚠️
npx	✅	⚠️	⚠️
Docker	N/A	N/A	N/A
Podman	N/A	N/A	N/A
Seatbelt	N/A	N/A	N/A

Testing matrix notes:

macOS local validation covered focused vitest, build, typecheck, and diff whitespace checks.
Windows and Linux were not tested locally.

Linked Issues / Bugs

Closes #4206

github-actions · 2026-05-16T13:04:55Z

📋 Review Summary

This PR introduces a configurable consecutive blocking cap for Stop/SubagentStop hooks to prevent turns from continuing indefinitely when blocking hooks keep firing. The implementation is well-structured with proper configuration wiring across CLI settings, core config, ACP integration, and subagent stop loops. The default cap of 8 iterations provides a reasonable safety net while allowing users to customize via settings or environment variable.

🔍 General Feedback

Architecture: Clean separation of concerns with the new stopHookCap.ts module handling normalization, resolution, and formatting logic independently from the hook execution paths.
Consistency: Excellent parity across all three Stop hook loop locations (Session, GeminiClient, BackgroundAgentResumeService/AgentTool).
Test Coverage: Focused unit tests for the new utility functions and integration tests for the cap behavior in client and agent tests.
Configuration Chain: Proper precedence (env var > config > default) with sensible validation and fallback behavior.
Documentation: Clear descriptions in settings schema and helpful inline comments explaining the why behind the cap.

🎯 Specific Feedback

🟡 High

packages/cli/src/acp-integration/session/Session.ts:763-777 - The Stop hook loop now emits emitStopHookLoop on every iteration (including the first), whereas previously it only emitted after the first iteration. This changes the event semantics and could affect downstream consumers of this event. Consider whether this behavioral change is intentional or if the condition stopHookIterationCount > 1 should be preserved.
packages/core/src/core/client.ts:1579-1598 - The cap check happens after incrementing currentIterationCount and building the response, but before the continueRequest is created. However, the emitStopHookLoop event is still emitted before checking the cap (line 1573-1578), which means the event fires even when the cap is exceeded and the turn ends. This could be confusing for event consumers.

🟢 Medium

packages/core/src/hooks/stopHookCap.ts:10-16 - The normalizeStopHookBlockingCap function treats 0 as invalid and falls back to default. While this prevents accidental disabling, users who explicitly want to disable the cap have no way to do so (the env var would also normalize 0 to default). Consider whether disabling should be possible (e.g., via -1 or Infinity) for advanced users who understand the risks.
packages/core/src/hooks/stopHookCap.ts:30-34 - The formatStopHookBlockingCapWarning function uses hardcoded hook labels. Consider making this more extensible if other hook types might need similar caps in the future, or add a comment explaining why only these two hook types need this protection.
packages/cli/src/config/settingsSchema.ts:1954-1961 - The setting has showInDialog: false, which is appropriate for advanced users. Consider adding this to the PR description or a comment explaining why this setting is hidden from the UI (to prevent casual modification of a safety feature).

🔵 Low

packages/core/src/hooks/stopHookCap.test.ts:20-28 - The test for normalizeStopHookBlockingCap covers undefined, 0, and NaN, but not negative numbers or non-integer floats. Consider adding tests for normalizeStopHookBlockingCap(-5) and normalizeStopHookBlockingCap(3.7) to verify the Math.floor and >= 1 check behavior.
packages/core/src/agents/background-agent-resume.ts:986-1027 - The SubagentStop hook loop uses this.config.getStopHookBlockingCap() but the warning message format differs slightly from the Session and Client implementations. The message includes the [BackgroundAgentResume] prefix but the core warning text is consistent. This is fine, but worth noting for future log parsing.
packages/core/src/tools/agent/agent.ts:977-1018 - Similar to background-agent-resume, the SubagentStop loop here also has a context-specific prefix in the warning. Consider whether these warnings should be identical for easier log aggregation, or if the context prefix is valuable for debugging.
PR Description - The testing matrix shows Windows and Linux as "⚠️" (not tested). Given this is a core behavioral change affecting hook execution, consider noting whether CI covers these platforms or if cross-platform testing is recommended before merge.

✅ Highlights

packages/core/src/hooks/stopHookCap.ts - Excellent utility module design with clear separation of normalization, resolution, and formatting concerns. The env var override pattern is well-implemented.
packages/core/src/goals/goalHook.ts:123-142 - The new abortGoalForStopHookCap function cleanly integrates with the existing goal lifecycle, properly finishing the goal with a system message rather than abruptly terminating.
packages/core/src/core/client.test.ts:4336-4389 - Well-structured test that verifies both the StopHookLoop event emission and the HookSystemMessage output when the cap is reached, providing clear verification of the expected behavior.
Configuration Integration - The wiring through CLI config, settings schema, core config, and VSCode schema is thorough and maintains consistency across all configuration surfaces.
Default Value Choice - Default of 8 consecutive blocking decisions strikes a good balance between allowing legitimate long-running hooks to work while providing protection against runaway loops.

wenshao · 2026-05-16T13:42:03Z

+  }
+
+  const normalized = Math.floor(value);
+  return normalized >= 1 ? normalized : DEFAULT_STOP_HOOK_BLOCK_CAP;


[Critical] normalizeStopHookBlockingCap has no upper bound — only enforces >= 1. A large value like 99999 passes through. In client.ts, the Stop hook loop is recursive (yield* this.sendMessageStream(...)), so each blocking decision adds a stack frame. A large cap will cause a "Maximum call stack size exceeded" crash long before hitting the iteration cap.

Suggested change

return normalized >= 1 ? normalized : DEFAULT_STOP_HOOK_BLOCK_CAP;

const MAX_STOP_HOOK_CAP = 100;

return normalized >= 1 ? Math.min(normalized, MAX_STOP_HOOK_CAP) : DEFAULT_STOP_HOOK_BLOCK_CAP;

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T13:42:03Z

+
+export function resolveStopHookBlockingCap(configValue?: number): number {
+  const envValue = process.env[STOP_HOOK_BLOCK_CAP_ENV];
+  if (envValue !== undefined) {


[Suggestion] process.env[STOP_HOOK_BLOCK_CAP_ENV] returns "" (not undefined) when the env var is set to empty — a common pattern in Docker/k8s configs. "" !== undefined is true, so Number("") = 0, then normalizeStopHookBlockingCap(0) = 8. An empty env var should be treated as "not set" and fall through to the config value.

Suggested change

if (envValue !== undefined) {

if (envValue !== undefined && envValue !== '') {

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T13:42:03Z

-    const maxIterations = 5;
+    const maxIterations = this.config.getStopHookBlockingCap();

    for (let i = 0; i < maxIterations; i++) {


[Suggestion] Stop/SubagentStop cap semantics have an off-by-one inconsistency. Stop hook paths (client.ts, Session.ts) check currentIterationCount >= cap after increment, allowing cap-1 retries. SubagentStop paths (agent.ts, background-agent-resume.ts) use for (let i = 0; i < maxIterations; i++), allowing exactly cap retries. For cap=8: Stop gets 7 retries, SubagentStop gets 8. Consider unifying: change the loop condition to i < maxIterations - 1 or restructure to match Stop's count-and-check pattern.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T14:05:56Z

@@ -1014,7 +1015,10 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
    }

    debugLogger.warn(


[Suggestion] When the SubagentStop cap is reached in agent.ts and background-agent-resume.ts, the warning is only logged via debugLogger.warn — never surfaced to the user. Contrast with client.ts (emits HookSystemMessage event visible in TUI) and Session.ts (calls emitAgentMessage to ACP clients). Users have no way to know their SubagentStop hook was overridden, breaking trust in hook transparency.

Consider emitting the warning through a user-visible channel (e.g., appending to the subagent's ToolResult.llmContent or returnDisplay), consistent with how Stop hook cap warnings reach the user.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao

No additional review findings. Downgraded from Approve to Comment: CI failing: Post Coverage Comment, Test (windows-latest, Node 22.x). — gpt-5.5 via Qwen Code /review

wenshao

Test coverage gaps: Three integration points for the new cap have no dedicated tests:

abortGoalForStopHookCap in goalHook.ts — no test exists for either branch (active goal present vs absent)
ACP Session.test.ts — no test exercises the cap-enforcement path in runStopHookLoop
background-agent-resume.test.ts — no test exercises the SubagentStop cap enforcement in handleSubagentStopLoop

All targeted unit tests (stopHookCap, client, agent) pass: 226 tests green.

wenshao · 2026-05-16T15:28:19Z

@@ -1014,7 +1026,10 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
    }


[Suggestion] Dead code: the debugLogger.warn after the for-loop is unreachable. The in-loop cap check currentIterationCount >= maxIterations always fires on the last iteration (i = maxIterations - 1) and executes return, so the loop can never exit naturally.

Suggested change

}

// (remove the post-loop debugLogger.warn block — it is unreachable)

}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T15:28:19Z

@@ -1022,7 +1034,10 @@ export class BackgroundAgentResumeService {
    }


[Suggestion] Same dead-code pattern as agent.ts: the debugLogger.warn after the for-loop is unreachable because the in-loop cap check always returns on the final iteration.

Suggested change

}

// (remove the post-loop debugLogger.warn block — it is unreachable)

}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T15:28:19Z

+export const DEFAULT_STOP_HOOK_BLOCK_CAP = 8;
+export const MAX_STOP_HOOK_BLOCK_CAP = 100;
+export const STOP_HOOK_BLOCK_CAP_ENV = 'QWEN_CODE_STOP_HOOK_BLOCK_CAP';
+


[Suggestion] DEFAULT_STOP_HOOK_BLOCK_CAP = 8 is a significant drop from the old hardcoded limits (100 for Stop hook in client.ts/Session.ts, 5 for SubagentStop). Users with iterative Stop hooks that relied on >8 retries will see a silent behavioral regression after upgrading. The warning message does not mention the old limit or how to restore it (QWEN_CODE_STOP_HOOK_BLOCK_CAP=100). Consider either bumping the default for backward compatibility or surfacing a one-time deprecation notice on the first cap hit.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T15:28:20Z

@@ -1559,15 +1561,26 @@ export class GeminiClient {
            continueReason,


[Suggestion] The design rationale comment explaining why StopHookLoop is emitted on the first blocking iteration (not just subsequent ones) was deleted. This context — /goal needing first-iteration visibility, and configured Stop hooks needing their blocking reason surfaced — helps future maintainers understand the intentional >= cap check placement relative to the yield.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T17:35:59Z

        stopHookReasons = [...stopHookReasons, continueReason];

-        // Emit StopHookLoop event for iterations after the first one
+        if (stopHookIterationCount >= stopHookBlockingCap) {


[Critical] #handleStopHookLoop 的 cap-enforcement 路径未测试。config mock 已添加 getStopHookBlockingCap: vi.fn().mockReturnValue(8)，但没有测试将 cap 设为低值（如 1 或 2）来触发 stopHookIterationCount >= stopHookBlockingCap 分支。当前的两个 Stop hook 测试在 cap=8 下分别仅运行 1-2 次迭代，远未达到上限。

Session.ts 的 cap 逻辑独立处理 ACP 会话（VSCode companion）的 stop hook 循环。此路径的 bug 仅会在 ACP 会话中暴露，现有测试无法捕获。

Suggested change

if (stopHookIterationCount >= stopHookBlockingCap) {

// Add a test that sets up a blocking Stop hook and getStopHookBlockingCap.mockReturnValue(1),

// then asserts the prompt completes with stopReason: 'end_turn' on cap hit

// and that the warning message is emitted via messageEmitter.emitAgentMessage.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T17:35:59Z

+  // model or subagent would otherwise finish, so keep user-facing labels
+  // explicit instead of accepting arbitrary hook names.
+  const hookName = hookLabel === 'Stop' ? 'Stop hook' : 'SubagentStop hook';
+  return `${hookName} blocked continuation ${cap} consecutive times; overriding and ending the turn.`;


[Suggestion] formatStopHookBlockingCapWarning 在 cap=1 时输出语法错误："1 consecutive times" 应为 "1 consecutive time"。cap=1 是完全合法的配置值。

Suggested change

return `${hookName} blocked continuation ${cap} consecutive times; overriding and ending the turn.`;

const hookName = hookLabel === 'Stop' ? 'Stop hook' : 'SubagentStop hook';

const timesWord = cap === 1 ? 'time' : 'times';

return `${hookName} blocked continuation ${cap} consecutive ${timesWord}; overriding and ending the turn.`;

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-05-16T17:35:59Z


 const debugLogger = createDebugLogger('AGENT');

+function appendSubagentStopWarning(


[Suggestion] appendSubagentStopWarning 辅助函数（4 行）和 runSubagentStopHookLoop 方法（~50 行）在 agent.ts 和 background-agent-resume.ts 中完全重复。两处代码结构相同——相同的循环、相同的 cap 解析、相同的 formatStopHookBlockingCapWarning 调用。

任何 bug 修复都需要在两处同步修改，极易出现分歧。建议将共享逻辑提取到 packages/core/src/hooks/stopHookCap.ts 或新建共享模块，从两个调用点导入。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao

No new review findings beyond the existing open inline comments. All 332 focused tests pass. ⚠️ Downgraded from Approve to Comment: CI still running.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao

Incremental review: 3 prior issues addressed (grammar fix, deduplication of appendSubagentStopWarning → appendStopHookBlockingCapWarning, ACP cap tests). 8 existing inline comments remain open from previous review but are not reintroduced. No new issues found; all 318 tests pass, typecheck + lint clean.

LGTM for the incremental changes. ✅

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao

Verified locally on top of main.

Local test results

PR-listed npx vitest run src/hooks/stopHookCap.test.ts src/core/client.test.ts src/tools/agent/agent.test.ts (in packages/core): 214 passed / 3 files (PR body lists 205 — count grew with the later test commits)
Full packages/core suite: 8302 passed / 307 files (4 skipped)
Touched cli tests (Session.test.ts + goalCommand.test.ts + GoalStatusMessage.test.tsx): 85 passed / 3 files
Full packages/cli suite: 6079 passed; the 2 failing files (doctorChecks.test.ts for Node v22 check, server.test.ts for missing supertest) reproduce identically on main — pre-existing local env, not caused by this PR
npm run typecheck across all workspaces: clean
git diff --check: clean
Remote CI: Lint / CodeQL / Test mac · ubuntu · windows (Node 22.x) all SUCCESS

Design notes

stopHookCap.ts is a small, well-factored module: invalid/0/negative → default 8, fractional → floor, clamped to MAX = 100; env (QWEN_CODE_STOP_HOOK_BLOCK_CAP) takes precedence over config; empty env string falls through.
The cap is enforced identically at all 4 hook-loop sites (core/client.ts main Stop, tools/agent/agent.ts foreground + worktree SubagentStop, agents/background-agent-resume.ts background SubagentStop, cli/acp-integration/session/Session.ts ACP Stop), each going through config.getStopHookBlockingCap() and formatStopHookBlockingCapWarning(...).
abortGoalForStopHookCap correctly finalizes an active /goal as aborted when the main-loop / ACP cap fires, so cap-induced termination doesn't leak zombie goal state.
Settings schema is registered as Advanced / requiresRestart: true / showInDialog: false, with default sourced from DEFAULT_STOP_HOOK_BLOCK_CAP so the dialog and runtime stay in sync.

Two things worth flagging for the author

ACP Session cap default tightening (100 → 8). cli/acp-integration/session/Session.ts previously hard-coded MAX_STOP_HOOK_ITERATIONS = 100; with this PR ACP clients running long-blocking Stop hooks will hit the cap ~12.5× sooner. The escape hatch (stopHookBlockingCap / QWEN_CODE_STOP_HOOK_BLOCK_CAP) is in place and the PR body calls this out, but it's a meaningful default change worth a one-line note in release/migration text.
Bundled /goal UX + comment cleanup (commit a7ea51488 fix(cli): show goal judge details) is outside the stop-hook-cap scope: GoalStatusMessage now shows the goal condition and judge reason on every checking iteration (the previous code intentionally hid these to avoid per-turn chip clutter), and several files lose their provenance comments referencing Claude Code 2.1.140 internals. The technical "why" comments are preserved. Both changes look fine on their own; just noting they aren't implied by the PR title.

Minor: runSubagentStopHookLoop is duplicated between agent.ts and background-agent-resume.ts — this PR consistently updates both but doesn't dedupe. Pre-existing; potential follow-up cleanup.

LGTM.

wenshao · 2026-05-16T22:51:53Z

Supplement to my approval review — the actual tmux session transcript from local verification.

1. PR-listed vitest (3 focused files)

$ cd /tmp/pr4208/packages/core && npx vitest run \
    src/hooks/stopHookCap.test.ts \
    src/core/client.test.ts \
    src/tools/agent/agent.test.ts

 RUN  v3.2.4 /tmp/pr4208/packages/core
      Coverage enabled with v8

 ✓ src/hooks/stopHookCap.test.ts (7 tests) 3ms
 ✓ src/tools/agent/agent.test.ts (75 tests) 261ms
 ✓ src/core/client.test.ts (132 tests) 968ms

 Test Files  3 passed (3)
      Tests  214 passed (214)
   Duration  4.66s

(PR body lists 205; later commits a449a9ebb "cover stop hook cap integrations" and 3d8a74646 "strengthen stop hook cap coverage" raised the count to 214.)

2. typecheck — all workspaces

$ cd /tmp/pr4208 && npm run typecheck

> @qwen-code/qwen-code@0.15.11 typecheck
> npm run typecheck --workspaces --if-present

> @qwen-code/qwen-code@0.15.11 typecheck
> tsc --noEmit

> @qwen-code/qwen-code-core@0.15.11 typecheck
> tsc --noEmit

> @qwen-code/sdk@0.1.7 typecheck
> tsc --noEmit

> @qwen-code/webui@0.15.11 typecheck
> tsc --noEmit

(clean, exit 0)

3. git diff --check (whitespace)

$ cd /tmp/pr4208 && git diff --check $(git merge-base main HEAD)..HEAD
(clean, exit 0)

4. Full `packages/core` test suite

$ cd /tmp/pr4208/packages/core && npx vitest run --no-coverage
...
 Test Files  307 passed (307)
      Tests  8302 passed | 4 skipped (8306)
   Duration  58.99s

5. Targeted CLI tests (3 PR-touched files)

$ cd /tmp/pr4208/packages/cli && npx vitest run \
    src/acp-integration/session/Session.test.ts \
    src/ui/commands/goalCommand.test.ts \
    src/ui/components/messages/GoalStatusMessage.test.tsx

 ✓ src/ui/commands/goalCommand.test.ts (16 tests) 14ms
 ✓ src/ui/components/messages/GoalStatusMessage.test.tsx (1 test) 22ms
 ✓ src/acp-integration/session/Session.test.ts (68 tests) 320ms

 Test Files  3 passed (3)
      Tests  85 passed (85)
   Duration  6.33s

6. Full `packages/cli` test suite

$ npx vitest run --no-coverage
...
 Test Files  2 failed | 368 passed (370)
      Tests  1 failed | 6079 passed | 9 skipped (6089)
   Duration  194.21s

Both failures are pre-existing local environment issues, not caused by this PR — same outcome reproduces on main:

$ cd /root/git/qwen-code-x3/packages/cli && npx vitest run --no-coverage \
    src/serve/server.test.ts src/utils/doctorChecks.test.ts
...
 Test Files  2 failed (2)
      Tests  1 failed | 17 passed (18)

src/serve/server.test.ts — Failed to resolve import "supertest" (missing dev dep in local install)
src/utils/doctorChecks.test.ts > should pass Node.js version check for v22+ — local env is Node v20.19.2; the doctor check correctly reports a fail

7. `npm run build` (note — pre-existing env issue, not the PR)

The local build initially failed with Module '@qwen-code/channel-base' has no exported member 'resolvePath' because the symlinked channel-base/dist/index.d.ts in the parent install was stale (mtime May 11). The source on both main and the PR branch already exports resolvePath. A single npm run build --workspace=packages/cli in the parent repo regenerated the dist and the symptom is gone. Remote CI's mac / ubuntu / windows builds are all SUCCESS, so the build is fine end-to-end in a clean environment.

A subsequent retry surfaced a second symptom in the same family — generate-settings-schema.ts resolved @qwen-code/qwen-code-core via node_modules to the parent install's stale core/dist/index.js, which doesn't yet export the PR's new DEFAULT_STOP_HOOK_BLOCK_CAP. Same root cause (parent install's dist out of date relative to the PR branch's source), not a PR defect.

I switched to running typecheck (which uses tsconfig path-mappings to resolve directly to the worktree source) to validate the PR's TS correctness, and that came back clean across every workspace (section 2 above).

Environment

Linux x86_64, Node v20.19.2 (project engines requires >=22.0.0 — tests/typecheck still run; only the doctorChecks Node-version assertion correctly fails)
PR head 3d8a74646e0e4d5e2d5f355c53cfcaef7d3426d7 against merge-base b9590283c
Worktree at /tmp/pr4208 with node_modules symlinked from a parent install

…ask module Folds PR #4324's per-instance BackgroundTaskRegistry cap into the collapsed tasks/agent-task.ts module as kind-local module state. The new architecture's per-kind module is the natural home for an agent-only behavior — the generic TaskRegistry no longer needs to carry it. What moves into tasks/agent-task.ts: - MAX_CONCURRENT_BACKGROUND_AGENTS and the env-var resolver - agentAssertCanStartBackground(registry) free function - The race-guard inside agentRegister() (skips re-registers of already-running entries so resume can recover) - setAgentBackgroundCapForTest() for test override (mirrors the existing setAgentNotificationCallback pattern) What moves out of agent.ts: - The early preflight calls agentAssertCanStartBackground instead of registry.assertCanStartBackgroundAgent(). - The post-register error path is unchanged structurally — it still aborts the bgAbortController, fires SubagentStop, cleans up the worktree, and returns the cap message to the model. Resume path (background-agent-resume.ts) now wraps agentRegister in the same try/catch #4324 added, but routes to the new free function. Also addresses wenshao's review on #3982: 1. config.ts — moves the four registerTaskKind(...) calls below all imports, instead of dangling between two import groups. 2. dispatcher.ts — comment updated to describe actual init order (module-load side effect in config.ts), removes the fictional registerAllTaskKinds() / Config.initialize() references. 3. monitor.ts — fixes a stale comment that still referred to monitorRegister(registry, ) (dangling comma). 4. monitor-task.ts — monitorAbortAll now clears the module-level owner-routed callbacks so a daemon process recycling sessions doesn't leak handlers to dead owner agents. 5. useBackgroundTaskView.ts — moves the registry shape probe to getAll() BEFORE buildMerged() so activity bursts and monitor event bumps don't pay the listDreamTasks cost. Tests added: - tasks/agent-task.test.ts — covers env resolution, cap enforcement (running-only, foreground exclusion, paused exclusion, resume race exception), and module-level cap reset. Tests fixed (rebase fallout): - nonInteractiveCli.test.ts — six structured-output assertions migrated from mockBackgroundTaskRegistry.abortAll to the module-level mockAgentAbortAll mock. - shell.test.ts — 13 assertions migrated from registry.{cancel,complete,fail} to shellTaskModule.shell{Cancel,Complete,Fail} (called with (registry, ...)). - useBackgroundTaskView.test.ts — sort assertions updated to the new descending-startTime active-bucket order. - agent.test.ts — adds the getStopHookBlockingCap mock that was lost in the auto-merge with PR #4208. - clearCommand.test.ts — drops the SessionStart assertion that was lost in the auto-merge with PR #4115. - background-agent-resume.test.ts — monitorRegistry is now a set of vi.spyOn against the new module-level free functions. - InlineParallelAgentsDisplay.test.tsx — makeRegistryConfig exposes getTaskRegistry (matching the renamed Config method).

feat(core): add stop hook blocking cap

b469493

fix(core): tighten stop hook cap behavior

3aec631

qqqys marked this pull request as ready for review May 16, 2026 13:39

wenshao requested changes May 16, 2026

View reviewed changes

fix(cli): show goal judge details

a7ea514

wenshao reviewed May 16, 2026

View reviewed changes

fix(core): bound stop hook blocking cap

cc6fae9

wenshao reviewed May 16, 2026

View reviewed changes

fix(core): surface subagent stop cap warnings

05fa9fa

wenshao reviewed May 16, 2026

View reviewed changes

qqqys added 3 commits May 17, 2026 00:14

fix(core): clean up stop hook cap loop

e34364c

Merge upstream main into stop-hook-blocking-cap

5628b4e

test(core): cover stop hook cap integrations

a449a9e

wenshao requested changes May 16, 2026

View reviewed changes

test(core): strengthen stop hook cap coverage

3d8a746

wenshao reviewed May 16, 2026

View reviewed changes

wenshao approved these changes May 16, 2026

View reviewed changes

wenshao merged commit 07165a0 into QwenLM:main May 16, 2026
11 checks passed

This was referenced May 17, 2026

📊 AI CLI 工具社区动态日报 2026-05-17 zx0828/big_model_radar#60

Open

📊 AI CLI 工具社区动态日报 2026-05-17 gsscsd/big_model_radar#356

Open

wenshao mentioned this pull request May 17, 2026

feat(cli): add configurable plansDirectory for Plan Mode #4062

Merged

	return normalized >= 1 ? normalized : DEFAULT_STOP_HOOK_BLOCK_CAP;
	const MAX_STOP_HOOK_CAP = 100;
	return normalized >= 1 ? Math.min(normalized, MAX_STOP_HOOK_CAP) : DEFAULT_STOP_HOOK_BLOCK_CAP;

	if (envValue !== undefined) {
	if (envValue !== undefined && envValue !== '') {

		@@ -1014,7 +1015,10 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
		}

		debugLogger.warn(

		@@ -1014,7 +1026,10 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
		}

	}
	// (remove the post-loop debugLogger.warn block — it is unreachable)
	}

		@@ -1022,7 +1034,10 @@ export class BackgroundAgentResumeService {
		}

		@@ -1559,15 +1561,26 @@ export class GeminiClient {
		continueReason,

-        if (stopHookIterationCount >= stopHookBlockingCap) {
+// Add a test that sets up a blocking Stop hook and getStopHookBlockingCap.mockReturnValue(1),
+// then asserts the prompt completes with stopReason: 'end_turn' on cap hit
+// and that the warning message is emitted via messageEmitter.emitAgentMessage.


		const debugLogger = createDebugLogger('AGENT');

		function appendSubagentStopWarning(

Conversation

qqqys commented May 16, 2026

Summary

Validation

Scope / Risk

Testing Matrix

Linked Issues / Bugs

Uh oh!

github-actions Bot commented May 16, 2026

📋 Review Summary

🔍 General Feedback

🎯 Specific Feedback

🟡 High

🟢 Medium

🔵 Low

✅ Highlights

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao commented May 16, 2026

1. PR-listed vitest (3 focused files)

2. typecheck — all workspaces

3. git diff --check (whitespace)

4. Full packages/core test suite

5. Targeted CLI tests (3 PR-touched files)

6. Full packages/cli test suite

7. npm run build (note — pre-existing env issue, not the PR)

Environment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

4. Full `packages/core` test suite

6. Full `packages/cli` test suite

7. `npm run build` (note — pre-existing env issue, not the PR)