From 72bc670f3aa1ae4e5559a587df1cc8a9253cdc5b Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Mon, 3 Aug 2026 23:18:41 +0800 Subject: [PATCH 1/5] feat(acp): protect repeated tool execution failures Add a conservative prompt-local guard for repeated typed ACP tool execution failures, with shadow/warn/enforce rollout modes, privacy-safe telemetry, and coverage for the final execution outcome contract. Co-authored-by: Qwen-Coder --- .../acp-repeated-tool-call-protection.md | 526 ++++++++++++++++++ .../acp-integration/session/Session.test.ts | 287 +++++++++- .../src/acp-integration/session/Session.ts | 259 ++++++++- .../repeated-tool-failure-guard.test.ts | 338 +++++++++++ .../session/repeated-tool-failure-guard.ts | 299 ++++++++++ packages/cli/src/nonInteractiveCli.ts | 5 +- packages/core/src/index.ts | 2 + packages/core/src/telemetry/constants.ts | 2 + packages/core/src/telemetry/index.ts | 3 + packages/core/src/telemetry/loggers.test.ts | 122 ++++ packages/core/src/telemetry/loggers.ts | 58 +- packages/core/src/telemetry/metrics.test.ts | 47 ++ packages/core/src/telemetry/metrics.ts | 47 ++ packages/core/src/telemetry/types.ts | 93 ++++ 14 files changed, 2081 insertions(+), 7 deletions(-) create mode 100644 docs/design/acp-repeated-tool-call-protection.md create mode 100644 packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts create mode 100644 packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts diff --git a/docs/design/acp-repeated-tool-call-protection.md b/docs/design/acp-repeated-tool-call-protection.md new file mode 100644 index 00000000000..d97e609f640 --- /dev/null +++ b/docs/design/acp-repeated-tool-call-protection.md @@ -0,0 +1,526 @@ +# ACP Repeated Tool-Call Protection + +Date: 2026-07-31 +Status: Implemented, pending shadow rollout; revised for PR #8176 and PR #8180 +Area: ACP foreground prompt loop + +## Summary + +ACP should stop an automatic model loop when the same resolved tool repeatedly +reaches the same trusted execution failure. The protection is conservative: +it observes only finalized, fully settled tool batches; gives the model one +fixed corrective reminder; and stops only if the next batch repeats the same +failure. + +The first version is an in-memory, per-prompt semantic guard. It does not +replace the existing protections for duplicate provider call IDs, invalid +parameters, or the per-turn tool-call cap. It also does not attempt to provide +cross-restart exactly-once execution. + +Two telemetry changes are prerequisites: + +- [PR #8176](https://github.com/QwenLM/qwen-code/pull/8176) makes terminal + `status` authoritative and normalizes cancellation and error fields. +- [PR #8180](https://github.com/QwenLM/qwen-code/pull/8180) adds the independent + `executionStatus` axis and fixes ACP permission-cancellation classification. + +Enforcement must remain disabled when either contract is unavailable or +unknown. ACP's internal batch receipt must also preserve the structured +execution error type that PR #8180 already freezes while `invocation.execute()` +settles; the public response and JSON protocols do not need another field. + +## Problem + +The current ACP path has three useful but incomplete circuit breakers: + +- repeated provider call IDs are deduplicated; +- repeated invalid tool parameters stop after three failures; and +- every prompt has a total tool-call cap. + +They do not catch the common semantic loop in which the model keeps issuing +fresh call IDs for a tool that is actually entered and repeatedly fails for the +same structured reason. A high total-call cap stops the eventual runaway but +wastes model rounds, tool latency, and tokens before doing so. + +Terminal `error` alone is not a safe signal. It currently includes calls that +never executed, such as validation and permission failures, as well as failures +that happen after successful execution. Treating all of those as repeated tool +failures creates false positives and, in particular, turns user cancellation +into a product stability problem. The 468 recent permission cancellations that +were recorded as errors are a concrete example. + +## Goals + +- Detect repeated failures of the actual tool execution boundary. +- Never count user cancellation, permission denial, validation failure, + post-processing failure, historical unknowns, or duplicate provider events. +- Never skip or cancel the tool calls in the currently admitted model batch. +- Give the model one bounded chance to change approach before stopping. +- Preserve complete tool results and a clear stop reason in ACP history. +- Make shadow and enforcement decisions explainable without logging arguments, + results, paths, or raw error messages. +- Reuse existing ACP loop protection and telemetry conventions with a small, + testable state machine. + +## Non-goals + +- Exactly-once execution across process crashes, provider reconnects, or + failover. +- Replacing provider call-ID deduplication. +- Replacing the invalid-parameter fast path. +- Inferring retryability from free-form error text. +- Persisting the guard across a daemon restart, rewind, branch, or fork. +- Applying the guard to TUI, Stop-hook/Todo automatic continuations, cron, + notifications, subagent-internal loops, or third-party producers in the + first release. +- Automatically retrying a tool or suppressing an admitted call. + +Those are separate problems with different trust and durability boundaries. +Combining them with semantic loop detection would make the first release harder +to validate without improving its decision signal. + +## Prerequisite outcome contract + +The reducer consumes the integrated result of PR #8176 and PR #8180. It must +not reconstruct execution state from legacy `success`, error strings, UI +frames, or spans. + +| Terminal `status` | `executionStatus` | Meaning for this guard | +| ----------------- | -------------------- | ---------------------------------------------------------------------- | +| `success` | `success` | Reset | +| `success` | `not_started` | Reset; protocol-level synthetic result | +| `error` | `not_started` | Reset; validation, permission rejection, hook block, or lookup failure | +| `error` | `error` | Eligible only with a trusted frozen `executionErrorType` | +| `error` | `success` | Reset; execution succeeded and later processing failed | +| `cancelled` | any | Reset | +| any | `cancelled` | Reset | +| any | missing or `unknown` | Reset and downgrade the prompt to at most `warn` | + +The two invalid combinations defined by PR #8180, +`success/error` and `success/cancelled`, are treated as contract violations: +reset the guard, emit a diagnostic, and do not enforce. + +`status` wins cancellation arbitration. If an execution error races with a +user or parent cancellation and the terminal status is `cancelled`, the call +does not count. + +Terminal `errorType` is not the repeated-failure key. A post-execution hook, +image bridge, or other finalization step may replace it even when +`executionStatus` remains `error`. The internal receipt therefore copies the +`executionErrorType` captured at execution settle. Missing or +`ToolErrorType.UNKNOWN` execution classifications are ineligible. + +## Identity and eligibility + +An eligible failure has: + +- terminal `status = error`; +- `executionStatus = error`; +- a non-empty structured `executionErrorType` other than + `ToolErrorType.UNKNOWN`; +- a resolved built-in or MCP tool identity from the tool registry; and +- a final result produced by a fully settled ACP foreground batch. + +The failure key is: + +```text +(policyToolName, executionErrorType) +``` + +`policyToolName` is the existing resolved `tool.name` value that ACP uses for +permission checks, not a model-provided display name. MCP registered names +already include their server-qualified identity. `executionErrorType` is frozen +at execution settle, independently from the terminal call error. + +Arguments are deliberately excluded. This catches parameter thrashing against +the same execution boundary, while the threshold and two-batch requirement +provide false-positive headroom. Raw arguments, output, paths, and error text +must not be stored in the guard or emitted to central telemetry. + +Unknown tools, blank identities, unclassified errors, and third-party events +with incomplete outcome fields are not eligible. + +## State machine + +The Session owns one guard per foreground ACP prompt: + +```ts +type RepeatedToolFailureState = + | { phase: 'idle' } + | { + phase: 'tracking'; + key: FailureKey; + failureCount: number; + batchCount: number; + } + | { + phase: 'warned'; + key: FailureKey; + failureCount: number; + batchCount: number; + } + | { + phase: 'latched'; + key: FailureKey; + failureCount: number; + batchCount: number; + }; +``` + +The threshold is fixed at eight eligible failures across at least two complete +model batches. It is a code constant in the first release, not a user setting. + +After every `runToolCalls` batch has fully settled: + +1. Ignore duplicate provider events already handled by the call-ID deduper. + They neither advance nor reset semantic state. +2. Drain accepted mid-turn input through the existing Session boundary. If new + external input or a queued full prompt is observed, reset to `idle` and + leave the existing input and FIFO behavior authoritative. If the drain is + unreliable, reset and disable enforcement for the rest of the prompt. +3. Reset to `idle` if the batch is incomplete, violates the outcome contract, + or contains any reset-class outcome. +4. Collect eligible failure keys from the batch. If there is not exactly one + unique key, reset to `idle`. +5. If the key differs from the tracked key, begin a new streak from this batch. + Otherwise add the batch's eligible failure count and increment the batch + count. +6. When `failureCount >= 8` and `batchCount >= 2`, transition to `warned` and + request the mode-specific reminder action shown below. +7. If the immediately following complete batch contains the same eligible key + and no reset condition, execute and record the whole batch, then transition + to `latched`. The configured mode determines whether another model request + is sent. + +The state transition and control action are separate: + +| Mode | Threshold reached | Next matching batch | +| --------- | ----------------------------------- | ----------------------------------------------------- | +| `shadow` | Record `would_warn`; inject nothing | Record `would_stop`, latch, and continue | +| `warn` | Inject once and record `warned` | Record `would_stop`, latch, and continue | +| `enforce` | Inject once and record `warned` | Record `stopped`, latch, and stop before another send | + +Once latched, the guard emits no further decisions for that prompt. This avoids +repeated reminders and telemetry amplification in shadow and warn modes. + +The reminder is: + +> System: the same tool execution has failed repeatedly for the same classified +> reason. Do not repeat the same approach. Inspect the returned result, change +> the approach or required preconditions, or explain the blocker. + +The stopped message is: + +> System: Automatic continuation stopped because the same tool execution +> failure continued after a corrective reminder. New user input is required to +> continue. + +The messages contain neither raw arguments nor raw error text. They are fixed +system context, not fabricated user input. + +## Batch and concurrency rules + +The unit of reduction is a completed model tool batch, not an individual +streaming event. This is required because Agent calls may execute concurrently +and terminal frames may arrive in a different order from the model's function +calls. + +`runToolCalls` returns a narrow batch receipt only after all admitted calls have +settled. Each receipt entry contains `callId`, `policyToolName`, terminal +`status`, frozen `executionStatus`, frozen `executionErrorType`, and whether it +was a provider duplicate; it contains no arguments, result, or raw error. +`#buildNextMessageAfterToolRun` drains mid-turn input first, then passes that +receipt and the drain's `parts`, `hasQueuedPrompt`, and `reliable` state to the +reducer before constructing the next model message. This preserves the existing +external-input priority. Outcomes are kept in original model call order, +although the reduction is order-independent. + +The guard never: + +- stops halfway through a batch; +- cancels siblings after one call reaches the threshold; +- turns a skipped sibling into an execution failure; or +- treats a late duplicate terminal frame as a new observation. + +If the Session cannot prove that the batch is complete, it resets and does not +enforce. PR #8180's frozen execution status is necessary but does not by itself +prove batch completion; the reducer is called only from the settled +`runToolCalls` boundary. + +## Interaction with existing protection + +The checks remain ordered from most specific to broadest: + +1. Provider call-ID deduplication handles transport/provider replay. +2. The existing invalid-parameter guard handles repeated pre-execution schema + failures (`executionStatus = not_started`). +3. This guard handles repeated, typed execution failures. +4. The total per-turn tool-call cap remains the absolute backstop. + +Only the first guard that stops the turn records the terminal loop reason. +This design adds a distinct `LoopType.REPEATED_TOOL_EXECUTION_FAILURE` so +operators can distinguish it from invalid parameters, duplicate IDs, and the +total cap. + +When this guard stops, ACP must: + +- preserve all settled function responses in chat history; +- add the fixed stop context to history and emit it once through the existing + replayable ACP agent-message update path; +- suspend Todo Stop Guard and other automatic continuations for the prompt; +- leave queued external input intact; and +- finish the active ACP request without opening another model stream. + +The guard stays latched until the active prompt finishes. A later top-level +prompt, including an explicit retry or continue request, creates a fresh guard +under the existing Session lifecycle. + +## Scope and enforcement modes + +The first release applies only to the selected live Session owner processing a +foreground ACP prompt. It is not process-global and must not fall back to a +legacy or primary runtime when workspace ownership is unknown. + +Modes: + +- `off`: no reducer or telemetry. +- `shadow`: compute decisions but do not inject or stop. +- `warn`: inject the reminder but never stop. +- `enforce`: inject and stop according to the state machine. + +Default is `shadow`. Unknown ownership, an untrusted producer, or mixed +deployment versions force at most `warn`. A missing `executionStatus` or an +unsupported outcome combination resets the streak and downgrades the rest of +that prompt to at most `warn`. Cron, notification, background, and custom +routes remain `off` in the first release. + +The mode is an operator-controlled deployment policy, not a user-facing +setting. `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` selects `off`, `shadow`, +`warn`, or `enforce` when the Session starts; missing or invalid values resolve +to `shadow`. The deployment control plane must set it only on the assigned +version-pinned cohort. This feature does not introduce a second rollout or +owner-assignment service. + +## Telemetry and privacy + +Emit low-cardinality counters plus one privacy-restricted structured diagnostic +log per reducer transition: + +- deployment environment and service version from the existing OpenTelemetry + resource rather than new guard labels; +- route: ACP foreground or other; +- mode; +- phase before and after; +- decision: reset, tracked, would_warn, warned, would_stop, or stopped; +- candidate terminal status, execution status, frozen execution error type, + and tool type when the batch has one eligible key; +- otherwise only a low-cardinality reset reason such as `success`, + `cancelled`, `not_started`, `unknown`, `mixed`, `incomplete`, + `external_input`, or `contract_violation`; +- failure count bucket: `0`, `1-2`, `3-4`, `5-7`, or `8+`; +- batch count bucket: `0`, `1`, `2`, or `3+`; +- the existing prompt ID in the diagnostic log only, for checking transition + order; and +- a prompt-local candidate ordinal in the diagnostic log only. The reducer + reuses the ordinal while the same private key is active and allocates a new + one when the key changes. + +Prompt ID and candidate ordinal are never metric labels. The ordinal cannot +correlate a tool across prompts and does not reveal its identity. An +`idle`-to-`idle` observation emits nothing. + +The terminal `repeated_tool_execution_failure` loop event uses the same +privacy-restricted OpenTelemetry path and bypasses session-scoped RUM. Other +loop types keep their existing telemetry behavior. + +Do not emit tool arguments, results, raw error messages, stack traces, paths, +MCP server names, user IDs, session IDs, or the unhashed failure key. + +Cancellation is excluded from every failure-rate numerator. The primary +execution SLI uses PR #8180's contract: + +```text +execution_status = error +──────────────────────────────────────── +execution_status in {success, error} +``` + +The legacy `success` field and pre-PR deployment data must not be used to +validate this guard. + +## Rollout + +### Phase 0: integrate outcome contracts + +1. Merge PR #8176. +2. Rebase PR #8180 on top and preserve both the terminal `status` dimension + from #8176 and the independent execution counter from #8180. +3. Preserve #8180's internal `executionErrorType` in the ACP batch receipt; + do not infer it later from terminal `errorType`. +4. Prefer splitting #8180 into reviewable changes for the execution contract, + ACP producer fixes, telemetry/spans, and MCP cancellation/timeout + arbitration. +5. Deploy the integrated contract before collecting a new baseline. + +The 468 historical permission-cancellation records remain evidence of the old +producer bug, not eligible guard failures. Recalculate the seven-day baseline +only for deployment versions containing the integrated contract, separately +for internal and public cloud. Do not mix old and new versions. + +### Phase 1: shadow + +Implement the pure reducer and wire it to the settled ACP batch boundary in +`shadow` mode. Run for at least seven complete days in both environments. +Shadow mode advances a virtual warned state without injecting the reminder, so +`would_warn` and `would_stop` estimate volume only. They cannot establish how a +model behaves after seeing the reminder. + +Required invariants: + +- zero cancelled calls counted as eligible failures; +- zero `not_started`, unknown execution classifications, or post-execution + failures counted; +- zero decisions based on incomplete batches; +- zero enforcement after an unreliable input drain; +- zero raw argument, result, path, or error-text fields in telemetry; +- every `would_stop` is preceded by `would_warn` for the same candidate ordinal + and prompt; and +- the existing total tool-call and duplicate-ID protections remain unchanged. + +Manually review a privacy-safe sample of would-stop sessions using authorized +local trace access. Classify whether the unmodified continuation made useful +progress; use this to reject a clearly unsafe threshold, not to approve +enforcement. + +### Phase 2: warn + +Enable `warn` for internal ACP foreground prompts. Hold for seven days and +confirm that reminder injection does not increase cancellation, reconnect, +latency, token, or round-count regressions. Public cloud remains in shadow. +Only warn-mode prompts show whether the model repeats the failure after the +actual corrective reminder; that cohort supplies the semantic evidence for +enforcement. + +### Phase 3: limited enforcement + +Enable enforcement for at most 5% of stable, version-pinned internal ACP +foreground owners. Assignment is deterministic by owner so one prompt cannot +switch treatment mid-run. The remaining 95% stays in `warn`, so both treatment +and control receive the same corrective reminder and differ only in whether the +post-reminder matching batch stops. Hold each wave for seven days. + +Promote only if: + +- all correctness invariants remain at zero violations; +- warn-mode review finds no clear useful progress after a matching + post-reminder failure; +- enforced stops have the expected key, reminder, complete batch receipt, and + preserved history; +- completion rate is not worse by more than one percentage point; +- disconnect-or-retry-within-ten-minutes is not worse by more than 0.5 + percentage points; +- p95 latency, mean tokens, and mean rounds are each no worse than 1.10 times + control; and +- the stopped-loop rate and saved-call estimate match warn-mode observations. + +Use owner-level blocked analysis and confidence intervals; calls from one owner +are not independent samples. Any contract violation or cancellation +misclassification immediately returns the environment to `shadow`. + +Do not ramp beyond 5% until treatment saturation has been checked at higher +assignment levels through capacity testing at projected full traffic. If +interference cannot be ruled out, keep a permanent control and cap enforcement +at 5%. + +Public cloud repeats shadow, warn, and limited enforcement independently after +the internal gate passes. It never inherits an internal pass. + +## Implementation shape + +Keep the change small: + +- add a pure `repeated-tool-failure-guard.ts` reducer beside ACP Session code; +- have Session translate finalized batch records into the reducer's narrow + receipt, drain external input, and apply the returned action; +- add the new loop type and telemetry event fields through the existing + low-cardinality logging path; and +- avoid changing tool implementations or adding another execution scheduler. + +Because the change touches Core telemetry types and ACP Session orchestration, +it requires maintainer ownership under the repository's core-infrastructure +gate. + +Suggested delivery sequence: + +1. Outcome-contract integration and the corrected seven-day baseline. +2. Reducer, unit tests, and shadow telemetry. +3. Reminder injection and warn-mode E2E coverage. +4. Stop wiring, lifecycle resets, and dormant enforce mode. +5. Controlled rollout; no code change is required to advance modes. + +## Verification + +Unit tests for the pure reducer cover: + +- the full terminal/execution decision table; +- terminal `errorType` cannot overwrite the frozen execution failure key; +- eight failures in one batch do not warn; +- eight failures across two batches warn; +- the next matching batch stops only after it settles; +- success, cancellation, `not_started`, `unknown`, post-processing failure, + mixed keys, incomplete batch, unreliable drain, queued prompt, and new input + reset; +- duplicate provider events are ignored rather than counted or reset; +- a new key starts a new streak; +- warning and stop text are fixed and contain no tool data; and +- unsupported outcome combinations never enforce. + +ACP Session tests cover: + +- sequential and concurrent batches; +- preservation of all current-batch results before stop; +- no extra model stream after stop; +- Todo Stop Guard suspension; +- mid-turn and queued user-input precedence; +- duplicate call-ID, invalid-parameter, repeated-execution, and total-cap + ordering; +- Session reset, retry, cancellation, disconnect, history replay, and model + switch behavior; and +- shadow, warn, enforce, and downgrade-to-warn routes. + +Telemetry tests cover: + +- normalization inherited from PR #8176 and PR #8180; +- cancellation exclusion; +- version-scoped baseline queries; +- low-cardinality attributes; and +- redaction of arguments, results, paths, raw messages, and stable identity. + +The behavioral change also needs an E2E plan under `.qwen/e2e-tests/` covering +one typed failing tool, permission cancellation, a successful recovery after +the reminder, a repeated failure that stops, concurrent siblings, reconnect, +and both internal and public-cloud policy modes. + +Before delivery, run targeted Core and CLI Vitest files from their package +directories, then `npm run build`, `npm run typecheck`, and `npm run lint`. + +## Failure handling + +- Telemetry emission failure never changes the tool or model control flow. +- Missing or malformed outcome data resets and downgrades enforcement. +- Reminder injection failure resets the guard; it must not stop without having + delivered the reminder. +- Stop-history persistence failure returns the existing ACP internal error and + does not pretend the stop was durably recorded. +- A process restart loses the semantic streak by design. Existing history-based + provider call-ID deduplication still prevents replay of an already answered + provider call. The total tool-call cap bounds a newly started prompt. + +## Final decision + +Adopt the two-axis outcome contract and implement the conservative per-prompt +state machine after PR #8176 and PR #8180 are integrated. Do not count any +legacy, cancelled, pre-execution, unknown, or post-execution outcome. Keep +exactly-once transport, durable cross-restart state, and global rollout +orchestration outside the first implementation unless production evidence +shows that the bounded in-memory guard is insufficient. diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index b7e9498ff8e..c3e615e3ee5 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -362,6 +362,7 @@ describe('Session', () => { let session: Session; let currentModel: string; let currentAuthType: AuthType; + let originalProcessGuardMode: string | undefined; let switchModelSpy: ReturnType; let getAvailableCommandsSpy: ReturnType; let mockChatRecordingService: { @@ -496,6 +497,9 @@ describe('Session', () => { } beforeEach(() => { + originalProcessGuardMode = + process.env['QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD']; + process.env['QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'] = 'shadow'; startToolSpanSpy.mockClear(); addToolArgumentsAttributesSpy.mockClear(); addToolCallResultAttributesSpy.mockClear(); @@ -704,6 +708,12 @@ describe('Session', () => { }); afterEach(() => { + if (originalProcessGuardMode === undefined) { + delete process.env['QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD']; + } else { + process.env['QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'] = + originalProcessGuardMode; + } // Reset global runtime base dir state to prevent state leakage between tests core.Storage.setRuntimeBaseDir(null); // Clear session reference to allow garbage collection @@ -6183,6 +6193,221 @@ describe('Session', () => { }); }); + describe('repeated tool execution failure guard', () => { + const guardModeEnv = 'QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'; + let originalGuardMode: string | undefined; + + const recreateSessionWithGuardMode = (mode: 'warn' | 'enforce') => { + originalGuardMode = process.env[guardModeEnv]; + process.env[guardModeEnv] = mode; + session = new Session( + 'test-session-id', + mockConfig, + mockClient, + mockSettings, + ); + }; + + const restoreGuardMode = () => { + if (originalGuardMode === undefined) { + delete process.env[guardModeEnv]; + } else { + process.env[guardModeEnv] = originalGuardMode; + } + }; + + const failureBatch = (batch: number, count: number): FunctionCall[] => + Array.from({ length: count }, (_, index) => ({ + id: `failure_${batch}_${index}`, + name: 'failing_tool', + args: { attempt: `${batch}_${index}` }, + })); + + const streamForBatch = (batch: number, count: number) => + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { functionCalls: failureBatch(batch, count) }, + }, + ]); + + const installFailingTool = () => { + const execute = vi.fn().mockResolvedValue({ + llmContent: 'failed', + returnDisplay: 'failed', + error: { + message: 'execution failed', + type: core.ToolErrorType.EXECUTION_FAILED, + }, + }); + mockToolRegistry.getTool.mockReturnValue({ + name: 'failing_tool', + kind: core.Kind.Execute, + displayName: 'Failing Tool', + description: 'Fails during execution', + build: vi.fn().mockReturnValue({ + params: {}, + execute, + getDefaultPermission: vi.fn().mockResolvedValue('allow'), + getDescription: vi.fn().mockReturnValue('Failing Tool'), + toolLocations: vi.fn().mockReturnValue([]), + }), + canUpdateOutput: false, + isOutputMarkdown: true, + }); + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(true); + return execute; + }; + + it('injects one corrective reminder in warn mode and keeps running', async () => { + recreateSessionWithGuardMode('warn'); + try { + const execute = installFailingTool(); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(streamForBatch(1, 4)) + .mockResolvedValueOnce(streamForBatch(2, 4)) + .mockResolvedValueOnce(streamForBatch(3, 1)) + .mockResolvedValueOnce(createEmptyStream()); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'end_turn' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); + const correctiveSend = vi.mocked(mockChat.sendMessageStream).mock + .calls[2][1] as { message: Part[] }; + expect(correctiveSend.message).toContainEqual( + expect.objectContaining({ + text: expect.stringContaining('Do not repeat the same approach'), + }), + ); + const allSentText = vi + .mocked(mockChat.sendMessageStream) + .mock.calls.flatMap(([, request]) => + (request as { message: Part[] }).message + .map((part) => part.text) + .filter((text): text is string => text !== undefined), + ); + expect( + allSentText.filter((text) => + text.includes('Do not repeat the same approach'), + ), + ).toHaveLength(1); + expect(logLoopDetectedSpy).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + }), + ); + } finally { + restoreGuardMode(); + } + }); + + it('stops after the post-reminder matching batch in enforce mode', async () => { + recreateSessionWithGuardMode('enforce'); + try { + const execute = installFailingTool(); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(streamForBatch(1, 4)) + .mockResolvedValueOnce(streamForBatch(2, 4)) + .mockResolvedValueOnce(streamForBatch(3, 1)) + .mockResolvedValueOnce(createEmptyStream()); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'end_turn' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(3); + expect(logLoopDetectedSpy).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + }), + ); + expect(mockChat.addHistory).toHaveBeenCalledWith({ + role: 'user', + parts: expect.arrayContaining([ + expect.objectContaining({ + functionResponse: expect.objectContaining({ + id: 'failure_3_0', + }), + }), + expect.objectContaining({ + text: expect.stringContaining('Automatic continuation stopped'), + }), + ]), + }); + expect(mockClient.sessionUpdate).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: 'test-session-id', + update: expect.objectContaining({ + sessionUpdate: 'agent_message_chunk', + content: expect.objectContaining({ + type: 'text', + text: expect.stringContaining( + 'Automatic continuation stopped', + ), + }), + }), + }), + ); + } finally { + restoreGuardMode(); + } + }); + + it('lets a user cancellation during the final drain win over enforcement', async () => { + recreateSessionWithGuardMode('enforce'); + try { + const execute = installFailingTool(); + let drainCount = 0; + mockClient.extMethod = vi.fn().mockImplementation(async () => { + drainCount++; + if (drainCount === 3) { + void session.cancelPendingPrompt(); + } + return { messages: [], hasQueuedPrompt: false }; + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(streamForBatch(1, 4)) + .mockResolvedValueOnce(streamForBatch(2, 4)) + .mockResolvedValueOnce(streamForBatch(3, 1)) + .mockResolvedValueOnce(createEmptyStream()); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'cancelled' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(3); + expect(logLoopDetectedSpy).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + }), + ); + } finally { + restoreGuardMode(); + } + }); + }); + describe('shell heartbeat forwarding', () => { const runShellToolCall = async ( execute: ReturnType, @@ -7117,7 +7342,10 @@ describe('Session', () => { expect(mockClient.extMethod).toHaveBeenCalledWith( 'craft/drainMidTurnQueue', - { sessionId: 'test-session-id' }, + { + sessionId: 'test-session-id', + todoStopGuardWatchQueuedPrompt: true, + }, ); const secondCall = vi.mocked(mockChat.sendMessageStream).mock.calls[1]; const midTurnPart = { @@ -16446,6 +16674,17 @@ describe('Session', () => { stopAfterPermissionCancel: boolean; loopDetected?: boolean; repeatedDuplicateProviderToolCall?: boolean; + repeatedToolFailureBatch?: { + complete: boolean; + observations: Array<{ + callId: string; + policyToolName?: string; + terminalStatus: 'success' | 'error' | 'cancelled'; + executionStatus?: core.ToolExecutionStatus | 'unknown'; + executionErrorType?: core.ToolErrorType; + providerDuplicate?: boolean; + }>; + }; }>; }; @@ -17115,6 +17354,18 @@ describe('Session', () => { errorType: core.ToolErrorType.EXECUTION_TIMEOUT, }), ); + expect(result.repeatedToolFailureBatch).toEqual({ + complete: true, + observations: [ + expect.objectContaining({ + callId: 'structured_timeout_call', + policyToolName: 'timeout_tool', + terminalStatus: 'error', + executionStatus: 'error', + executionErrorType: core.ToolErrorType.EXECUTION_TIMEOUT, + }), + ], + }); expect(messageBus.request).toHaveBeenCalledWith( expect.objectContaining({ eventName: 'PostToolUseFailure', @@ -17231,6 +17482,18 @@ describe('Session', () => { errorType: core.ToolErrorType.UNHANDLED_EXCEPTION, }), ); + expect(result.repeatedToolFailureBatch).toEqual({ + complete: true, + observations: [ + expect.objectContaining({ + callId: 'postprocess_success_call', + policyToolName: 'postprocess_tool', + terminalStatus: 'error', + executionStatus: 'success', + executionErrorType: undefined, + }), + ], + }); }); it('preserves a structured postprocessing error type after successful execution', async () => { @@ -17457,6 +17720,18 @@ describe('Session', () => { errorType: core.ToolErrorType.UNHANDLED_EXCEPTION, }), ); + expect(result.repeatedToolFailureBatch).toEqual({ + complete: true, + observations: [ + expect.objectContaining({ + callId: 'postprocess_error_call', + policyToolName: 'timeout_tool', + terminalStatus: 'error', + executionStatus: 'error', + executionErrorType: core.ToolErrorType.EXECUTION_TIMEOUT, + }), + ], + }); }); it.each([ @@ -20220,6 +20495,16 @@ describe('Session', () => { const { parts } = result; expect(parts).toHaveLength(1); expect(result.stopAfterPermissionCancel).toBe(false); + expect(result.repeatedToolFailureBatch).toEqual({ + complete: true, + observations: [ + expect.objectContaining({ + callId: 'shell_1__qwen_dup_2', + providerDuplicate: true, + executionStatus: 'not_started', + }), + ], + }); expect(parts[0].functionResponse?.id).toBe('shell_1__qwen_dup_2'); expect(parts[0].functionResponse?.response).toEqual({ error: expect.stringContaining( diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 39a91ec35cc..f95b8425708 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -140,8 +140,10 @@ import { logConversationFinishedEvent, ConversationFinishedEvent, logLoopDetected, + logRepeatedToolFailureGuard, LoopDetectedEvent, LoopType, + RepeatedToolFailureGuardEvent, acquireSleepInhibitor, refreshMemoryAfterManagedWrite, clearGoalTerminalObserver, @@ -281,6 +283,17 @@ import { DaemonTodoStopGuard, type TodoStopGuardContinuation, } from './daemon-todo-stop-guard.js'; +import { + createRepeatedToolFailureGuardState, + reduceRepeatedToolFailureGuard, + REPEATED_TOOL_FAILURE_REMINDER, + REPEATED_TOOL_FAILURE_STOP_MESSAGE, + resolveRepeatedToolFailureGuardMode, + type RepeatedToolFailureBatch, + type RepeatedToolFailureGuardMode, + type RepeatedToolFailureGuardDecision, + type RepeatedToolFailureGuardState, +} from './repeated-tool-failure-guard.js'; const debugLogger = createDebugLogger('SESSION'); const permissionRequestTails = new WeakMap< @@ -291,6 +304,8 @@ const USER_CANCEL_ABORT_REASON = 'qwen:user-cancel'; const SESSION_DISPOSE_ABORT_REASON = 'qwen:session-dispose'; const DAEMON_RETRY_META_KEY = 'qwen.daemon.retry'; const DAEMON_CONTINUE_META_KEY = 'qwen.daemon.continueLastTurn'; +const REPEATED_TOOL_FAILURE_GUARD_MODE_ENV = + 'QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'; const TODO_STOP_GUARD_PROMPT_PREFIX = '[Todo Stop Guard] '; const TODO_STOP_GUARD_PROMPT_BODY_SUFFIX = ' todo item(s) are still pending or in progress. Continue executing the current task now. Do not ask the user whether to continue. If progress requires user input, use the structured question or permission flow. If progress depends on external state, report the blocker explicitly.'; @@ -371,6 +386,7 @@ type RunToolResult = { stopAfterPermissionCancel: boolean; repeatedDuplicateProviderToolCall?: boolean; loopDetected?: boolean; + repeatedToolFailureBatch?: RepeatedToolFailureBatch; memoryWriteCandidates?: MemoryWriteCandidate[]; }; @@ -385,6 +401,7 @@ type TodoStopGuardClaimResult = 'claimed' | 'queued' | 'unavailable'; type NextMessageAfterToolRun = { message: Content | null; hadMidTurnUserInput: boolean; + stoppedByRepeatedToolFailure?: boolean; }; type TodoStopGuardBackgroundBaseline = { @@ -422,6 +439,10 @@ type PendingToolResultRecord = { toolName: string; responseParts: Part[]; persistedOutputFiles?: string[]; + policyToolName?: string; + toolType?: 'native' | 'mcp'; + executionErrorType?: ToolErrorType; + providerDuplicate?: boolean; metadata: Omit, 'executionStatus'> & { status: 'success' | 'error' | 'cancelled'; executionStatus: ToolExecutionStatus; @@ -437,6 +458,8 @@ type DaemonToolLoopState = { totalToolCalls: number; invalidToolParamErrors: Map; loopDetected: boolean; + repeatedToolFailureMode: RepeatedToolFailureGuardMode; + repeatedToolFailureState: RepeatedToolFailureGuardState; }; const DAEMON_INVALID_TOOL_PARAMS_THRESHOLD = 3; @@ -451,14 +474,104 @@ const TOOL_EXECUTION_CANCELLED_MESSAGE = 'Tool execution was cancelled.'; const TOOL_POST_EXECUTION_CANCELLED_MESSAGE = 'The tool had already completed; its output was discarded.'; -function createDaemonToolLoopState(): DaemonToolLoopState { +function createDaemonToolLoopState( + repeatedToolFailureMode: RepeatedToolFailureGuardMode = 'off', +): DaemonToolLoopState { return { totalToolCalls: 0, invalidToolParamErrors: new Map(), loopDetected: false, + repeatedToolFailureMode, + repeatedToolFailureState: createRepeatedToolFailureGuardState(), }; } +function repeatedToolFailureCountBucket( + count: number, +): '0' | '1-2' | '3-4' | '5-7' | '8+' { + if (count === 0) return '0'; + if (count <= 2) return '1-2'; + if (count <= 4) return '3-4'; + if (count <= 7) return '5-7'; + return '8+'; +} + +function repeatedToolFailureBatchBucket(count: number): '0' | '1' | '2' | '3+' { + if (count === 0) return '0'; + if (count === 1) return '1'; + if (count === 2) return '2'; + return '3+'; +} + +function recordRepeatedToolFailureDecision( + config: Config, + promptId: string, + mode: RepeatedToolFailureGuardMode, + previousState: RepeatedToolFailureGuardState, + decision: RepeatedToolFailureGuardDecision, + batch: RepeatedToolFailureBatch, +): void { + if (mode === 'off' || decision.kind === 'none') return; + + const countState = decision.kind === 'reset' ? previousState : decision.state; + const telemetryDecision = + decision.kind === 'warn' + ? 'warned' + : decision.kind === 'stop' + ? 'stopped' + : decision.kind; + const key = decision.kind === 'reset' ? undefined : decision.state.key; + const matchingToolTypes = new Set( + key + ? batch.observations + .filter( + (observation) => + !observation.providerDuplicate && + observation.policyToolName === key.policyToolName && + observation.executionErrorType === key.executionErrorType, + ) + .map((observation) => observation.toolType) + .filter((toolType) => toolType !== undefined) + : [], + ); + const toolType = + matchingToolTypes.size === 1 ? [...matchingToolTypes][0] : undefined; + + try { + logRepeatedToolFailureGuard( + config, + new RepeatedToolFailureGuardEvent({ + prompt_id: promptId, + route: 'acp_foreground', + mode, + phase_before: previousState.phase, + phase_after: decision.state.phase, + decision: telemetryDecision, + failure_count_bucket: repeatedToolFailureCountBucket( + countState.failureCount, + ), + batch_count_bucket: repeatedToolFailureBatchBucket( + countState.batchCount, + ), + candidate_ordinal: countState.candidateOrdinal, + ...(decision.kind === 'reset' + ? { reset_reason: decision.reason } + : { + terminal_status: 'error', + execution_status: 'error', + execution_error_type: key?.executionErrorType, + tool_type: toolType, + }), + }), + ); + } catch (error) { + debugLogger.debug( + '[repeated-tool-failure-guard] Failed to record telemetry', + error, + ); + } +} + function recordDaemonLoopDetected( config: Config, promptId: string, @@ -1305,6 +1418,7 @@ export class Session implements SessionContext { // `#drainMidTurnUserMessages`. private midTurnRecoveredMessages: DrainedMidTurnMessage[] = []; private readonly todoStopGuard: DaemonTodoStopGuard; + private readonly repeatedToolFailureGuardMode: RepeatedToolFailureGuardMode; private todoStopGuardBackgroundBaseline: TodoStopGuardBackgroundBaseline; private readonly relatedAgentIds = new Set(); private readonly provisionalRelatedAgentCounts = new Map(); @@ -1376,6 +1490,9 @@ export class Session implements SessionContext { !this.config.getBareMode() && !this.config.isSafeMode(); this.todoStopGuard = new DaemonTodoStopGuard(todoStopGuardEnabled); + this.repeatedToolFailureGuardMode = resolveRepeatedToolFailureGuardMode( + process.env[REPEATED_TOOL_FAILURE_GUARD_MODE_ENV], + ); this.todoStopGuardBackgroundBaseline = this.#captureTodoStopGuardBackgroundBaseline(); @@ -3122,7 +3239,9 @@ export class Session implements SessionContext { let nextMessage: Content | null = { role: 'user', parts }; let turnCount = 0; - const toolLoopState = createDaemonToolLoopState(); + const toolLoopState = createDaemonToolLoopState( + this.repeatedToolFailureGuardMode, + ); // conversation_finished must fire on every terminal path of the // turn — the loop below has cancel/abort/no-stream early-returns @@ -3384,9 +3503,13 @@ export class Session implements SessionContext { toolRun, pendingSend.signal, promptId, + toolLoopState, onFullTurnModel, ); nextMessage = nextAfterTools.message; + if (nextAfterTools.stoppedByRepeatedToolFailure) { + return { stopReason: 'end_turn' }; + } if (toolRun.loopDetected) { this.todoStopGuard.suspend(); await this.#preserveStoppedToolRun( @@ -4333,9 +4456,19 @@ export class Session implements SessionContext { toolRun, pendingSend.signal, toolPromptId, + toolLoopState, options.onFullTurnModel, ); nextMessage = nextAfterTools.message; + if (nextAfterTools.stoppedByRepeatedToolFailure) { + return { + kind: 'terminal', + stopReason: 'end_turn', + ...(supersededAutomaticContinuation + ? { supersededAutomaticContinuation: true } + : {}), + }; + } if (nextAfterTools.hadMidTurnUserInput) { nextGuardContinuation = undefined; continue; @@ -4741,6 +4874,7 @@ export class Session implements SessionContext { toolRun: RunToolResult, abortSignal: AbortSignal, promptId: string, + toolLoopState: DaemonToolLoopState, onFullTurnModel?: (model: string) => boolean, ): Promise { if (toolRun.loopDetected) { @@ -4755,18 +4889,98 @@ export class Session implements SessionContext { return { message: null, hadMidTurnUserInput: false }; } const drained = await this.#drainMidTurnInput(abortSignal, { + watchQueuedPromptForTodoStopGuard: + toolLoopState.repeatedToolFailureMode !== 'off', onFullTurnModel, }); const hadMidTurnUserInput = drained.parts.length > 0; if (hadMidTurnUserInput) { this.todoStopGuard.acceptMidTurnUserInput(); } + if (abortSignal.aborted) { + return { + message: { + role: 'user', + parts: [...toolRun.parts, ...drained.parts], + }, + hadMidTurnUserInput, + }; + } + const previousRepeatedToolFailureState = + toolLoopState.repeatedToolFailureState; + const repeatedToolFailureBatch = toolRun.repeatedToolFailureBatch ?? { + complete: false, + observations: [], + }; + const repeatedToolFailureDecision = reduceRepeatedToolFailureGuard( + previousRepeatedToolFailureState, + { + mode: toolLoopState.repeatedToolFailureMode, + batch: repeatedToolFailureBatch, + hasExternalInput: hadMidTurnUserInput, + hasQueuedPrompt: drained.hasQueuedPrompt, + inputReliable: drained.reliable, + }, + ); + recordRepeatedToolFailureDecision( + this.config, + promptId, + toolLoopState.repeatedToolFailureMode, + previousRepeatedToolFailureState, + repeatedToolFailureDecision, + repeatedToolFailureBatch, + ); + toolLoopState.repeatedToolFailureState = repeatedToolFailureDecision.state; + if (repeatedToolFailureDecision.kind !== 'none') { + const { state } = repeatedToolFailureDecision; + debugLogger.debug( + `[repeated-tool-failure-guard] mode=${toolLoopState.repeatedToolFailureMode} decision=${repeatedToolFailureDecision.kind} phase=${state.phase} candidate=${state.candidateOrdinal} failures=${state.failureCount} batches=${state.batchCount}`, + ); + } const activeTodoReminder = this.config.takeActiveTodoReminder(promptId); const parts = [ ...toolRun.parts, ...(activeTodoReminder ? [{ text: activeTodoReminder }] : []), + ...(repeatedToolFailureDecision.kind === 'warn' + ? [{ text: REPEATED_TOOL_FAILURE_REMINDER }] + : []), ...drained.parts, ]; + if (repeatedToolFailureDecision.kind === 'stop') { + this.todoStopGuard.suspend(); + this.#preserveUnsentMessageHistory( + { + role: 'user', + parts: [ + ...parts, + { text: `System: ${REPEATED_TOOL_FAILURE_STOP_MESSAGE}` }, + ], + }, + true, + ); + await this.messageRewriter?.waitForPendingRewrites(); + recordDaemonLoopDetected( + this.config, + promptId, + LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + REPEATED_TOOL_FAILURE_STOP_MESSAGE, + toolLoopState, + ); + try { + await this.messageEmitter.emitAgentMessage( + REPEATED_TOOL_FAILURE_STOP_MESSAGE, + ); + } catch (error) { + debugLogger.warn( + `Failed to emit repeated tool failure stop message: ${this.#formatError(error)}`, + ); + } + return { + message: null, + hadMidTurnUserInput, + stoppedByRepeatedToolFailure: true, + }; + } return { message: { role: 'user', parts }, hadMidTurnUserInput, @@ -5687,8 +5901,12 @@ export class Session implements SessionContext { toolRun, ac.signal, promptId, + toolLoopState, ); nextMessage = nextAfterTools.message; + if (nextAfterTools.stoppedByRepeatedToolFailure) { + return; + } if (toolRun.loopDetected) { this.todoStopGuard.suspend(); await this.#preserveStoppedToolRun(toolRun, ac.signal); @@ -6202,8 +6420,13 @@ export class Session implements SessionContext { toolRun, ac.signal, promptId, + toolLoopState, ); nextMessage = nextAfterTools.message; + if (nextAfterTools.stoppedByRepeatedToolFailure) { + await this.#emitBackgroundNotificationEndTurn('end_turn'); + return; + } if (toolRun.loopDetected) { this.todoStopGuard.suspend(); await this.#preserveStoppedToolRun(toolRun, ac.signal); @@ -6704,11 +6927,28 @@ export class Session implements SessionContext { const finalizeRunToolResult = async ( result: RunToolResult, ): Promise => { - if (pendingToolResultRecords.length === 0) return result; const orderedRecords = [...pendingToolResultRecords].sort( (left, right) => left.ordinal - right.ordinal || left.sequence - right.sequence, ); + const repeatedToolFailureBatch: RepeatedToolFailureBatch = { + complete: + orderedRecords.length === dedupedFunctionCalls.length && + new Set(orderedRecords.map((record) => record.ordinal)).size === + dedupedFunctionCalls.length, + observations: orderedRecords.map((record) => ({ + callId: record.callId, + policyToolName: record.policyToolName, + toolType: record.toolType, + terminalStatus: record.metadata.status, + executionStatus: record.metadata.executionStatus, + executionErrorType: record.executionErrorType, + providerDuplicate: record.providerDuplicate, + })), + }; + if (orderedRecords.length === 0) { + return { ...result, repeatedToolFailureBatch }; + } const finalized = await finalizeToolResponses( this.config, orderedRecords.map((record) => ({ @@ -6726,6 +6966,7 @@ export class Session implements SessionContext { return { ...result, parts: finalized.flatMap((entry) => entry.responseParts), + repeatedToolFailureBatch, }; }; let skippedToolCallCounter = 0; @@ -6903,6 +7144,7 @@ export class Session implements SessionContext { toolName: request.name, responseParts: response.responseParts, persistedOutputFiles: response.persistedOutputFiles, + providerDuplicate: true, metadata: { callId: response.callId, status: 'error', @@ -7317,6 +7559,7 @@ export class Session implements SessionContext { let terminalStatus: 'success' | 'error' | 'cancelled' | undefined; let toolType: 'native' | 'mcp' = 'native'; let mcpServerName: string | undefined = undefined; + const guardContext: { policyToolName?: string } = {}; if (toolLoopState?.loopDetected) { return { parts: [ @@ -7437,6 +7680,12 @@ export class Session implements SessionContext { callId, toolName, responseParts: errorParts, + policyToolName: guardContext.policyToolName, + toolType, + executionErrorType: + executionStatus === 'error' + ? (executionErrorType ?? opts.errorType) + : undefined, metadata: { callId, status: opts.status, @@ -7517,6 +7766,7 @@ export class Session implements SessionContext { mcpServerName = tool instanceof DiscoveredMCPTool ? tool.serverName : undefined; const policyToolName = tool.name; + guardContext.policyToolName = policyToolName; const originalPolicyRequestArgs = policyToolName === ToolNames.SHELL || policyToolName === ToolNames.MONITOR ? structuredClone(args) @@ -9007,6 +9257,9 @@ export class Session implements SessionContext { toolName, responseParts, persistedOutputFiles: toolResult.persistedOutputFiles, + policyToolName, + toolType, + executionErrorType, metadata: { callId, status, diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts new file mode 100644 index 00000000000..9dbdc2ca0e9 --- /dev/null +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts @@ -0,0 +1,338 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + ToolErrorType, + type ToolExecutionStatus, +} from '@qwen-code/qwen-code-core'; +import { describe, expect, it } from 'vitest'; +import { + createRepeatedToolFailureGuardState, + reduceRepeatedToolFailureGuard, + REPEATED_TOOL_FAILURE_REMINDER, + REPEATED_TOOL_FAILURE_STOP_MESSAGE, + resolveRepeatedToolFailureGuardMode, + type RepeatedToolFailureGuardMode, + type RepeatedToolFailureObservation, + type RepeatedToolFailureTerminalStatus, +} from './repeated-tool-failure-guard.js'; + +function observation( + overrides: Partial = {}, +): RepeatedToolFailureObservation { + return { + callId: 'call-1', + policyToolName: 'read_file', + toolType: 'native', + terminalStatus: 'error', + executionStatus: 'error', + executionErrorType: ToolErrorType.FILE_NOT_FOUND, + ...overrides, + }; +} + +function reduce( + state: ReturnType, + observations: RepeatedToolFailureObservation[], + options: { + mode?: RepeatedToolFailureGuardMode; + complete?: boolean; + hasExternalInput?: boolean; + hasQueuedPrompt?: boolean; + inputReliable?: boolean; + } = {}, +) { + return reduceRepeatedToolFailureGuard(state, { + mode: options.mode ?? 'enforce', + batch: { + complete: options.complete ?? true, + observations, + }, + hasExternalInput: options.hasExternalInput ?? false, + hasQueuedPrompt: options.hasQueuedPrompt ?? false, + inputReliable: options.inputReliable ?? true, + }); +} + +describe('repeated tool failure guard', () => { + it('defaults invalid or missing deployment modes to shadow', () => { + expect(resolveRepeatedToolFailureGuardMode(undefined)).toBe('shadow'); + expect(resolveRepeatedToolFailureGuardMode('invalid')).toBe('shadow'); + expect(resolveRepeatedToolFailureGuardMode(' WARN ')).toBe('warn'); + }); + + it('does no work when the deployment mode is off', () => { + const state = createRepeatedToolFailureGuardState(); + expect(reduce(state, [observation()], { mode: 'off' })).toEqual({ + kind: 'none', + state, + }); + }); + + it('requires eight failures across at least two batches before warning', () => { + const first = reduce( + createRepeatedToolFailureGuardState(), + Array.from({ length: 8 }, (_, index) => + observation({ callId: `call-${index}` }), + ), + ); + expect(first.kind).toBe('tracked'); + expect(first.state).toMatchObject({ + phase: 'tracking', + failureCount: 8, + batchCount: 1, + }); + + const second = reduce(first.state, [observation({ callId: 'call-9' })]); + expect(second.kind).toBe('warn'); + expect(second.state).toMatchObject({ + phase: 'warned', + failureCount: 9, + batchCount: 2, + }); + }); + + it('stops only after the next complete matching batch', () => { + const first = reduce( + createRepeatedToolFailureGuardState(), + Array.from({ length: 4 }, (_, index) => + observation({ callId: `first-${index}` }), + ), + ); + const warned = reduce( + first.state, + Array.from({ length: 4 }, (_, index) => + observation({ callId: `second-${index}` }), + ), + ); + const stopped = reduce(warned.state, [ + observation({ callId: 'post-warning' }), + ]); + + expect(warned.kind).toBe('warn'); + expect(stopped.kind).toBe('stop'); + expect(stopped.state.phase).toBe('latched'); + expect( + reduce(stopped.state, [observation({ callId: 'ignored' })]).kind, + ).toBe('none'); + }); + + it.each([ + ['shadow', 'would_warn', 'would_stop'], + ['warn', 'warn', 'would_stop'], + ['enforce', 'warn', 'stop'], + ] as const)( + 'applies %s mode without changing detection semantics', + (mode, warningKind, stopKind) => { + const first = reduce( + createRepeatedToolFailureGuardState(), + Array.from({ length: 4 }, (_, index) => + observation({ callId: `first-${index}` }), + ), + { mode }, + ); + const warning = reduce( + first.state, + Array.from({ length: 4 }, (_, index) => + observation({ callId: `second-${index}` }), + ), + { mode }, + ); + const stop = reduce( + warning.state, + [observation({ callId: 'post-warning' })], + { mode }, + ); + + expect(warning.kind).toBe(warningKind); + expect(stop.kind).toBe(stopKind); + }, + ); + + it.each([ + ['success', 'success', 'success'], + ['synthetic success', 'success', 'not_started'], + ['cancelled', 'cancelled', 'cancelled'], + ['execution cancelled', 'error', 'cancelled'], + ['not started', 'error', 'not_started'], + ['post execution', 'error', 'success'], + ] as const)( + 'resets a streak for a %s outcome', + ( + _label, + terminalStatus: RepeatedToolFailureTerminalStatus, + executionStatus: ToolExecutionStatus, + ) => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const result = reduce(tracked.state, [ + observation({ terminalStatus, executionStatus }), + ]); + + expect(result).toMatchObject({ + kind: 'reset', + state: { phase: 'idle', failureCount: 0, batchCount: 0 }, + }); + }, + ); + + it('ignores provider duplicates without advancing or resetting', () => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const duplicate = reduce(tracked.state, [ + observation({ providerDuplicate: true }), + ]); + + expect(duplicate).toEqual({ kind: 'none', state: tracked.state }); + }); + + it('resets mixed failure keys and assigns a new candidate to a new key', () => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const mixed = reduce(tracked.state, [ + observation(), + observation({ + callId: 'call-2', + executionErrorType: ToolErrorType.PERMISSION_DENIED, + }), + ]); + const next = reduce(mixed.state, [ + observation({ + executionErrorType: ToolErrorType.PERMISSION_DENIED, + }), + ]); + + expect(mixed).toMatchObject({ kind: 'reset', reason: 'mixed' }); + expect(next).toMatchObject({ + kind: 'tracked', + state: { candidateOrdinal: 2 }, + }); + }); + + it.each([ + [{ complete: false }, 'incomplete'], + [{ hasExternalInput: true }, 'external_input'], + [{ hasQueuedPrompt: true }, 'queued_prompt'], + [{ inputReliable: false }, 'unreliable_input'], + ] as const)('resets for boundary condition %s', (options, reason) => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const result = reduce(tracked.state, [observation()], options); + + expect(result).toMatchObject({ kind: 'reset', reason }); + }); + + it('downgrades enforcement after an unknown execution contract', () => { + const invalid = reduce(createRepeatedToolFailureGuardState(), [ + observation({ executionStatus: undefined }), + ]); + expect(invalid).toMatchObject({ + reason: 'unknown', + state: { enforcementDisabled: true }, + }); + + const first = reduce( + invalid.state, + Array.from({ length: 4 }, (_, index) => + observation({ callId: `first-${index}` }), + ), + ); + const warning = reduce( + first.state, + Array.from({ length: 4 }, (_, index) => + observation({ callId: `second-${index}` }), + ), + ); + const stop = reduce(warning.state, [observation()]); + + expect(warning.kind).toBe('warn'); + expect(stop.kind).toBe('would_stop'); + }); + + it.each([ + [ + 'success/error', + { terminalStatus: 'success' as const, executionStatus: 'error' as const }, + 'contract_violation', + ], + [ + 'success/cancelled', + { + terminalStatus: 'success' as const, + executionStatus: 'cancelled' as const, + }, + 'contract_violation', + ], + [ + 'unknown execution error type', + { executionErrorType: ToolErrorType.UNKNOWN }, + 'unknown', + ], + ['unknown execution status', { executionStatus: 'unknown' }, 'unknown'], + ['missing policy tool identity', { policyToolName: undefined }, 'unknown'], + ] as const)('downgrades enforcement for %s', (_label, overrides, reason) => { + const result = reduce(createRepeatedToolFailureGuardState(), [ + observation(overrides), + ]); + + expect(result).toMatchObject({ + reason, + state: { + phase: 'idle', + enforcementDisabled: true, + }, + }); + }); + + it('does not count the terminal error type after successful execution', () => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const result = reduce(tracked.state, [ + observation({ + executionStatus: 'success', + executionErrorType: ToolErrorType.UNHANDLED_EXCEPTION, + }), + ]); + + expect(result).toMatchObject({ + kind: 'reset', + reason: 'post_execution_failure', + }); + }); + + it('lets terminal cancellation win when execution status is missing', () => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const result = reduce(tracked.state, [ + observation({ + terminalStatus: 'cancelled', + executionStatus: undefined, + executionErrorType: undefined, + }), + ]); + + expect(result).toMatchObject({ + kind: 'reset', + reason: 'cancelled', + state: { enforcementDisabled: false }, + }); + }); + + it('uses fixed privacy-safe reminder and stop text', () => { + expect(REPEATED_TOOL_FAILURE_REMINDER).toBe( + 'System: the same tool execution has failed repeatedly for the same classified reason. Do not repeat the same approach. Inspect the returned result, change the approach or required preconditions, or explain the blocker.', + ); + expect(REPEATED_TOOL_FAILURE_STOP_MESSAGE).toBe( + 'Automatic continuation stopped because the same tool execution failure continued after a corrective reminder. New user input is required to continue.', + ); + }); +}); diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts new file mode 100644 index 00000000000..fd5ea895598 --- /dev/null +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts @@ -0,0 +1,299 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + ToolErrorType, + type ToolExecutionStatus, +} from '@qwen-code/qwen-code-core'; + +export const REPEATED_TOOL_FAILURE_THRESHOLD = 8; +export const REPEATED_TOOL_FAILURE_BATCH_THRESHOLD = 2; + +export const REPEATED_TOOL_FAILURE_REMINDER = + 'System: the same tool execution has failed repeatedly for the same classified reason. Do not repeat the same approach. Inspect the returned result, change the approach or required preconditions, or explain the blocker.'; + +export const REPEATED_TOOL_FAILURE_STOP_MESSAGE = + 'Automatic continuation stopped because the same tool execution failure continued after a corrective reminder. New user input is required to continue.'; + +export type RepeatedToolFailureGuardMode = + | 'off' + | 'shadow' + | 'warn' + | 'enforce'; + +export type RepeatedToolFailureTerminalStatus = + | 'success' + | 'error' + | 'cancelled'; + +export type RepeatedToolFailureObservation = { + callId: string; + policyToolName?: string; + toolType?: 'native' | 'mcp'; + terminalStatus: RepeatedToolFailureTerminalStatus; + executionStatus?: ToolExecutionStatus | 'unknown'; + executionErrorType?: ToolErrorType; + providerDuplicate?: boolean; +}; + +export type RepeatedToolFailureBatch = { + complete: boolean; + observations: readonly RepeatedToolFailureObservation[]; +}; + +type FailureKey = { + policyToolName: string; + executionErrorType: ToolErrorType; +}; + +export type RepeatedToolFailureGuardPhase = + | 'idle' + | 'tracking' + | 'warned' + | 'latched'; + +export type RepeatedToolFailureGuardState = { + phase: RepeatedToolFailureGuardPhase; + key?: FailureKey; + failureCount: number; + batchCount: number; + candidateOrdinal: number; + nextCandidateOrdinal: number; + enforcementDisabled: boolean; +}; + +export type RepeatedToolFailureResetReason = + | 'success' + | 'cancelled' + | 'not_started' + | 'post_execution_failure' + | 'unknown' + | 'mixed' + | 'incomplete' + | 'external_input' + | 'queued_prompt' + | 'unreliable_input' + | 'contract_violation'; + +export type RepeatedToolFailureGuardDecision = + | { kind: 'none'; state: RepeatedToolFailureGuardState } + | { + kind: 'reset'; + state: RepeatedToolFailureGuardState; + reason: RepeatedToolFailureResetReason; + } + | { + kind: 'tracked'; + state: RepeatedToolFailureGuardState; + } + | { + kind: 'would_warn' | 'warn' | 'would_stop' | 'stop'; + state: RepeatedToolFailureGuardState; + }; + +export type RepeatedToolFailureGuardInput = { + mode: RepeatedToolFailureGuardMode; + batch: RepeatedToolFailureBatch; + hasExternalInput: boolean; + hasQueuedPrompt: boolean; + inputReliable: boolean; +}; + +export function createRepeatedToolFailureGuardState(): RepeatedToolFailureGuardState { + return { + phase: 'idle', + failureCount: 0, + batchCount: 0, + candidateOrdinal: 0, + nextCandidateOrdinal: 1, + enforcementDisabled: false, + }; +} + +export function resolveRepeatedToolFailureGuardMode( + value: string | undefined, +): RepeatedToolFailureGuardMode { + switch (value?.trim().toLowerCase()) { + case 'off': + case 'shadow': + case 'warn': + case 'enforce': + return value.trim().toLowerCase() as RepeatedToolFailureGuardMode; + default: + return 'shadow'; + } +} + +function resetState( + state: RepeatedToolFailureGuardState, + enforcementDisabled = state.enforcementDisabled, +): RepeatedToolFailureGuardState { + return { + phase: 'idle', + failureCount: 0, + batchCount: 0, + candidateOrdinal: 0, + nextCandidateOrdinal: state.nextCandidateOrdinal, + enforcementDisabled, + }; +} + +function reset( + state: RepeatedToolFailureGuardState, + reason: RepeatedToolFailureResetReason, + enforcementDisabled = state.enforcementDisabled, +): RepeatedToolFailureGuardDecision { + const nextState = resetState(state, enforcementDisabled); + if ( + state.phase === 'idle' && + state.enforcementDisabled === enforcementDisabled + ) { + return { kind: 'none', state: nextState }; + } + return { kind: 'reset', state: nextState, reason }; +} + +function keysEqual(left: FailureKey | undefined, right: FailureKey): boolean { + return ( + left?.policyToolName === right.policyToolName && + left.executionErrorType === right.executionErrorType + ); +} + +function effectiveMode( + mode: RepeatedToolFailureGuardMode, + enforcementDisabled: boolean, +): RepeatedToolFailureGuardMode { + return mode === 'enforce' && enforcementDisabled ? 'warn' : mode; +} + +export function reduceRepeatedToolFailureGuard( + state: RepeatedToolFailureGuardState, + input: RepeatedToolFailureGuardInput, +): RepeatedToolFailureGuardDecision { + if (input.mode === 'off' || state.phase === 'latched') { + return { kind: 'none', state }; + } + if (!input.inputReliable) { + return reset(state, 'unreliable_input', true); + } + if (input.hasExternalInput) { + return reset(state, 'external_input'); + } + if (input.hasQueuedPrompt) { + return reset(state, 'queued_prompt'); + } + if (!input.batch.complete) { + return reset(state, 'incomplete'); + } + + const observations = input.batch.observations.filter( + (observation) => !observation.providerDuplicate, + ); + if (observations.length === 0) { + return { kind: 'none', state }; + } + + const eligible: FailureKey[] = []; + for (const observation of observations) { + const { terminalStatus, executionStatus } = observation; + if ( + terminalStatus === 'success' && + (executionStatus === 'error' || executionStatus === 'cancelled') + ) { + return reset(state, 'contract_violation', true); + } + if (terminalStatus === 'cancelled') { + return reset(state, 'cancelled'); + } + if (executionStatus === undefined || executionStatus === 'unknown') { + return reset(state, 'unknown', true); + } + if (executionStatus === 'cancelled') { + return reset(state, 'cancelled'); + } + if (terminalStatus === 'success') { + return reset(state, 'success'); + } + if (executionStatus === 'not_started') { + return reset(state, 'not_started'); + } + if (executionStatus === 'success') { + return reset(state, 'post_execution_failure'); + } + if ( + !observation.policyToolName || + observation.executionErrorType === undefined || + observation.executionErrorType === ToolErrorType.UNKNOWN + ) { + return reset(state, 'unknown', true); + } + eligible.push({ + policyToolName: observation.policyToolName, + executionErrorType: observation.executionErrorType, + }); + } + + const key = eligible[0]; + if (!key) { + return reset(state, 'unknown', true); + } + if (eligible.some((entry) => !keysEqual(entry, key))) { + return reset(state, 'mixed'); + } + + if (!keysEqual(state.key, key)) { + const nextState: RepeatedToolFailureGuardState = { + phase: 'tracking', + key, + failureCount: eligible.length, + batchCount: 1, + candidateOrdinal: state.nextCandidateOrdinal, + nextCandidateOrdinal: state.nextCandidateOrdinal + 1, + enforcementDisabled: state.enforcementDisabled, + }; + return { kind: 'tracked', state: nextState }; + } + + const failureCount = state.failureCount + eligible.length; + const batchCount = state.batchCount + 1; + if (state.phase === 'warned') { + const nextState: RepeatedToolFailureGuardState = { + ...state, + phase: 'latched', + failureCount, + batchCount, + }; + return { + kind: + effectiveMode(input.mode, state.enforcementDisabled) === 'enforce' + ? 'stop' + : 'would_stop', + state: nextState, + }; + } + + const nextState: RepeatedToolFailureGuardState = { + ...state, + failureCount, + batchCount, + }; + if ( + failureCount < REPEATED_TOOL_FAILURE_THRESHOLD || + batchCount < REPEATED_TOOL_FAILURE_BATCH_THRESHOLD + ) { + return { kind: 'tracked', state: nextState }; + } + + nextState.phase = 'warned'; + return { + kind: + effectiveMode(input.mode, state.enforcementDisabled) === 'shadow' + ? 'would_warn' + : 'warn', + state: nextState, + }; +} diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 725f705044f..8bcad6f8596 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -172,6 +172,8 @@ const LOOP_TYPE_LABELS: Record = { 'the turn reached the per-turn tool-call limit', [LoopType.INVALID_TOOL_PARAMS_STAGNATION]: 'the model repeatedly sent invalid tool parameters without correcting them', + [LoopType.REPEATED_TOOL_EXECUTION_FAILURE]: + 'the same tool execution failure continued after a corrective reminder', }; function formatLoopDetectedMessage(loopType: LoopType | undefined): string { @@ -185,7 +187,8 @@ function formatLoopDetectedMessage(loopType: LoopType | undefined): string { loopType === LoopType.CONSECUTIVE_IDENTICAL_TOOL_CALLS || loopType === LoopType.SHELL_COMMAND_STAGNATION || loopType === LoopType.GLOBAL_TOOL_CALL_DUPLICATE || - loopType === LoopType.INVALID_TOOL_PARAMS_STAGNATION; + loopType === LoopType.INVALID_TOOL_PARAMS_STAGNATION || + loopType === LoopType.REPEATED_TOOL_EXECUTION_FAILURE; const hint = loopType === LoopType.TURN_TOOL_CALL_CAP ? ' A per-turn tool-call cap was reached. The default is adaptive (allows up to 1000 diverse calls, halting only on repeated calls); an explicitly set `model.maxToolCallsPerTurn` is a hard cap. If the model was repeating the same call, investigate the repetition; otherwise unset the value to use the adaptive default, or raise it (set 0 to disable).' diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 98759126b2b..bdc1fac4e6f 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -457,6 +457,7 @@ export { logExtensionEnable, logIdeConnection, logLoopDetected, + logRepeatedToolFailureGuard, logModelSlashCommand, logPromptSuggestion, logSpeculation, @@ -473,6 +474,7 @@ export { IdeConnectionType, LoopDetectedEvent, LoopType, + RepeatedToolFailureGuardEvent, ModelSlashCommandEvent, PromptSuggestionEvent, SpeculationEvent, diff --git a/packages/core/src/telemetry/constants.ts b/packages/core/src/telemetry/constants.ts index 66e89e612f2..44d69d2004c 100644 --- a/packages/core/src/telemetry/constants.ts +++ b/packages/core/src/telemetry/constants.ts @@ -9,6 +9,8 @@ export const SERVICE_NAME = 'qwen-code'; export const EVENT_USER_PROMPT = 'qwen-code.user_prompt'; export const EVENT_USER_RETRY = 'qwen-code.user_retry'; export const EVENT_TOOL_CALL = 'qwen-code.tool_call'; +export const EVENT_REPEATED_TOOL_FAILURE_GUARD = + 'qwen-code.repeated_tool_failure_guard'; export const EVENT_API_REQUEST = 'qwen-code.api_request'; export const EVENT_API_ERROR = 'qwen-code.api_error'; export const EVENT_API_CANCEL = 'qwen-code.api_cancel'; diff --git a/packages/core/src/telemetry/index.ts b/packages/core/src/telemetry/index.ts index 363982d5e4b..cf3b7723e66 100644 --- a/packages/core/src/telemetry/index.ts +++ b/packages/core/src/telemetry/index.ts @@ -35,6 +35,7 @@ export { logUserPrompt, logUserRetry, logToolCall, + logRepeatedToolFailureGuard, logApiRequest, logApiError, logApiCancel, @@ -113,6 +114,7 @@ export { // Core metrics functions recordToolCallMetrics, recordToolExecutionMetrics, + recordRepeatedToolFailureGuardMetrics, recordTokenUsageMetrics, recordApiResponseMetrics, recordApiErrorMetrics, @@ -150,6 +152,7 @@ export { ApiRequestPhase, FileOperation, } from './metrics.js'; +export { RepeatedToolFailureGuardEvent } from './types.js'; export { QwenLogger } from './qwen-logger/qwen-logger.js'; export { sanitizeHookName } from './sanitize.js'; export { diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 62221808a35..34b891dc7fb 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -29,6 +29,7 @@ import { EVENT_CLI_CONFIG, EVENT_FLASH_FALLBACK, EVENT_TOOL_CALL, + EVENT_REPEATED_TOOL_FAILURE_GUARD, EVENT_USER_PROMPT, EVENT_MALFORMED_JSON_RESPONSE, EVENT_FILE_OPERATION, @@ -49,6 +50,8 @@ import { logStartSession, logUserPrompt, logToolCall, + logLoopDetected, + logRepeatedToolFailureGuard, logFlashFallback, logChatCompression, logMalformedJsonResponse, @@ -97,6 +100,9 @@ import { ApiRetryEvent, ProtocolTagSanitizedEvent, MemoryRecallDeliveryEvent, + LoopDetectedEvent, + LoopType, + RepeatedToolFailureGuardEvent, } from './types.js'; import { FileOperation } from './metrics.js'; import type { @@ -347,6 +353,122 @@ describe('loggers', () => { }); }); + describe('logRepeatedToolFailureGuard', () => { + it('emits a privacy-safe transition log and low-cardinality metric', () => { + const config = makeFakeConfig({ sessionId: 'test-session-id' }); + vi.spyOn( + metrics, + 'recordRepeatedToolFailureGuardMetrics', + ).mockImplementation(() => undefined); + const event = new RepeatedToolFailureGuardEvent({ + prompt_id: 'prompt-id', + route: 'acp_foreground', + mode: 'shadow', + phase_before: 'tracking', + phase_after: 'warned', + decision: 'would_warn', + failure_count_bucket: '8+', + batch_count_bucket: '2', + candidate_ordinal: 1, + terminal_status: 'error', + execution_status: 'error', + execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, + tool_type: 'mcp', + }); + + logRepeatedToolFailureGuard(config, event); + + expect(mockLogger.emit).toHaveBeenCalledWith({ + body: 'Repeated tool failure guard decision: would_warn.', + attributes: { + ...event, + 'event.name': EVENT_REPEATED_TOOL_FAILURE_GUARD, + }, + }); + expect( + metrics.recordRepeatedToolFailureGuardMetrics, + ).toHaveBeenCalledWith({ + route: 'acp_foreground', + mode: 'shadow', + phase_before: 'tracking', + phase_after: 'warned', + decision: 'would_warn', + failure_count_bucket: '8+', + batch_count_bucket: '2', + terminal_status: 'error', + execution_status: 'error', + execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, + tool_type: 'mcp', + }); + const serialized = JSON.stringify(mockLogger.emit.mock.calls.at(-1)); + expect(serialized).not.toMatch( + /session.id|user.id|policyToolName|function_args|result|error_message|server_name/, + ); + }); + + it('isolates transition log and metric sink failures', () => { + const config = makeFakeConfig({ sessionId: 'test-session-id' }); + const event = new RepeatedToolFailureGuardEvent({ + prompt_id: 'prompt-id', + route: 'acp_foreground', + mode: 'enforce', + phase_before: 'warned', + phase_after: 'latched', + decision: 'stopped', + failure_count_bucket: '8+', + batch_count_bucket: '3+', + candidate_ordinal: 1, + }); + vi.spyOn( + metrics, + 'recordRepeatedToolFailureGuardMetrics', + ).mockImplementationOnce(() => { + throw new Error('metric unavailable'); + }); + mockLogger.emit.mockImplementationOnce(() => { + throw new Error('log unavailable'); + }); + + expect(() => logRepeatedToolFailureGuard(config, event)).not.toThrow(); + expect(event).not.toHaveProperty('reset_reason'); + expect(event).not.toHaveProperty('terminal_status'); + expect(event).not.toHaveProperty('execution_status'); + expect(event).not.toHaveProperty('execution_error_type'); + expect(event).not.toHaveProperty('tool_type'); + }); + }); + + describe('logLoopDetected', () => { + it('keeps repeated execution failure stops out of session-scoped RUM', () => { + const config = makeFakeConfig({ sessionId: 'test-session-id' }); + const logLoopDetectedEvent = vi.fn(); + const getInstanceSpy = vi + .spyOn(QwenLogger, 'getInstance') + .mockReturnValue({ + logLoopDetectedEvent, + } as unknown as QwenLogger); + const event = new LoopDetectedEvent( + LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + 'prompt-id', + ); + + try { + logLoopDetected(config, event); + + expect(logLoopDetectedEvent).not.toHaveBeenCalled(); + expect(mockLogger.emit).toHaveBeenCalledWith({ + body: `Loop detected. Type: ${LoopType.REPEATED_TOOL_EXECUTION_FAILURE}.`, + attributes: event, + }); + expect( + mockLogger.emit.mock.calls.at(-1)?.[0].attributes, + ).not.toHaveProperty('session.id'); + } finally { + getInstanceSpy.mockRestore(); + } + }); + }); + describe('logUserPrompt', () => { const mockConfig = { getSessionId: () => 'test-session-id', diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index acd4a647d02..fd721ebe117 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -20,6 +20,7 @@ import { EVENT_EXTENSION_ENABLE, EVENT_IDE_CONNECTION, EVENT_TOOL_CALL, + EVENT_REPEATED_TOOL_FAILURE_GUARD, EVENT_USER_PROMPT, EVENT_USER_RETRY, EVENT_FLASH_FALLBACK, @@ -72,6 +73,7 @@ import { recordTokenUsageMetrics, recordToolCallMetrics, recordToolExecutionMetrics, + recordRepeatedToolFailureGuardMetrics, recordArenaSessionStartedMetrics, recordArenaAgentCompletedMetrics, recordArenaSessionEndedMetrics, @@ -96,6 +98,7 @@ import type { FlashFallbackEvent, NextSpeakerCheckEvent, LoopDetectedEvent, + RepeatedToolFailureGuardEvent, LoopDetectionDisabledEvent, SlashCommandEvent, ConversationFinishedEvent, @@ -132,6 +135,7 @@ import type { MemoryRecallEvent, MemoryRecallDeliveryEvent, } from './types.js'; +import { LoopType } from './types.js'; import type { HookCallEvent } from './types.js'; import type { UiEvent } from './uiTelemetry.js'; import { uiTelemetryService } from './uiTelemetry.js'; @@ -636,11 +640,15 @@ export function logLoopDetected( config: Config, event: LoopDetectedEvent, ): void { - QwenLogger.getInstance(config)?.logLoopDetectedEvent(event); + const privacyRestricted = + event.loop_type === LoopType.REPEATED_TOOL_EXECUTION_FAILURE; + if (!privacyRestricted) { + QwenLogger.getInstance(config)?.logLoopDetectedEvent(event); + } if (!isTelemetrySdkInitialized()) return; const attributes: LogAttributes = { - ...getCommonAttributes(config), + ...(privacyRestricted ? {} : getCommonAttributes(config)), ...event, }; @@ -652,6 +660,52 @@ export function logLoopDetected( logger.emit(logRecord); } +export function logRepeatedToolFailureGuard( + _config: Config, + event: RepeatedToolFailureGuardEvent, +): void { + let sdkInitialized = false; + runToolTelemetrySink(() => { + sdkInitialized = isTelemetrySdkInitialized(); + }); + if (sdkInitialized) { + runToolTelemetrySink(() => { + const logger = logs.getLogger(SERVICE_NAME); + logger.emit({ + body: `Repeated tool failure guard decision: ${event.decision}.`, + attributes: { + ...event, + 'event.name': EVENT_REPEATED_TOOL_FAILURE_GUARD, + }, + }); + }); + } + runToolTelemetrySink(() => { + recordRepeatedToolFailureGuardMetrics({ + route: event.route, + mode: event.mode, + phase_before: event.phase_before, + phase_after: event.phase_after, + decision: event.decision, + failure_count_bucket: event.failure_count_bucket, + batch_count_bucket: event.batch_count_bucket, + ...(event.reset_reason !== undefined + ? { reset_reason: event.reset_reason } + : {}), + ...(event.terminal_status !== undefined + ? { terminal_status: event.terminal_status } + : {}), + ...(event.execution_status !== undefined + ? { execution_status: event.execution_status } + : {}), + ...(event.execution_error_type !== undefined + ? { execution_error_type: event.execution_error_type } + : {}), + ...(event.tool_type !== undefined ? { tool_type: event.tool_type } : {}), + }); + }); +} + export function logLoopDetectionDisabled( config: Config, _event: LoopDetectionDisabledEvent, diff --git a/packages/core/src/telemetry/metrics.test.ts b/packages/core/src/telemetry/metrics.test.ts index df147e6ab4e..e391141a416 100644 --- a/packages/core/src/telemetry/metrics.test.ts +++ b/packages/core/src/telemetry/metrics.test.ts @@ -20,6 +20,7 @@ import { ApiRequestPhase, } from './metrics.js'; import { makeFakeConfig } from '../test-utils/config.js'; +import { ToolErrorType } from '../tools/tool-error.js'; const mockCounterAddFn: Mock< (value: number, attributes?: Attributes, context?: Context) => void @@ -70,6 +71,7 @@ describe('Telemetry Metrics', () => { let recordToolCallMetricsModule: typeof import('./metrics.js').recordToolCallMetrics; let recordTokenUsageMetricsModule: typeof import('./metrics.js').recordTokenUsageMetrics; let recordToolExecutionMetricsModule: typeof import('./metrics.js').recordToolExecutionMetrics; + let recordRepeatedToolFailureGuardMetricsModule: typeof import('./metrics.js').recordRepeatedToolFailureGuardMetrics; let recordFileOperationMetricModule: typeof import('./metrics.js').recordFileOperationMetric; let recordChatCompressionMetricsModule: typeof import('./metrics.js').recordChatCompressionMetrics; let recordStartupPerformanceModule: typeof import('./metrics.js').recordStartupPerformance; @@ -99,6 +101,8 @@ describe('Telemetry Metrics', () => { recordTokenUsageMetricsModule = metricsJsModule.recordTokenUsageMetrics; recordToolExecutionMetricsModule = metricsJsModule.recordToolExecutionMetrics; + recordRepeatedToolFailureGuardMetricsModule = + metricsJsModule.recordRepeatedToolFailureGuardMetrics; recordFileOperationMetricModule = metricsJsModule.recordFileOperationMetric; recordChatCompressionMetricsModule = metricsJsModule.recordChatCompressionMetrics; @@ -336,6 +340,49 @@ describe('Telemetry Metrics', () => { }); }); + describe('recordRepeatedToolFailureGuardMetrics', () => { + const config = makeFakeConfig({ + sessionId: 'test-session-id', + }); + + it('records only low-cardinality transition attributes', () => { + initializeMetricsModule(config); + mockCounterAddFn.mockClear(); + + recordRepeatedToolFailureGuardMetricsModule({ + route: 'acp_foreground', + mode: 'enforce', + phase_before: 'warned', + phase_after: 'latched', + decision: 'stopped', + failure_count_bucket: '8+', + batch_count_bucket: '3+', + terminal_status: 'error', + execution_status: 'error', + execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, + tool_type: 'mcp', + }); + + expect(mockCreateCounterFn).toHaveBeenCalledWith( + 'qwen-code.repeated_tool_failure_guard.count', + expect.any(Object), + ); + expect(mockCounterAddFn).toHaveBeenCalledWith(1, { + route: 'acp_foreground', + mode: 'enforce', + phase_before: 'warned', + phase_after: 'latched', + decision: 'stopped', + failure_count_bucket: '8+', + batch_count_bucket: '3+', + terminal_status: 'error', + execution_status: 'error', + execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, + tool_type: 'mcp', + }); + }); + }); + describe('recordFileOperationMetric', () => { const mockConfig = { getSessionId: () => 'test-session-id', diff --git a/packages/core/src/telemetry/metrics.ts b/packages/core/src/telemetry/metrics.ts index c8b2676aed5..675adc73a1d 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -16,9 +16,11 @@ import type { MemoryRecallDiscardReason, } from './types.js'; import type { ToolExecutionStatus } from '../core/turn.js'; +import type { ToolErrorType } from '../tools/tool-error.js'; const TOOL_CALL_COUNT = `${SERVICE_NAME}.tool.call.count`; const TOOL_EXECUTION_COUNT = `${SERVICE_NAME}.tool.execution.count`; +export const REPEATED_TOOL_FAILURE_GUARD_COUNT = `${SERVICE_NAME}.repeated_tool_failure_guard.count`; const TOOL_CALL_LATENCY = `${SERVICE_NAME}.tool.call.latency`; const API_REQUEST_COUNT = `${SERVICE_NAME}.api.request.count`; const API_REQUEST_LATENCY = `${SERVICE_NAME}.api.request.latency`; @@ -107,6 +109,43 @@ const COUNTER_DEFINITIONS = { tool_type: 'native' | 'mcp'; }, }, + [REPEATED_TOOL_FAILURE_GUARD_COUNT]: { + description: + 'Counts privacy-safe repeated tool execution failure guard transitions.', + valueType: ValueType.INT, + assign: (c: Counter) => (repeatedToolFailureGuardCounter = c), + attributes: {} as { + route: 'acp_foreground'; + mode: 'shadow' | 'warn' | 'enforce'; + phase_before: 'idle' | 'tracking' | 'warned' | 'latched'; + phase_after: 'idle' | 'tracking' | 'warned' | 'latched'; + decision: + | 'reset' + | 'tracked' + | 'would_warn' + | 'warned' + | 'would_stop' + | 'stopped'; + failure_count_bucket: '0' | '1-2' | '3-4' | '5-7' | '8+'; + batch_count_bucket: '0' | '1' | '2' | '3+'; + reset_reason?: + | 'success' + | 'cancelled' + | 'not_started' + | 'post_execution_failure' + | 'unknown' + | 'mixed' + | 'incomplete' + | 'external_input' + | 'queued_prompt' + | 'unreliable_input' + | 'contract_violation'; + terminal_status?: 'error'; + execution_status?: 'error'; + execution_error_type?: ToolErrorType; + tool_type?: 'native' | 'mcp'; + }, + }, [API_REQUEST_COUNT]: { description: 'Counts API requests, tagged by model and status.', valueType: ValueType.INT, @@ -383,6 +422,7 @@ export enum ApiRequestPhase { let cliMeter: Meter | undefined; let toolCallCounter: Counter | undefined; let toolExecutionCounter: Counter | undefined; +let repeatedToolFailureGuardCounter: Counter | undefined; let toolCallLatencyHistogram: Histogram | undefined; let apiRequestCounter: Counter | undefined; let apiRequestLatencyHistogram: Histogram | undefined; @@ -625,6 +665,13 @@ export function recordToolExecutionMetrics( }); } +export function recordRepeatedToolFailureGuardMetrics( + attributes: MetricDefinitions[typeof REPEATED_TOOL_FAILURE_GUARD_COUNT]['attributes'], +): void { + if (!repeatedToolFailureGuardCounter || !isMetricsInitialized) return; + repeatedToolFailureGuardCounter.add(1, attributes); +} + export function recordTokenUsageMetrics( config: Config, tokenCount: number, diff --git a/packages/core/src/telemetry/types.ts b/packages/core/src/telemetry/types.ts index 9222c8b0e4a..a6604409046 100644 --- a/packages/core/src/telemetry/types.ts +++ b/packages/core/src/telemetry/types.ts @@ -24,6 +24,7 @@ import { ToolNames } from '../tools/tool-names.js'; import { STRUCTURED_OUTPUT_REDACTED_ARGS } from '../tools/syntheticOutput.js'; import type { SkillTool } from '../tools/skill.js'; import type { AgentTool } from '../tools/agent/agent.js'; +import type { ToolErrorType } from '../tools/tool-error.js'; export interface BaseTelemetryEvent { 'event.name': string; @@ -486,6 +487,8 @@ export enum LoopType { TURN_TOOL_CALL_CAP = 'turn_tool_call_cap', /** The same tool repeatedly failed schema validation with fresh tool-call ids. */ INVALID_TOOL_PARAMS_STAGNATION = 'invalid_tool_params_stagnation', + /** The same tool execution failure continued after a corrective reminder. */ + REPEATED_TOOL_EXECUTION_FAILURE = 'repeated_tool_execution_failure', } export class LoopDetectedEvent implements BaseTelemetryEvent { @@ -502,6 +505,95 @@ export class LoopDetectedEvent implements BaseTelemetryEvent { } } +export type RepeatedToolFailureGuardTelemetryMode = + | 'shadow' + | 'warn' + | 'enforce'; +export type RepeatedToolFailureGuardTelemetryPhase = + | 'idle' + | 'tracking' + | 'warned' + | 'latched'; +export type RepeatedToolFailureGuardTelemetryDecision = + | 'reset' + | 'tracked' + | 'would_warn' + | 'warned' + | 'would_stop' + | 'stopped'; +export type RepeatedToolFailureGuardCountBucket = + | '0' + | '1-2' + | '3-4' + | '5-7' + | '8+'; +export type RepeatedToolFailureGuardBatchBucket = '0' | '1' | '2' | '3+'; +export type RepeatedToolFailureGuardResetReason = + | 'success' + | 'cancelled' + | 'not_started' + | 'post_execution_failure' + | 'unknown' + | 'mixed' + | 'incomplete' + | 'external_input' + | 'queued_prompt' + | 'unreliable_input' + | 'contract_violation'; + +export class RepeatedToolFailureGuardEvent implements BaseTelemetryEvent { + 'event.name': 'repeated_tool_failure_guard'; + 'event.timestamp': string; + prompt_id: string; + route: 'acp_foreground'; + mode: RepeatedToolFailureGuardTelemetryMode; + phase_before: RepeatedToolFailureGuardTelemetryPhase; + phase_after: RepeatedToolFailureGuardTelemetryPhase; + decision: RepeatedToolFailureGuardTelemetryDecision; + failure_count_bucket: RepeatedToolFailureGuardCountBucket; + batch_count_bucket: RepeatedToolFailureGuardBatchBucket; + candidate_ordinal: number; + declare reset_reason?: RepeatedToolFailureGuardResetReason; + declare terminal_status?: 'error'; + declare execution_status?: 'error'; + declare execution_error_type?: ToolErrorType; + declare tool_type?: 'native' | 'mcp'; + + constructor( + params: Omit< + RepeatedToolFailureGuardEvent, + 'event.name' | 'event.timestamp' + >, + ) { + this['event.name'] = 'repeated_tool_failure_guard'; + this['event.timestamp'] = new Date().toISOString(); + this.prompt_id = params.prompt_id; + this.route = params.route; + this.mode = params.mode; + this.phase_before = params.phase_before; + this.phase_after = params.phase_after; + this.decision = params.decision; + this.failure_count_bucket = params.failure_count_bucket; + this.batch_count_bucket = params.batch_count_bucket; + this.candidate_ordinal = params.candidate_ordinal; + if (params.reset_reason !== undefined) { + this.reset_reason = params.reset_reason; + } + if (params.terminal_status !== undefined) { + this.terminal_status = params.terminal_status; + } + if (params.execution_status !== undefined) { + this.execution_status = params.execution_status; + } + if (params.execution_error_type !== undefined) { + this.execution_error_type = params.execution_error_type; + } + if (params.tool_type !== undefined) { + this.tool_type = params.tool_type; + } + } +} + export class LoopDetectionDisabledEvent implements BaseTelemetryEvent { 'event.name': 'loop_detection_disabled'; 'event.timestamp': string; @@ -1137,6 +1229,7 @@ export type TelemetryEvent = | FlashFallbackEvent | RipgrepRuntimeRecoveryEvent | LoopDetectedEvent + | RepeatedToolFailureGuardEvent | LoopDetectionDisabledEvent | NextSpeakerCheckEvent | KittySequenceOverflowEvent From 955a6923d458f3261b97b2b93e785777c0e7cc6a Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Tue, 4 Aug 2026 21:22:13 +0800 Subject: [PATCH 2/5] fix(acp): harden repeated tool failure guard rollout Co-authored-by: Qwen-Coder --- .../acp-repeated-tool-call-protection.md | 128 +++++--- docs/users/configuration/settings.md | 59 ++-- packages/channels/base/src/AcpBridge.test.ts | 10 +- packages/channels/base/src/AcpBridge.ts | 14 +- .../channels/base/src/ChannelAgentBridge.ts | 2 + .../base/src/DaemonChannelBridge.test.ts | 3 + .../channels/base/src/DaemonChannelBridge.ts | 24 +- .../acp-integration/session/Session.test.ts | 288 ++++++++++++++++-- .../src/acp-integration/session/Session.ts | 57 +++- .../session/repeated-tool-failure-guard.ts | 17 +- packages/cli/src/config/environment.test.ts | 9 + .../cli/src/config/shared-env-keys.test.ts | 11 +- packages/cli/src/config/shared-env-keys.ts | 5 + packages/core/src/telemetry/index.ts | 2 +- packages/core/src/telemetry/loggers.test.ts | 1 - packages/core/src/telemetry/loggers.ts | 15 +- packages/core/src/telemetry/metrics.test.ts | 3 - packages/core/src/telemetry/metrics.ts | 2 - 18 files changed, 493 insertions(+), 157 deletions(-) diff --git a/docs/design/acp-repeated-tool-call-protection.md b/docs/design/acp-repeated-tool-call-protection.md index d97e609f640..e0154052288 100644 --- a/docs/design/acp-repeated-tool-call-protection.md +++ b/docs/design/acp-repeated-tool-call-protection.md @@ -2,15 +2,15 @@ Date: 2026-07-31 Status: Implemented, pending shadow rollout; revised for PR #8176 and PR #8180 -Area: ACP foreground prompt loop +Area: Interactive ACP foreground prompt loop ## Summary ACP should stop an automatic model loop when the same resolved tool repeatedly reaches the same trusted execution failure. The protection is conservative: it observes only finalized, fully settled tool batches; gives the model one -fixed corrective reminder; and stops only if the next batch repeats the same -failure. +fixed corrective reminder per candidate streak; and stops only if the next +batch repeats the same failure. The first version is an in-memory, per-prompt semantic guard. It does not replace the existing protections for duplicate provider call IDs, invalid @@ -71,8 +71,8 @@ were recorded as errors are a concrete example. - Inferring retryability from free-form error text. - Persisting the guard across a daemon restart, rewind, branch, or fork. - Applying the guard to TUI, Stop-hook/Todo automatic continuations, cron, - notifications, subagent-internal loops, or third-party producers in the - first release. + notifications, channel-driven prompts, subagent-internal loops, or + third-party producers in the first release. - Automatically retrying a tool or suppressing an admitted call. Those are separate problems with different trust and durability boundaries. @@ -119,7 +119,7 @@ An eligible failure has: - a non-empty structured `executionErrorType` other than `ToolErrorType.UNKNOWN`; - a resolved built-in or MCP tool identity from the tool registry; and -- a final result produced by a fully settled ACP foreground batch. +- a final result produced by a fully settled interactive ACP foreground batch. The failure key is: @@ -142,7 +142,7 @@ with incomplete outcome fields are not eligible. ## State machine -The Session owns one guard per foreground ACP prompt: +The Session owns one guard per interactive foreground ACP prompt: ```ts type RepeatedToolFailureState = @@ -202,6 +202,9 @@ The state transition and control action are separate: Once latched, the guard emits no further decisions for that prompt. This avoids repeated reminders and telemetry amplification in shadow and warn modes. +Resetting a candidate and later tracking a different failure key may produce a +new reminder; the one-reminder guarantee is per candidate streak, not per +top-level prompt. The reminder is: @@ -277,9 +280,12 @@ under the existing Session lifecycle. ## Scope and enforcement modes -The first release applies only to the selected live Session owner processing a -foreground ACP prompt. It is not process-global and must not fall back to a -legacy or primary runtime when workspace ownership is unknown. +The first release applies only to the selected live Session owner processing an +interactive foreground ACP prompt. Both channel bridge implementations mark +their prompts explicitly, and Session forces those marked prompts to `off` +even when the process is configured for enforcement. It is not process-global +and must not fall back to a legacy or primary runtime when workspace ownership +is unknown. Modes: @@ -297,9 +303,20 @@ routes remain `off` in the first release. The mode is an operator-controlled deployment policy, not a user-facing setting. `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` selects `off`, `shadow`, `warn`, or `enforce` when the Session starts; missing or invalid values resolve -to `shadow`. The deployment control plane must set it only on the assigned -version-pinned cohort. This feature does not introduce a second rollout or -owner-assignment service. +to `shadow`, and a non-empty invalid value records an operator diagnostic. +Project `.env`, project `.qwen/.env`, and workspace `settings.env` sources are +not allowed to set this policy; an exported process value or a user-level +environment file remains valid. The deployment control plane must set it only +on the assigned version-pinned cohort. This feature does not introduce a +second rollout or owner-assignment service. + +The guard depends on the ACP host implementing `craft/drainMidTurnQueue` with a +boolean `hasQueuedPrompt` when the guard is enabled. Older or third-party hosts +that reject, time out, or return an incomplete drain response produce one +`unreliable_input` reset diagnostic, disable enforcement for that prompt, and +remain unable to accumulate a candidate until the input boundary is reliable. +This fail-open behavior is compatibility-safe but must be segmented from a +supported-host shadow baseline. ## Telemetry and privacy @@ -308,26 +325,29 @@ log per reducer transition: - deployment environment and service version from the existing OpenTelemetry resource rather than new guard labels; -- route: ACP foreground or other; +- route: interactive ACP foreground; - mode; - phase before and after; - decision: reset, tracked, would_warn, warned, would_stop, or stopped; -- candidate terminal status, execution status, frozen execution error type, - and tool type when the batch has one eligible key; +- candidate terminal status, execution status, and tool type when the batch + has one eligible key; the frozen execution error type is retained only in + the structured diagnostic log, not as a metric label; - otherwise only a low-cardinality reset reason such as `success`, `cancelled`, `not_started`, `unknown`, `mixed`, `incomplete`, `external_input`, or `contract_violation`; - failure count bucket: `0`, `1-2`, `3-4`, `5-7`, or `8+`; - batch count bucket: `0`, `1`, `2`, or `3+`; -- the existing prompt ID in the diagnostic log only, for checking transition - order; and +- a prompt-local guard correlation ID in the diagnostic log only, for checking + transition order; it is a one-way SHA-256 digest of the ACP prompt ID, so an + authorized investigation can hash a known trace prompt ID while the central + event does not disclose the prompt or session ID; and - a prompt-local candidate ordinal in the diagnostic log only. The reducer reuses the ordinal while the same private key is active and allocates a new one when the key changes. -Prompt ID and candidate ordinal are never metric labels. The ordinal cannot -correlate a tool across prompts and does not reveal its identity. An -`idle`-to-`idle` observation emits nothing. +The guard correlation ID and candidate ordinal are never metric labels. The +ordinal cannot correlate a tool across prompts and does not reveal its +identity. An `idle`-to-`idle` observation emits nothing. The terminal `repeated_tool_execution_failure` loop event uses the same privacy-restricted OpenTelemetry path and bypasses session-scoped RUM. Other @@ -375,6 +395,11 @@ Shadow mode advances a virtual warned state without injecting the reminder, so `would_warn` and `would_stop` estimate volume only. They cannot establish how a model behaves after seeing the reminder. +The default shadow mode adds `todoStopGuardWatchQueuedPrompt: true` to the +existing mid-turn drain request so the reducer can prove that no full prompt is +queued. Hosts without that response contract are counted through +`unreliable_input` and excluded from supported-host shadow conclusions. + Required invariants: - zero cancelled calls counted as eligible failures; @@ -394,7 +419,7 @@ enforcement. ### Phase 2: warn -Enable `warn` for internal ACP foreground prompts. Hold for seven days and +Enable `warn` for internal interactive ACP foreground prompts. Hold for seven days and confirm that reminder injection does not increase cancellation, reconnect, latency, token, or round-count regressions. Public cloud remains in shadow. Only warn-mode prompts show whether the model repeats the failure after the @@ -403,11 +428,12 @@ enforcement. ### Phase 3: limited enforcement -Enable enforcement for at most 5% of stable, version-pinned internal ACP -foreground owners. Assignment is deterministic by owner so one prompt cannot -switch treatment mid-run. The remaining 95% stays in `warn`, so both treatment -and control receive the same corrective reminder and differ only in whether the -post-reminder matching batch stops. Hold each wave for seven days. +Enable enforcement for at most 5% of stable, version-pinned internal +interactive ACP foreground owners. Assignment is deterministic by owner so one +prompt cannot switch treatment mid-run. The remaining 95% stays in `warn`, so +both treatment and control receive the same corrective reminder and differ only +in whether the post-reminder matching batch stops. Hold each wave for seven +days. Promote only if: @@ -444,6 +470,8 @@ Keep the change small: receipt, drain external input, and apply the returned action; - add the new loop type and telemetry event fields through the existing low-cardinality logging path; and +- mark channel prompts at both bridge boundaries and keep the rollout variable + out of project-controlled environment sources; and - avoid changing tool implementations or adding another execution scheduler. Because the change touches Core telemetry types and ACP Session orchestration, @@ -460,10 +488,9 @@ Suggested delivery sequence: ## Verification -Unit tests for the pure reducer cover: +Automated unit tests for the pure reducer cover: - the full terminal/execution decision table; -- terminal `errorType` cannot overwrite the frozen execution failure key; - eight failures in one batch do not warn; - eight failures across two batches warn; - the next matching batch stops only after it settles; @@ -475,31 +502,31 @@ Unit tests for the pure reducer cover: - warning and stop text are fixed and contain no tool data; and - unsupported outcome combinations never enforce. -ACP Session tests cover: +Automated ACP Session and channel tests cover: -- sequential and concurrent batches; -- preservation of all current-batch results before stop; -- no extra model stream after stop; -- Todo Stop Guard suspension; -- mid-turn and queued user-input precedence; -- duplicate call-ID, invalid-parameter, repeated-execution, and total-cap - ordering; -- Session reset, retry, cancellation, disconnect, history replay, and model - switch behavior; and -- shadow, warn, enforce, and downgrade-to-warn routes. +- the internal receipt preserves frozen execution failure type instead of a + later terminal error type; +- off keeps the legacy drain request shape; +- shadow observes without changing the turn; +- warn injects exactly one reminder for the candidate; +- enforce preserves the settled post-reminder result in history and opens no + extra model stream; +- queued input and cancellation take precedence over enforcement; +- unsupported hosts downgrade through `unreliable_input`; and +- both channel bridge paths mark their prompts and Session forces them off. Telemetry tests cover: -- normalization inherited from PR #8176 and PR #8180; -- cancellation exclusion; -- version-scoped baseline queries; - low-cardinality attributes; and -- redaction of arguments, results, paths, raw messages, and stable identity. +- exclusion of session-scoped common attributes and sensitive tool fields from + the guard and terminal loop events. -The behavioral change also needs an E2E plan under `.qwen/e2e-tests/` covering -one typed failing tool, permission cancellation, a successful recovery after -the reminder, a repeated failure that stops, concurrent siblings, reconnect, -and both internal and public-cloud policy modes. +The behavioral change also has an E2E plan under `.qwen/e2e-tests/`. Its manual +ACP fixture run remains required before promotion out of Draft. It covers one +typed failing tool, permission cancellation, successful recovery after the +reminder, a repeated failure that stops, concurrent siblings, unsupported +hosts, channel exclusion, reconnect and restart behavior, history replay, a +fresh prompt after stop, and both internal and public-cloud policy modes. Before delivery, run targeted Core and CLI Vitest files from their package directories, then `npm run build`, `npm run typecheck`, and `npm run lint`. @@ -508,8 +535,9 @@ directories, then `npm run build`, `npm run typecheck`, and `npm run lint`. - Telemetry emission failure never changes the tool or model control flow. - Missing or malformed outcome data resets and downgrades enforcement. -- Reminder injection failure resets the guard; it must not stop without having - delivered the reminder. +- If the model request carrying the reminder fails, that prompt exits before a + later stop decision; the guard never stops without first constructing and + sending the reminder turn. - Stop-history persistence failure returns the existing ACP internal error and does not pretend the stop was durably recorded. - A process restart loses the semantic streak by design. Existing history-based diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index 5d51ef2a0f6..42ea586be43 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -705,35 +705,36 @@ For authentication-related variables (like `OPENAI_*`) and the recommended `.qwe ### Environment Variables Table -| Variable | Description | Notes | -| ---------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `QWEN_HOME` | Customizes the global configuration directory (default: `~/.qwen`). Accepts an absolute or relative path (relative paths are resolved from the current working directory). Leading `~` is expanded to the user's home directory. | Stores credentials, settings, memory, skills, and other global state. When set, project-level `.qwen/` directories are unaffected. An empty string is treated as unset. | -| `QWEN_RUNTIME_DIR` | Overrides the runtime output directory (conversations, logs, todos). When unset, defaults to the `QWEN_HOME` directory. | Use this to separate ephemeral runtime data from persistent config. Useful when `QWEN_HOME` is on a shared/slow filesystem. | -| `QWEN_USAGE_STATISTICS_ENABLED` | Set to `true` or `1` to enable usage statistics. Any other value is treated as disabling them. | Overrides the `privacy.usageStatisticsEnabled` setting. Defaults to enabled when neither is configured. | -| `QWEN_TELEMETRY_ENABLED` | Set to `true` or `1` to enable telemetry. Any other value is treated as disabling it. | Overrides the `telemetry.enabled` setting. | -| `QWEN_TELEMETRY_TARGET` | Sets an informational label for the telemetry destination (`local` or `gcp`). Does not control routing; use `QWEN_TELEMETRY_OTLP_ENDPOINT` or `QWEN_TELEMETRY_OUTFILE` to configure where data is sent. | Overrides the `telemetry.target` setting. | -| `QWEN_TELEMETRY_OTLP_ENDPOINT` | Sets the OTLP endpoint for telemetry. | Overrides the `telemetry.otlpEndpoint` setting. | -| `QWEN_TELEMETRY_OTLP_PROTOCOL` | Sets the OTLP protocol (`grpc` or `http`). | Overrides the `telemetry.otlpProtocol` setting. | -| `QWEN_TELEMETRY_LOG_PROMPTS` | Set to `true` or `1` to enable or disable logging of user prompts. Any other value is treated as disabling it. | Overrides the `telemetry.logPrompts` setting. | -| `QWEN_TELEMETRY_USER_ID` | Sets a stable end-user identifier on interaction, LLM, Tool, and Agent spans as `gen_ai.user.id`. Prefer a pseudonymous value. | Overrides `telemetry.userId` after trimming. A blank value falls back to settings. This is process-wide and must not be used as per-request identity in a shared multi-user process. | -| `QWEN_TELEMETRY_INCLUDE_SENSITIVE_SPAN_ATTRIBUTES` | Set to `true` or `1` to attach verbatim user prompts, system prompts, tool I/O, and model responses to native OTel span attributes (and keep `prompt` / `function_args` / `response_text` on log-to-span bridge spans). Any other value disables it. | Overrides the `telemetry.includeSensitiveSpanAttributes` setting. ⚠️ Streams sensitive data to your OTLP backend. | -| `QWEN_TELEMETRY_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH` | Sets the maximum JavaScript string length for each sensitive native OTel span attribute content payload. Must be a positive integer no greater than `104857600` (100 MiB). | Overrides the `telemetry.sensitiveSpanAttributeMaxLength` setting. Default is `1048576` (1 MiB); lower it if your collector or backend rejects large span attributes. | -| `QWEN_TELEMETRY_OUTFILE` | Sets the file path to write telemetry to. When set, overrides OTLP export. | Overrides the `telemetry.outfile` setting. | -| `QWEN_SANDBOX` | Alternative to the `sandbox` setting in `settings.json`. | Accepts `true`, `false`, `docker`, `podman`, or a custom command string. | -| `QWEN_SANDBOX_IMAGE` | Overrides sandbox image selection for Docker/Podman. | Takes precedence over `tools.sandboxImage`. | -| `SEATBELT_PROFILE` | (macOS specific) Switches the Seatbelt (`sandbox-exec`) profile on macOS. | `permissive-open`: (Default) Restricts writes to the project folder (and a few other folders, see `packages/cli/src/utils/sandbox-macos-permissive-open.sb`) but allows other operations. `strict`: Uses a strict profile that declines operations by default. ``: Uses a custom profile. To define a custom profile, create a file named `sandbox-macos-.sb` in your project's `.qwen/` directory (e.g., `my-project/.qwen/sandbox-macos-custom.sb`). | -| `DEBUG` or `DEBUG_MODE` | (often used by underlying libraries or the CLI itself) Set to `true` or `1` to enable verbose debug logging, which can be helpful for troubleshooting. | **Note:** These variables are automatically excluded from project `.env` files by default to prevent interference with the CLI behavior. Use `.qwen/.env` files if you need to set these for Qwen Code specifically. | -| `NO_COLOR` | Set to any value to disable all color output in the CLI. | | -| `FORCE_HYPERLINK` | Override the OSC 8 clickable-link detection in the markdown renderer. Set to `1` (or any non-zero integer, or empty string) to force-enable; set to `0` or a non-numeric value such as `false` / `off` to force-disable. Honors `NO_COLOR` / `QWEN_DISABLE_HYPERLINKS` opt-outs above it. | Use this to opt into OSC 8 inside `tmux` / GNU `screen` (auto-detection refuses by default because the host terminal's capabilities are hidden behind the multiplexer). Requires `set -g allow-passthrough on` on tmux 3.3+. Also enables Hyper, which isn't auto-detected. | -| `QWEN_DISABLE_HYPERLINKS` | Set to `1` to hard-disable OSC 8 clickable hyperlinks in the markdown renderer even on terminals that auto-detect as capable. | Useful when a terminal advertises support but breaks on long URLs, or when piping output through an intermediary that mangles escape sequences. The renderer falls back to plain `label (url)` rendering. | -| `CLI_TITLE` | Set to a string to customize the title of the CLI. | | -| `CODE_ASSIST_ENDPOINT` | Specifies the endpoint for the code assist server. | This is useful for development and testing. | -| `QWEN_CODE_MAX_OUTPUT_TOKENS` | Overrides the default maximum output tokens per response. When not set, Qwen Code defaults to the model's declared output limit and, if a response is truncated, automatically escalates (64K floor) and recovers across turns. Set this to a specific value (e.g., `16000`) to use a fixed limit instead — useful for capacity-constrained self-hosted backends that want a lower per-request slot reservation. | Takes precedence over the model-limit default but is overridden by `samplingParams.max_tokens` in settings. Disables automatic escalation when set. Example: `export QWEN_CODE_MAX_OUTPUT_TOKENS=16000` | -| `QWEN_CODE_UNATTENDED_RETRY` | Set to `true` or `1` to enable persistent retry mode. When enabled, transient API capacity errors (HTTP 429 Rate Limit and 529 Overloaded) are retried indefinitely with exponential backoff (capped at 5 minutes per retry) and heartbeat keepalives every 30 seconds on stderr. | Designed for CI/CD pipelines and background automation where long-running tasks should survive temporary API outages. Must be set explicitly — `CI=true` alone does **not** activate this mode. See [Headless Mode](../features/headless#persistent-retry-mode) for details. Example: `export QWEN_CODE_UNATTENDED_RETRY=1` | -| `QWEN_CODE_PROFILE_STARTUP` | Set to `1` to enable startup performance profiling. Writes a JSON timing report to `~/.qwen/startup-perf/` with per-phase durations. | Only active inside the sandbox child process (or with `QWEN_CODE_PROFILE_STARTUP_OUTER=1`). Zero overhead when not set. Example: `export QWEN_CODE_PROFILE_STARTUP=1` | -| `QWEN_CODE_PROFILE_STARTUP_OUTER` | Set to `1` together with `QWEN_CODE_PROFILE_STARTUP=1` to also collect a startup profile in the outer (pre-sandbox) process. Outer-process reports get an `outer-` filename prefix to keep them distinct from the sandbox child's report. | Off by default — only the sandbox child collects, to avoid duplicate reports. Useful for local development where the cli isn't relaunched into a sandbox. | -| `QWEN_CODE_PROFILE_STARTUP_NO_HEAP` | Set to `1` together with `QWEN_CODE_PROFILE_STARTUP=1` to skip the per-checkpoint `process.memoryUsage()` snapshots. Useful when measuring the profiler's own Heisenberg overhead. | Off by default. Heap snapshots cost ~50 µs each (well below 1% of total startup) so most users should leave this alone. | -| `QWEN_CODE_LEGACY_MCP_BLOCKING` | Set to `1` to restore the pre-progressive-MCP behavior where `Config.initialize()` waits synchronously for every configured MCP server's discover handshake before returning. | Off by default. Modern qwen-code lets MCP servers come online in the background while the UI is already interactive; the model sees each batch of new tools within ~16 ms of the server settling. This flag is kept as a rollback escape hatch for ≥ 1 release. Example: `export QWEN_CODE_LEGACY_MCP_BLOCKING=1` | +| Variable | Description | Notes | +| ---------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `QWEN_HOME` | Customizes the global configuration directory (default: `~/.qwen`). Accepts an absolute or relative path (relative paths are resolved from the current working directory). Leading `~` is expanded to the user's home directory. | Stores credentials, settings, memory, skills, and other global state. When set, project-level `.qwen/` directories are unaffected. An empty string is treated as unset. | +| `QWEN_RUNTIME_DIR` | Overrides the runtime output directory (conversations, logs, todos). When unset, defaults to the `QWEN_HOME` directory. | Use this to separate ephemeral runtime data from persistent config. Useful when `QWEN_HOME` is on a shared/slow filesystem. | +| `QWEN_USAGE_STATISTICS_ENABLED` | Set to `true` or `1` to enable usage statistics. Any other value is treated as disabling them. | Overrides the `privacy.usageStatisticsEnabled` setting. Defaults to enabled when neither is configured. | +| `QWEN_TELEMETRY_ENABLED` | Set to `true` or `1` to enable telemetry. Any other value is treated as disabling it. | Overrides the `telemetry.enabled` setting. | +| `QWEN_TELEMETRY_TARGET` | Sets an informational label for the telemetry destination (`local` or `gcp`). Does not control routing; use `QWEN_TELEMETRY_OTLP_ENDPOINT` or `QWEN_TELEMETRY_OUTFILE` to configure where data is sent. | Overrides the `telemetry.target` setting. | +| `QWEN_TELEMETRY_OTLP_ENDPOINT` | Sets the OTLP endpoint for telemetry. | Overrides the `telemetry.otlpEndpoint` setting. | +| `QWEN_TELEMETRY_OTLP_PROTOCOL` | Sets the OTLP protocol (`grpc` or `http`). | Overrides the `telemetry.otlpProtocol` setting. | +| `QWEN_TELEMETRY_LOG_PROMPTS` | Set to `true` or `1` to enable or disable logging of user prompts. Any other value is treated as disabling it. | Overrides the `telemetry.logPrompts` setting. | +| `QWEN_TELEMETRY_USER_ID` | Sets a stable end-user identifier on interaction, LLM, Tool, and Agent spans as `gen_ai.user.id`. Prefer a pseudonymous value. | Overrides `telemetry.userId` after trimming. A blank value falls back to settings. This is process-wide and must not be used as per-request identity in a shared multi-user process. | +| `QWEN_TELEMETRY_INCLUDE_SENSITIVE_SPAN_ATTRIBUTES` | Set to `true` or `1` to attach verbatim user prompts, system prompts, tool I/O, and model responses to native OTel span attributes (and keep `prompt` / `function_args` / `response_text` on log-to-span bridge spans). Any other value disables it. | Overrides the `telemetry.includeSensitiveSpanAttributes` setting. ⚠️ Streams sensitive data to your OTLP backend. | +| `QWEN_TELEMETRY_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH` | Sets the maximum JavaScript string length for each sensitive native OTel span attribute content payload. Must be a positive integer no greater than `104857600` (100 MiB). | Overrides the `telemetry.sensitiveSpanAttributeMaxLength` setting. Default is `1048576` (1 MiB); lower it if your collector or backend rejects large span attributes. | +| `QWEN_TELEMETRY_OUTFILE` | Sets the file path to write telemetry to. When set, overrides OTLP export. | Overrides the `telemetry.outfile` setting. | +| `QWEN_SANDBOX` | Alternative to the `sandbox` setting in `settings.json`. | Accepts `true`, `false`, `docker`, `podman`, or a custom command string. | +| `QWEN_SANDBOX_IMAGE` | Overrides sandbox image selection for Docker/Podman. | Takes precedence over `tools.sandboxImage`. | +| `SEATBELT_PROFILE` | (macOS specific) Switches the Seatbelt (`sandbox-exec`) profile on macOS. | `permissive-open`: (Default) Restricts writes to the project folder (and a few other folders, see `packages/cli/src/utils/sandbox-macos-permissive-open.sb`) but allows other operations. `strict`: Uses a strict profile that declines operations by default. ``: Uses a custom profile. To define a custom profile, create a file named `sandbox-macos-.sb` in your project's `.qwen/` directory (e.g., `my-project/.qwen/sandbox-macos-custom.sb`). | +| `DEBUG` or `DEBUG_MODE` | (often used by underlying libraries or the CLI itself) Set to `true` or `1` to enable verbose debug logging, which can be helpful for troubleshooting. | **Note:** These variables are automatically excluded from project `.env` files by default to prevent interference with the CLI behavior. Use `.qwen/.env` files if you need to set these for Qwen Code specifically. | +| `NO_COLOR` | Set to any value to disable all color output in the CLI. | | +| `FORCE_HYPERLINK` | Override the OSC 8 clickable-link detection in the markdown renderer. Set to `1` (or any non-zero integer, or empty string) to force-enable; set to `0` or a non-numeric value such as `false` / `off` to force-disable. Honors `NO_COLOR` / `QWEN_DISABLE_HYPERLINKS` opt-outs above it. | Use this to opt into OSC 8 inside `tmux` / GNU `screen` (auto-detection refuses by default because the host terminal's capabilities are hidden behind the multiplexer). Requires `set -g allow-passthrough on` on tmux 3.3+. Also enables Hyper, which isn't auto-detected. | +| `QWEN_DISABLE_HYPERLINKS` | Set to `1` to hard-disable OSC 8 clickable hyperlinks in the markdown renderer even on terminals that auto-detect as capable. | Useful when a terminal advertises support but breaks on long URLs, or when piping output through an intermediary that mangles escape sequences. The renderer falls back to plain `label (url)` rendering. | +| `CLI_TITLE` | Set to a string to customize the title of the CLI. | | +| `CODE_ASSIST_ENDPOINT` | Specifies the endpoint for the code assist server. | This is useful for development and testing. | +| `QWEN_CODE_MAX_OUTPUT_TOKENS` | Overrides the default maximum output tokens per response. When not set, Qwen Code defaults to the model's declared output limit and, if a response is truncated, automatically escalates (64K floor) and recovers across turns. Set this to a specific value (e.g., `16000`) to use a fixed limit instead — useful for capacity-constrained self-hosted backends that want a lower per-request slot reservation. | Takes precedence over the model-limit default but is overridden by `samplingParams.max_tokens` in settings. Disables automatic escalation when set. Example: `export QWEN_CODE_MAX_OUTPUT_TOKENS=16000` | +| `QWEN_CODE_UNATTENDED_RETRY` | Set to `true` or `1` to enable persistent retry mode. When enabled, transient API capacity errors (HTTP 429 Rate Limit and 529 Overloaded) are retried indefinitely with exponential backoff (capped at 5 minutes per retry) and heartbeat keepalives every 30 seconds on stderr. | Designed for CI/CD pipelines and background automation where long-running tasks should survive temporary API outages. Must be set explicitly — `CI=true` alone does **not** activate this mode. See [Headless Mode](../features/headless#persistent-retry-mode) for details. Example: `export QWEN_CODE_UNATTENDED_RETRY=1` | +| `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` | Operator rollout mode for ACP repeated tool-execution failure protection. Accepts `off`, `shadow`, `warn`, or `enforce`; missing or invalid values default to `shadow`. | Applies only to interactive foreground ACP prompts; channel-driven and other automatic routes remain off. Non-empty invalid values produce a diagnostic. Because this is operator policy, project `.env`, project `.qwen/.env`, and workspace `settings.env` cannot set it; export it in the process environment or use a user-level environment file. Shadow, warn, and enforce require the ACP host's `craft/drainMidTurnQueue` response to include reliable queued-prompt state. | +| `QWEN_CODE_PROFILE_STARTUP` | Set to `1` to enable startup performance profiling. Writes a JSON timing report to `~/.qwen/startup-perf/` with per-phase durations. | Only active inside the sandbox child process (or with `QWEN_CODE_PROFILE_STARTUP_OUTER=1`). Zero overhead when not set. Example: `export QWEN_CODE_PROFILE_STARTUP=1` | +| `QWEN_CODE_PROFILE_STARTUP_OUTER` | Set to `1` together with `QWEN_CODE_PROFILE_STARTUP=1` to also collect a startup profile in the outer (pre-sandbox) process. Outer-process reports get an `outer-` filename prefix to keep them distinct from the sandbox child's report. | Off by default — only the sandbox child collects, to avoid duplicate reports. Useful for local development where the cli isn't relaunched into a sandbox. | +| `QWEN_CODE_PROFILE_STARTUP_NO_HEAP` | Set to `1` together with `QWEN_CODE_PROFILE_STARTUP=1` to skip the per-checkpoint `process.memoryUsage()` snapshots. Useful when measuring the profiler's own Heisenberg overhead. | Off by default. Heap snapshots cost ~50 µs each (well below 1% of total startup) so most users should leave this alone. | +| `QWEN_CODE_LEGACY_MCP_BLOCKING` | Set to `1` to restore the pre-progressive-MCP behavior where `Config.initialize()` waits synchronously for every configured MCP server's discover handshake before returning. | Off by default. Modern qwen-code lets MCP servers come online in the background while the UI is already interactive; the model sees each batch of new tools within ~16 ms of the server settling. This flag is kept as a rollback escape hatch for ≥ 1 release. Example: `export QWEN_CODE_LEGACY_MCP_BLOCKING=1` | When both user-level `.env` files define the same variable, the Qwen-specific file wins: `/.env` (or `~/.qwen/.env` when `QWEN_HOME` is unset) is diff --git a/packages/channels/base/src/AcpBridge.test.ts b/packages/channels/base/src/AcpBridge.test.ts index 9e33aca4002..94ea4475e2d 100644 --- a/packages/channels/base/src/AcpBridge.test.ts +++ b/packages/channels/base/src/AcpBridge.test.ts @@ -7,7 +7,10 @@ import { AcpBridge, } from './AcpBridge.js'; import { CHANNEL_LOOP_MCP_SERVER_NAME } from './ChannelLoopTools.js'; -import type { ChannelLoopToolHandler } from './ChannelAgentBridge.js'; +import { + CHANNEL_PROMPT_META_KEY, + type ChannelLoopToolHandler, +} from './ChannelAgentBridge.js'; const child = vi.hoisted(() => { class MockEmitter { @@ -430,6 +433,11 @@ describe('AcpBridge', () => { await expect(bridge.prompt('s-1', 'question')).resolves.toBe( 'Final answer.', ); + expect(bridge.connection.prompt).toHaveBeenCalledWith({ + sessionId: 's-1', + prompt: [{ type: 'text', text: 'question' }], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }); }); it('excludes nested subagent text from the final response', async () => { diff --git a/packages/channels/base/src/AcpBridge.ts b/packages/channels/base/src/AcpBridge.ts index 881d0d905f5..4abb896233a 100644 --- a/packages/channels/base/src/AcpBridge.ts +++ b/packages/channels/base/src/AcpBridge.ts @@ -14,12 +14,13 @@ import type { RequestPermissionRequest, RequestPermissionResponse, } from '@agentclientprotocol/sdk'; -import type { - AvailableCommand, - ChannelAgentBridge, - ChannelAgentBridgeSessionOptions, - ChannelLoopToolHandler, - ToolCallEvent, +import { + CHANNEL_PROMPT_META_KEY, + type AvailableCommand, + type ChannelAgentBridge, + type ChannelAgentBridgeSessionOptions, + type ChannelLoopToolHandler, + type ToolCallEvent, } from './ChannelAgentBridge.js'; import { CHANNEL_LOOP_MCP_SERVER_NAME, @@ -280,6 +281,7 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { await conn.prompt({ sessionId, prompt: prompt as Array<{ type: 'text'; text: string }>, + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, }); } finally { this.off('textChunk', onChunk); diff --git a/packages/channels/base/src/ChannelAgentBridge.ts b/packages/channels/base/src/ChannelAgentBridge.ts index 4f6d166f6c7..f5b5464e94c 100644 --- a/packages/channels/base/src/ChannelAgentBridge.ts +++ b/packages/channels/base/src/ChannelAgentBridge.ts @@ -3,6 +3,8 @@ import type { RequestPermissionResponse, } from '@agentclientprotocol/sdk'; +export const CHANNEL_PROMPT_META_KEY = 'qwen.channel.prompt'; + export interface AvailableCommand { name: string; description: string; diff --git a/packages/channels/base/src/DaemonChannelBridge.test.ts b/packages/channels/base/src/DaemonChannelBridge.test.ts index e4a3122a9c8..8335baf17c5 100644 --- a/packages/channels/base/src/DaemonChannelBridge.test.ts +++ b/packages/channels/base/src/DaemonChannelBridge.test.ts @@ -9,6 +9,7 @@ import { type DaemonChannelLoopMcpHost, type DaemonChannelSessionClient, } from './DaemonChannelBridge.js'; +import { CHANNEL_PROMPT_META_KEY } from './ChannelAgentBridge.js'; class EventQueue implements AsyncGenerator { private events: DaemonChannelEvent[] = []; @@ -243,6 +244,7 @@ describe('DaemonChannelBridge', () => { expect(session.prompt).toHaveBeenCalledWith( { prompt: [{ type: 'text', text: 'summarize' }], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, }, expect.any(AbortSignal), ); @@ -1849,6 +1851,7 @@ describe('DaemonChannelBridge', () => { { type: 'image', data: 'base64-image', mimeType: 'image/png' }, { type: 'text', text: 'describe' }, ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, }, expect.any(AbortSignal), ); diff --git a/packages/channels/base/src/DaemonChannelBridge.ts b/packages/channels/base/src/DaemonChannelBridge.ts index afcd6c8606a..806165fad99 100644 --- a/packages/channels/base/src/DaemonChannelBridge.ts +++ b/packages/channels/base/src/DaemonChannelBridge.ts @@ -3,13 +3,14 @@ import type { RequestPermissionRequest, RequestPermissionResponse, } from '@agentclientprotocol/sdk'; -import type { - AvailableCommand, - BridgeSessionInfo, - ChannelAgentBridge, - ChannelAgentBridgeSessionOptions, - ChannelLoopToolHandler, - ToolCallEvent, +import { + CHANNEL_PROMPT_META_KEY, + type AvailableCommand, + type BridgeSessionInfo, + type ChannelAgentBridge, + type ChannelAgentBridgeSessionOptions, + type ChannelLoopToolHandler, + type ToolCallEvent, } from './ChannelAgentBridge.js'; import { readAvailableCommandAltNames } from './AcpBridge.js'; import { @@ -35,6 +36,7 @@ export interface DaemonChannelSessionClient { prompt( req: { prompt: Array>; + _meta?: Record; }, signal?: AbortSignal, ): Promise<{ stopReason?: string; [key: string]: unknown }>; @@ -404,7 +406,13 @@ export class DaemonChannelBridge prompt.push({ type: 'text', text }); try { - const result = await session.prompt({ prompt }, controller.signal); + const result = await session.prompt( + { + prompt, + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + controller.signal, + ); // Prefer turn_complete for deterministic chunk collection (SSE path). // Fall back to one event-loop tick for non-SSE prompt paths (blocking // HTTP, non-202 responses) where turn_complete never arrives. diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index c3e615e3ee5..889e3293e33 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -6,6 +6,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { EventEmitter } from 'node:events'; +import { createHash } from 'node:crypto'; import * as fsSync from 'node:fs'; import * as fs from 'node:fs/promises'; import * as os from 'node:os'; @@ -60,6 +61,7 @@ const startToolSpanSpy = vi.hoisted(() => vi.fn()); const addToolArgumentsAttributesSpy = vi.hoisted(() => vi.fn()); const addToolCallResultAttributesSpy = vi.hoisted(() => vi.fn()); const logLoopDetectedSpy = vi.hoisted(() => vi.fn()); +const logRepeatedToolFailureGuardSpy = vi.hoisted(() => vi.fn()); const TODO_STOP_GUARD_CONTINUATION_CLAIM_METHOD = 'craft/claimTodoStopGuardContinuation'; // Records every LoopTickResolver construction's deps so a test can assert what @@ -102,6 +104,12 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { logLoopDetectedSpy(...args); return actual.logLoopDetected(...args); }, + logRepeatedToolFailureGuard: ( + ...args: Parameters + ) => { + logRepeatedToolFailureGuardSpy(...args); + return actual.logRepeatedToolFailureGuard(...args); + }, // Transparent recording wrapper: records the constructor deps, then behaves // exactly like the real resolver (subclass → instanceof + methods preserved). LoopTickResolver: class extends actual.LoopTickResolver { @@ -504,6 +512,7 @@ describe('Session', () => { addToolArgumentsAttributesSpy.mockClear(); addToolCallResultAttributesSpy.mockClear(); logLoopDetectedSpy.mockReset(); + logRepeatedToolFailureGuardSpy.mockReset(); runVisionBridgeSpy.mockReset(); bridgeToolResultImagesSpy.mockReset(); bridgeToolResultImagesSpy.mockImplementation( @@ -6197,7 +6206,9 @@ describe('Session', () => { const guardModeEnv = 'QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'; let originalGuardMode: string | undefined; - const recreateSessionWithGuardMode = (mode: 'warn' | 'enforce') => { + const recreateSessionWithGuardMode = ( + mode: 'off' | 'shadow' | 'warn' | 'enforce', + ) => { originalGuardMode = process.env[guardModeEnv]; process.env[guardModeEnv] = mode; session = new Session( @@ -6260,17 +6271,215 @@ describe('Session', () => { return execute; }; - it('injects one corrective reminder in warn mode and keeps running', async () => { - recreateSessionWithGuardMode('warn'); + const sentText = () => + vi + .mocked(mockChat.sendMessageStream) + .mock.calls.flatMap(([, request]) => + (request as { message: Part[] }).message + .map((part) => part.text) + .filter((text): text is string => text !== undefined), + ); + + const queueMatchingFailureStreak = () => { + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValueOnce(streamForBatch(1, 4)) + .mockResolvedValueOnce(streamForBatch(2, 4)) + .mockResolvedValueOnce(streamForBatch(3, 1)) + .mockResolvedValueOnce(createEmptyStream()); + }; + + it('defaults an invalid operator mode to shadow and records a warning', () => { + debugLoggerWarnSpy.mockClear(); + const previous = process.env[guardModeEnv]; + process.env[guardModeEnv] = 'invalid-mode'; + try { + session = new Session( + 'test-session-id', + mockConfig, + mockClient, + mockSettings, + ); + + expect( + ( + session as unknown as { + repeatedToolFailureGuardMode: string; + } + ).repeatedToolFailureGuardMode, + ).toBe('shadow'); + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('has an invalid value'), + ); + } finally { + if (previous === undefined) delete process.env[guardModeEnv]; + else process.env[guardModeEnv] = previous; + } + }); + + it('observes would-stop behavior in shadow mode without changing the turn', async () => { + recreateSessionWithGuardMode('shadow'); try { const execute = installFailingTool(); + queueMatchingFailureStreak(); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'end_turn' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); + expect( + sentText().filter((text) => + text.includes('Do not repeat the same approach'), + ), + ).toHaveLength(0); + expect(logLoopDetectedSpy).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + }), + ); + } finally { + restoreGuardMode(); + } + }); + + it('keeps the pre-guard drain payload when the mode is off', async () => { + recreateSessionWithGuardMode('off'); + try { + installFailingTool(); mockChat.sendMessageStream = vi .fn() - .mockResolvedValueOnce(streamForBatch(1, 4)) - .mockResolvedValueOnce(streamForBatch(2, 4)) - .mockResolvedValueOnce(streamForBatch(3, 1)) + .mockResolvedValueOnce(streamForBatch(1, 1)) .mockResolvedValueOnce(createEmptyStream()); + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run once' }], + }); + + expect(mockClient.extMethod).toHaveBeenCalledWith( + 'craft/drainMidTurnQueue', + { sessionId: 'test-session-id' }, + ); + } finally { + restoreGuardMode(); + } + }); + + it('forces channel-routed prompts off even when enforcement is configured', async () => { + recreateSessionWithGuardMode('enforce'); + try { + const execute = installFailingTool(); + queueMatchingFailureStreak(); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'channel task' }], + _meta: { 'qwen.channel.prompt': true }, + }), + ).resolves.toEqual({ stopReason: 'end_turn' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); + expect(sentText()).not.toContainEqual( + expect.stringContaining('Do not repeat the same approach'), + ); + const drainCalls = vi + .mocked(mockClient.extMethod) + .mock.calls.filter( + ([method]) => method === 'craft/drainMidTurnQueue', + ); + expect(drainCalls).not.toContainEqual([ + 'craft/drainMidTurnQueue', + expect.objectContaining({ todoStopGuardWatchQueuedPrompt: true }), + ]); + expect(logRepeatedToolFailureGuardSpy).not.toHaveBeenCalled(); + } finally { + restoreGuardMode(); + } + }); + + it('fails open when the ACP host cannot provide reliable input state', async () => { + recreateSessionWithGuardMode('enforce'); + try { + const execute = installFailingTool(); + mockClient.extMethod = vi.fn().mockRejectedValue({ + code: -32601, + message: 'Method not found', + }); + queueMatchingFailureStreak(); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'end_turn' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); + expect(mockClient.extMethod).toHaveBeenCalledTimes(1); + expect( + logRepeatedToolFailureGuardSpy.mock.calls.some( + ([, event]) => event.reset_reason === 'unreliable_input', + ), + ).toBe(true); + expect(logLoopDetectedSpy).not.toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ + loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + }), + ); + } finally { + restoreGuardMode(); + } + }); + + it('resets the streak when the host reports a queued prompt', async () => { + recreateSessionWithGuardMode('enforce'); + try { + const execute = installFailingTool(); + let drainCount = 0; + mockClient.extMethod = vi.fn().mockImplementation(async () => ({ + messages: [], + hasQueuedPrompt: ++drainCount === 2, + })); + queueMatchingFailureStreak(); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'end_turn' }); + + expect(execute).toHaveBeenCalledTimes(9); + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); + expect( + logRepeatedToolFailureGuardSpy.mock.calls.some( + ([, event]) => event.reset_reason === 'queued_prompt', + ), + ).toBe(true); + expect(sentText()).not.toContainEqual( + expect.stringContaining('Do not repeat the same approach'), + ); + } finally { + restoreGuardMode(); + } + }); + + it('injects one corrective reminder in warn mode and keeps running', async () => { + recreateSessionWithGuardMode('warn'); + try { + const execute = installFailingTool(); + queueMatchingFailureStreak(); + await expect( session.prompt({ sessionId: 'test-session-id', @@ -6287,15 +6496,8 @@ describe('Session', () => { text: expect.stringContaining('Do not repeat the same approach'), }), ); - const allSentText = vi - .mocked(mockChat.sendMessageStream) - .mock.calls.flatMap(([, request]) => - (request as { message: Part[] }).message - .map((part) => part.text) - .filter((text): text is string => text !== undefined), - ); expect( - allSentText.filter((text) => + sentText().filter((text) => text.includes('Do not repeat the same approach'), ), ).toHaveLength(1); @@ -6314,12 +6516,7 @@ describe('Session', () => { recreateSessionWithGuardMode('enforce'); try { const execute = installFailingTool(); - mockChat.sendMessageStream = vi - .fn() - .mockResolvedValueOnce(streamForBatch(1, 4)) - .mockResolvedValueOnce(streamForBatch(2, 4)) - .mockResolvedValueOnce(streamForBatch(3, 1)) - .mockResolvedValueOnce(createEmptyStream()); + queueMatchingFailureStreak(); await expect( session.prompt({ @@ -6336,6 +6533,20 @@ describe('Session', () => { loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, }), ); + const telemetryPromptIds = + logRepeatedToolFailureGuardSpy.mock.calls.map( + ([, event]) => event.prompt_id, + ); + expect(new Set(telemetryPromptIds).size).toBe(1); + expect(telemetryPromptIds[0]).toBe( + createHash('sha256') + .update('test-session-id########1') + .digest('hex'), + ); + expect(logLoopDetectedSpy).toHaveBeenCalledWith( + mockConfig, + expect.objectContaining({ prompt_id: telemetryPromptIds[0] }), + ); expect(mockChat.addHistory).toHaveBeenCalledWith({ role: 'user', parts: expect.arrayContaining([ @@ -6380,12 +6591,7 @@ describe('Session', () => { } return { messages: [], hasQueuedPrompt: false }; }); - mockChat.sendMessageStream = vi - .fn() - .mockResolvedValueOnce(streamForBatch(1, 4)) - .mockResolvedValueOnce(streamForBatch(2, 4)) - .mockResolvedValueOnce(streamForBatch(3, 1)) - .mockResolvedValueOnce(createEmptyStream()); + queueMatchingFailureStreak(); await expect( session.prompt({ @@ -6406,6 +6612,36 @@ describe('Session', () => { restoreGuardMode(); } }); + + it('returns cancelled when cancellation arrives while the stop message is emitted', async () => { + recreateSessionWithGuardMode('enforce'); + try { + installFailingTool(); + vi.mocked(mockClient.sessionUpdate).mockImplementation( + async ({ update }) => { + if ( + update.sessionUpdate === 'agent_message_chunk' && + update.content.type === 'text' && + update.content.text.includes('Automatic continuation stopped') + ) { + await session.cancelPendingPrompt(); + } + }, + ); + queueMatchingFailureStreak(); + + await expect( + session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run the failing tool' }], + }), + ).resolves.toEqual({ stopReason: 'cancelled' }); + + expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(3); + } finally { + restoreGuardMode(); + } + }); }); describe('shell heartbeat forwarding', () => { diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index f95b8425708..ec5b5ef9101 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -5,7 +5,7 @@ */ import { Buffer } from 'node:buffer'; -import { randomUUID } from 'node:crypto'; +import { createHash, randomUUID } from 'node:crypto'; import { realpathSync, statSync } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -170,6 +170,7 @@ import { runWithInvocationContext, } from '@qwen-code/qwen-code-core'; import { NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE } from '@qwen-code/acp-bridge/bridgeErrors'; +import { ENV_ACP_REPEATED_TOOL_FAILURE_GUARD } from '../../config/shared-env-keys.js'; // Single source of truth shared with the daemon-side answerer (BridgeClient), // so a rename can't desync caller and answerer into a silent -32601 latch. import { @@ -288,7 +289,7 @@ import { reduceRepeatedToolFailureGuard, REPEATED_TOOL_FAILURE_REMINDER, REPEATED_TOOL_FAILURE_STOP_MESSAGE, - resolveRepeatedToolFailureGuardMode, + parseRepeatedToolFailureGuardMode, type RepeatedToolFailureBatch, type RepeatedToolFailureGuardMode, type RepeatedToolFailureGuardDecision, @@ -304,8 +305,7 @@ const USER_CANCEL_ABORT_REASON = 'qwen:user-cancel'; const SESSION_DISPOSE_ABORT_REASON = 'qwen:session-dispose'; const DAEMON_RETRY_META_KEY = 'qwen.daemon.retry'; const DAEMON_CONTINUE_META_KEY = 'qwen.daemon.continueLastTurn'; -const REPEATED_TOOL_FAILURE_GUARD_MODE_ENV = - 'QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'; +const CHANNEL_PROMPT_META_KEY = 'qwen.channel.prompt'; const TODO_STOP_GUARD_PROMPT_PREFIX = '[Todo Stop Guard] '; const TODO_STOP_GUARD_PROMPT_BODY_SUFFIX = ' todo item(s) are still pending or in progress. Continue executing the current task now. Do not ask the user whether to continue. If progress requires user input, use the structured question or permission flow. If progress depends on external state, report the blocker explicitly.'; @@ -460,6 +460,7 @@ type DaemonToolLoopState = { loopDetected: boolean; repeatedToolFailureMode: RepeatedToolFailureGuardMode; repeatedToolFailureState: RepeatedToolFailureGuardState; + repeatedToolFailureTelemetryId?: string; }; const DAEMON_INVALID_TOOL_PARAMS_THRESHOLD = 3; @@ -476,6 +477,7 @@ const TOOL_POST_EXECUTION_CANCELLED_MESSAGE = function createDaemonToolLoopState( repeatedToolFailureMode: RepeatedToolFailureGuardMode = 'off', + telemetrySourceId?: string, ): DaemonToolLoopState { return { totalToolCalls: 0, @@ -483,6 +485,13 @@ function createDaemonToolLoopState( loopDetected: false, repeatedToolFailureMode, repeatedToolFailureState: createRepeatedToolFailureGuardState(), + ...(repeatedToolFailureMode === 'off' || telemetrySourceId === undefined + ? {} + : { + repeatedToolFailureTelemetryId: createHash('sha256') + .update(telemetrySourceId) + .digest('hex'), + }), }; } @@ -505,13 +514,19 @@ function repeatedToolFailureBatchBucket(count: number): '0' | '1' | '2' | '3+' { function recordRepeatedToolFailureDecision( config: Config, - promptId: string, + telemetryPromptId: string | undefined, mode: RepeatedToolFailureGuardMode, previousState: RepeatedToolFailureGuardState, decision: RepeatedToolFailureGuardDecision, batch: RepeatedToolFailureBatch, ): void { - if (mode === 'off' || decision.kind === 'none') return; + if ( + mode === 'off' || + decision.kind === 'none' || + telemetryPromptId === undefined + ) { + return; + } const countState = decision.kind === 'reset' ? previousState : decision.state; const telemetryDecision = @@ -541,7 +556,7 @@ function recordRepeatedToolFailureDecision( logRepeatedToolFailureGuard( config, new RepeatedToolFailureGuardEvent({ - prompt_id: promptId, + prompt_id: telemetryPromptId, route: 'acp_foreground', mode, phase_before: previousState.phase, @@ -1490,9 +1505,16 @@ export class Session implements SessionContext { !this.config.getBareMode() && !this.config.isSafeMode(); this.todoStopGuard = new DaemonTodoStopGuard(todoStopGuardEnabled); - this.repeatedToolFailureGuardMode = resolveRepeatedToolFailureGuardMode( - process.env[REPEATED_TOOL_FAILURE_GUARD_MODE_ENV], - ); + const configuredGuardMode = + process.env[ENV_ACP_REPEATED_TOOL_FAILURE_GUARD]; + const parsedGuardMode = + parseRepeatedToolFailureGuardMode(configuredGuardMode); + this.repeatedToolFailureGuardMode = parsedGuardMode ?? 'shadow'; + if (configuredGuardMode?.trim() && parsedGuardMode === undefined) { + debugLogger.warn( + `${ENV_ACP_REPEATED_TOOL_FAILURE_GUARD} has an invalid value; defaulting to shadow. Expected off, shadow, warn, or enforce.`, + ); + } this.todoStopGuardBackgroundBaseline = this.#captureTodoStopGuardBackgroundBaseline(); @@ -3240,7 +3262,10 @@ export class Session implements SessionContext { let nextMessage: Content | null = { role: 'user', parts }; let turnCount = 0; const toolLoopState = createDaemonToolLoopState( - this.repeatedToolFailureGuardMode, + promptMetadata?.[CHANNEL_PROMPT_META_KEY] === true + ? 'off' + : this.repeatedToolFailureGuardMode, + promptId, ); // conversation_finished must fire on every terminal path of the @@ -3508,7 +3533,11 @@ export class Session implements SessionContext { ); nextMessage = nextAfterTools.message; if (nextAfterTools.stoppedByRepeatedToolFailure) { - return { stopReason: 'end_turn' }; + return { + stopReason: getAbortAwareEndTurnStopReason( + pendingSend.signal, + ), + }; } if (toolRun.loopDetected) { this.todoStopGuard.suspend(); @@ -4924,7 +4953,7 @@ export class Session implements SessionContext { ); recordRepeatedToolFailureDecision( this.config, - promptId, + toolLoopState.repeatedToolFailureTelemetryId, toolLoopState.repeatedToolFailureMode, previousRepeatedToolFailureState, repeatedToolFailureDecision, @@ -4961,7 +4990,7 @@ export class Session implements SessionContext { await this.messageRewriter?.waitForPendingRewrites(); recordDaemonLoopDetected( this.config, - promptId, + toolLoopState.repeatedToolFailureTelemetryId ?? promptId, LoopType.REPEATED_TOOL_EXECUTION_FAILURE, REPEATED_TOOL_FAILURE_STOP_MESSAGE, toolLoopState, diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts index fd5ea895598..48f9ad87f90 100644 --- a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts @@ -113,20 +113,27 @@ export function createRepeatedToolFailureGuardState(): RepeatedToolFailureGuardS }; } -export function resolveRepeatedToolFailureGuardMode( +export function parseRepeatedToolFailureGuardMode( value: string | undefined, -): RepeatedToolFailureGuardMode { - switch (value?.trim().toLowerCase()) { +): RepeatedToolFailureGuardMode | undefined { + const normalized = value?.trim().toLowerCase(); + switch (normalized) { case 'off': case 'shadow': case 'warn': case 'enforce': - return value.trim().toLowerCase() as RepeatedToolFailureGuardMode; + return normalized; default: - return 'shadow'; + return undefined; } } +export function resolveRepeatedToolFailureGuardMode( + value: string | undefined, +): RepeatedToolFailureGuardMode { + return parseRepeatedToolFailureGuardMode(value) ?? 'shadow'; +} + function resetState( state: RepeatedToolFailureGuardState, enforcementDisabled = state.enforcementDisabled, diff --git a/packages/cli/src/config/environment.test.ts b/packages/cli/src/config/environment.test.ts index 29e6ba78472..26ab189dc9d 100644 --- a/packages/cli/src/config/environment.test.ts +++ b/packages/cli/src/config/environment.test.ts @@ -9,6 +9,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { buildRuntimeEnvironment, loadEnvironment } from './environment.js'; +import { ENV_ACP_REPEATED_TOOL_FAILURE_GUARD } from './shared-env-keys.js'; import type { Settings } from './settingsSchema.js'; const TRACKED_ENV = [ @@ -25,6 +26,7 @@ const TRACKED_ENV = [ 'NODE_COMPILE_CACHE', 'NODE_DISABLE_COMPILE_CACHE', 'QWEN_HOME', + ENV_ACP_REPEATED_TOOL_FAILURE_GUARD, 'QWEN_CODE_PENDING_COMPILE_CACHE', 'QWEN_RUNTIME_DIR', 'QWEN_SERVER_TOKEN', @@ -82,6 +84,7 @@ describe('buildRuntimeEnvironment', () => { 'NODE_OPTIONS=--require ./bad.js', 'QWEN_SERVER_TOKEN=dotenv-token', 'QWEN_HOME=/tmp/ignored-qwen-home', + `${ENV_ACP_REPEATED_TOOL_FAILURE_GUARD}=enforce`, '', ].join('\n'), ); @@ -101,6 +104,7 @@ describe('buildRuntimeEnvironment', () => { RUNTIME_SETTINGS_EXCLUDED: 'settings-excluded', BASH_ENV: '/tmp/bad-profile', QWEN_RUNTIME_DIR: '/tmp/ignored-runtime-dir', + [ENV_ACP_REPEATED_TOOL_FAILURE_GUARD]: 'warn', }, }), workspace, @@ -121,6 +125,9 @@ describe('buildRuntimeEnvironment', () => { expect(snapshot.effectiveEnv['QWEN_SERVER_TOKEN']).toBeUndefined(); expect(snapshot.effectiveEnv['QWEN_HOME']).toBeUndefined(); expect(snapshot.effectiveEnv['QWEN_RUNTIME_DIR']).toBeUndefined(); + expect( + snapshot.effectiveEnv[ENV_ACP_REPEATED_TOOL_FAILURE_GUARD], + ).toBeUndefined(); expect(snapshot.overlayKeys).toEqual([ 'RUNTIME_DOTENV', 'RUNTIME_EMPTY', @@ -226,6 +233,7 @@ describe('loadEnvironment', () => { BASH_ENV: '/tmp/bad-profile', NODE_OPTIONS: '--require ./bad.js', QWEN_SERVER_TOKEN: 'bad-token', + [ENV_ACP_REPEATED_TOOL_FAILURE_GUARD]: 'enforce', }, }), workspace, @@ -235,5 +243,6 @@ describe('loadEnvironment', () => { expect(process.env['BASH_ENV']).toBeUndefined(); expect(process.env['NODE_OPTIONS']).toBeUndefined(); expect(process.env['QWEN_SERVER_TOKEN']).toBeUndefined(); + expect(process.env[ENV_ACP_REPEATED_TOOL_FAILURE_GUARD]).toBeUndefined(); }); }); diff --git a/packages/cli/src/config/shared-env-keys.test.ts b/packages/cli/src/config/shared-env-keys.test.ts index cf94d79481e..7895080d934 100644 --- a/packages/cli/src/config/shared-env-keys.test.ts +++ b/packages/cli/src/config/shared-env-keys.test.ts @@ -5,7 +5,10 @@ */ import { describe, it, expect } from 'vitest'; -import { PROJECT_ENV_HARDCODED_EXCLUSIONS } from './shared-env-keys.js'; +import { + ENV_ACP_REPEATED_TOOL_FAILURE_GUARD, + PROJECT_ENV_HARDCODED_EXCLUSIONS, +} from './shared-env-keys.js'; describe('PROJECT_ENV_HARDCODED_EXCLUSIONS', () => { // Security guard: a project `.env` must never be able to disable TLS @@ -23,4 +26,10 @@ describe('PROJECT_ENV_HARDCODED_EXCLUSIONS', () => { 'NODE_TLS_REJECT_UNAUTHORIZED', ); }); + + it('keeps ACP repeated-tool-failure rollout policy operator-owned', () => { + expect(PROJECT_ENV_HARDCODED_EXCLUSIONS).toContain( + ENV_ACP_REPEATED_TOOL_FAILURE_GUARD, + ); + }); }); diff --git a/packages/cli/src/config/shared-env-keys.ts b/packages/cli/src/config/shared-env-keys.ts index 277ede58d75..795df6e0658 100644 --- a/packages/cli/src/config/shared-env-keys.ts +++ b/packages/cli/src/config/shared-env-keys.ts @@ -8,6 +8,8 @@ export const DEFAULT_EXCLUDED_ENV_VARS = ['DEBUG', 'DEBUG_MODE']; export const ENV_CORRUPTED_PATH = 'QWEN_CODE_SETTINGS_CORRUPTED_PATH'; export const ENV_WAS_RECOVERED = 'QWEN_CODE_SETTINGS_WAS_RECOVERED'; +export const ENV_ACP_REPEATED_TOOL_FAILURE_GUARD = + 'QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD'; // QWEN_HOME and QWEN_RUNTIME_DIR control where global state (settings, OAuth // credentials, installation IDs, etc.) is written. A project `.env` must never @@ -21,6 +23,9 @@ export const PROJECT_ENV_HARDCODED_EXCLUSIONS = [ 'QWEN_CODE_TRUSTED_FOLDERS_PATH', ENV_CORRUPTED_PATH, ENV_WAS_RECOVERED, + // This is an operator rollout policy. A project must not be able to promote + // itself from the default shadow cohort into warning or enforcement. + ENV_ACP_REPEATED_TOOL_FAILURE_GUARD, // QWEN_TLS_INSECURE (and NODE_TLS_REJECT_UNAUTHORIZED, which it mirrors) // disable TLS certificate verification for all outbound API connections. A // project `.env` must never enable either — that would let an untrusted repo diff --git a/packages/core/src/telemetry/index.ts b/packages/core/src/telemetry/index.ts index cf3b7723e66..b49e6dd3f49 100644 --- a/packages/core/src/telemetry/index.ts +++ b/packages/core/src/telemetry/index.ts @@ -96,6 +96,7 @@ export { MemoryDreamEvent, MemoryRecallEvent, MemoryRecallDeliveryEvent, + RepeatedToolFailureGuardEvent, } from './types.js'; export { makeSlashCommandEvent, makeChatCompressionEvent } from './types.js'; export type { @@ -152,7 +153,6 @@ export { ApiRequestPhase, FileOperation, } from './metrics.js'; -export { RepeatedToolFailureGuardEvent } from './types.js'; export { QwenLogger } from './qwen-logger/qwen-logger.js'; export { sanitizeHookName } from './sanitize.js'; export { diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 34b891dc7fb..cd6199401d1 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -397,7 +397,6 @@ describe('loggers', () => { batch_count_bucket: '2', terminal_status: 'error', execution_status: 'error', - execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, tool_type: 'mcp', }); const serialized = JSON.stringify(mockLogger.emit.mock.calls.at(-1)); diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index fd721ebe117..5d026a9a76e 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -642,6 +642,8 @@ export function logLoopDetected( ): void { const privacyRestricted = event.loop_type === LoopType.REPEATED_TOOL_EXECUTION_FAILURE; + // This loop type uses its guard-generated opaque prompt correlation ID and + // intentionally omits the session-scoped RUM and common session attribute. if (!privacyRestricted) { QwenLogger.getInstance(config)?.logLoopDetectedEvent(event); } @@ -664,12 +666,8 @@ export function logRepeatedToolFailureGuard( _config: Config, event: RepeatedToolFailureGuardEvent, ): void { - let sdkInitialized = false; runToolTelemetrySink(() => { - sdkInitialized = isTelemetrySdkInitialized(); - }); - if (sdkInitialized) { - runToolTelemetrySink(() => { + if (isTelemetrySdkInitialized()) { const logger = logs.getLogger(SERVICE_NAME); logger.emit({ body: `Repeated tool failure guard decision: ${event.decision}.`, @@ -678,8 +676,8 @@ export function logRepeatedToolFailureGuard( 'event.name': EVENT_REPEATED_TOOL_FAILURE_GUARD, }, }); - }); - } + } + }); runToolTelemetrySink(() => { recordRepeatedToolFailureGuardMetrics({ route: event.route, @@ -698,9 +696,6 @@ export function logRepeatedToolFailureGuard( ...(event.execution_status !== undefined ? { execution_status: event.execution_status } : {}), - ...(event.execution_error_type !== undefined - ? { execution_error_type: event.execution_error_type } - : {}), ...(event.tool_type !== undefined ? { tool_type: event.tool_type } : {}), }); }); diff --git a/packages/core/src/telemetry/metrics.test.ts b/packages/core/src/telemetry/metrics.test.ts index e391141a416..890213f45d1 100644 --- a/packages/core/src/telemetry/metrics.test.ts +++ b/packages/core/src/telemetry/metrics.test.ts @@ -20,7 +20,6 @@ import { ApiRequestPhase, } from './metrics.js'; import { makeFakeConfig } from '../test-utils/config.js'; -import { ToolErrorType } from '../tools/tool-error.js'; const mockCounterAddFn: Mock< (value: number, attributes?: Attributes, context?: Context) => void @@ -359,7 +358,6 @@ describe('Telemetry Metrics', () => { batch_count_bucket: '3+', terminal_status: 'error', execution_status: 'error', - execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, tool_type: 'mcp', }); @@ -377,7 +375,6 @@ describe('Telemetry Metrics', () => { batch_count_bucket: '3+', terminal_status: 'error', execution_status: 'error', - execution_error_type: ToolErrorType.EXECUTION_TIMEOUT, tool_type: 'mcp', }); }); diff --git a/packages/core/src/telemetry/metrics.ts b/packages/core/src/telemetry/metrics.ts index 675adc73a1d..b413714bc57 100644 --- a/packages/core/src/telemetry/metrics.ts +++ b/packages/core/src/telemetry/metrics.ts @@ -16,7 +16,6 @@ import type { MemoryRecallDiscardReason, } from './types.js'; import type { ToolExecutionStatus } from '../core/turn.js'; -import type { ToolErrorType } from '../tools/tool-error.js'; const TOOL_CALL_COUNT = `${SERVICE_NAME}.tool.call.count`; const TOOL_EXECUTION_COUNT = `${SERVICE_NAME}.tool.execution.count`; @@ -142,7 +141,6 @@ const COUNTER_DEFINITIONS = { | 'contract_violation'; terminal_status?: 'error'; execution_status?: 'error'; - execution_error_type?: ToolErrorType; tool_type?: 'native' | 'mcp'; }, }, From ba8fe4e1d26f18f55bfc09f092c704f77c25dbc0 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 5 Aug 2026 16:18:25 +0800 Subject: [PATCH 3/5] fix(acp): address repeated failure guard review Co-authored-by: Qwen-Coder --- .../acp-repeated-tool-call-protection.md | 61 +++++++------- docs/users/configuration/settings.md | 2 +- packages/channels/base/src/index.ts | 1 + .../acp-integration/session/Session.test.ts | 16 ++-- .../src/acp-integration/session/Session.ts | 79 +++++++------------ .../repeated-tool-failure-guard.test.ts | 61 ++++++++++++-- .../session/repeated-tool-failure-guard.ts | 73 ++++++++++++----- packages/core/src/telemetry/loggers.test.ts | 34 ++++++-- packages/core/src/telemetry/loggers.ts | 10 +-- 9 files changed, 208 insertions(+), 129 deletions(-) diff --git a/docs/design/acp-repeated-tool-call-protection.md b/docs/design/acp-repeated-tool-call-protection.md index e0154052288..51939039aa3 100644 --- a/docs/design/acp-repeated-tool-call-protection.md +++ b/docs/design/acp-repeated-tool-call-protection.md @@ -285,7 +285,9 @@ interactive foreground ACP prompt. Both channel bridge implementations mark their prompts explicitly, and Session forces those marked prompts to `off` even when the process is configured for enforcement. It is not process-global and must not fall back to a legacy or primary runtime when workspace ownership -is unknown. +is unknown. The marker is client-asserted ACP metadata, so another client can +also opt its prompt out of this conservative protection; it is a routing trust +signal, not an authorization boundary. Modes: @@ -294,11 +296,12 @@ Modes: - `warn`: inject the reminder but never stop. - `enforce`: inject and stop according to the state machine. -Default is `shadow`. Unknown ownership, an untrusted producer, or mixed -deployment versions force at most `warn`. A missing `executionStatus` or an -unsupported outcome combination resets the streak and downgrades the rest of -that prompt to at most `warn`. Cron, notification, background, and custom -routes remain `off` in the first release. +Default is `shadow`. The deployment control plane must not assign `enforce` to +unknown ownership, untrusted producers, or mixed deployment versions; the +runtime does not infer those deployment properties. A missing +`executionStatus` or an unsupported outcome combination resets the streak and +downgrades the rest of that prompt to at most `warn`. Cron, notification, and +background routes remain `off` in the first release. The mode is an operator-controlled deployment policy, not a user-facing setting. `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` selects `off`, `shadow`, @@ -306,9 +309,9 @@ setting. `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` selects `off`, `shadow`, to `shadow`, and a non-empty invalid value records an operator diagnostic. Project `.env`, project `.qwen/.env`, and workspace `settings.env` sources are not allowed to set this policy; an exported process value or a user-level -environment file remains valid. The deployment control plane must set it only -on the assigned version-pinned cohort. This feature does not introduce a -second rollout or owner-assignment service. +environment file remains valid. The deployment control plane must set it above +`shadow` only on the assigned version-pinned cohort. This feature does not +introduce a second rollout or owner-assignment service. The guard depends on the ACP host implementing `craft/drainMidTurnQueue` with a boolean `hasQueuedPrompt` when the guard is enabled. Older or third-party hosts @@ -320,8 +323,8 @@ supported-host shadow baseline. ## Telemetry and privacy -Emit low-cardinality counters plus one privacy-restricted structured diagnostic -log per reducer transition: +Emit low-cardinality counters plus one data-minimized structured diagnostic log +per reducer transition: - deployment environment and service version from the existing OpenTelemetry resource rather than new guard labels; @@ -337,24 +340,26 @@ log per reducer transition: `external_input`, or `contract_violation`; - failure count bucket: `0`, `1-2`, `3-4`, `5-7`, or `8+`; - batch count bucket: `0`, `1`, `2`, or `3+`; -- a prompt-local guard correlation ID in the diagnostic log only, for checking - transition order; it is a one-way SHA-256 digest of the ACP prompt ID, so an - authorized investigation can hash a known trace prompt ID while the central - event does not disclose the prompt or session ID; and +- the same raw ACP `prompt_id` already emitted by tool-call telemetry, in the + diagnostic log only, so authorized rollout analysis can join guard + transitions to the settled tool batch without a second identifier space; and - a prompt-local candidate ordinal in the diagnostic log only. The reducer reuses the ordinal while the same private key is active and allocates a new one when the key changes. -The guard correlation ID and candidate ordinal are never metric labels. The -ordinal cannot correlate a tool across prompts and does not reveal its -identity. An `idle`-to-`idle` observation emits nothing. +The prompt ID and candidate ordinal are never metric labels. The ordinal cannot +correlate a tool across prompts and does not reveal its identity. An +`idle`-to-`idle` observation emits nothing. The terminal `repeated_tool_execution_failure` loop event uses the same -privacy-restricted OpenTelemetry path and bypasses session-scoped RUM. Other -loop types keep their existing telemetry behavior. +OpenTelemetry prompt ID and explicitly bypasses QwenLogger/RUM at the Session +call site. The shared Core logger does not infer destinations from the loop +type. Other loop types keep their existing telemetry behavior. Do not emit tool arguments, results, raw error messages, stack traces, paths, -MCP server names, user IDs, session IDs, or the unhashed failure key. +MCP server names, user IDs, or the private failure key in guard-specific +fields. The existing OpenTelemetry prompt correlation and the terminal loop +event's session field retain their normal meaning and access policy. Cancellation is excluded from every failure-rate numerator. The primary execution SLI uses PR #8180's contract: @@ -395,10 +400,11 @@ Shadow mode advances a virtual warned state without injecting the reminder, so `would_warn` and `would_stop` estimate volume only. They cannot establish how a model behaves after seeing the reminder. -The default shadow mode adds `todoStopGuardWatchQueuedPrompt: true` to the -existing mid-turn drain request so the reducer can prove that no full prompt is -queued. Hosts without that response contract are counted through -`unreliable_input` and excluded from supported-host shadow conclusions. +The default shadow mode does not change model continuation or injected +messages. It does add `todoStopGuardWatchQueuedPrompt: true` to the existing +mid-turn drain request so the reducer can prove that no full prompt is queued. +Hosts without that response contract are counted through `unreliable_input` +and excluded from supported-host shadow conclusions. Required invariants: @@ -518,8 +524,9 @@ Automated ACP Session and channel tests cover: Telemetry tests cover: - low-cardinality attributes; and -- exclusion of session-scoped common attributes and sensitive tool fields from - the guard and terminal loop events. +- explicit exclusion of the terminal loop event from QwenLogger/RUM while + preserving standard OpenTelemetry correlation fields, plus exclusion of + sensitive tool fields from guard-specific telemetry. The behavioral change also has an E2E plan under `.qwen/e2e-tests/`. Its manual ACP fixture run remains required before promotion out of Draft. It covers one diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index 42ea586be43..f412c0c5cf4 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -730,7 +730,7 @@ For authentication-related variables (like `OPENAI_*`) and the recommended `.qwe | `CODE_ASSIST_ENDPOINT` | Specifies the endpoint for the code assist server. | This is useful for development and testing. | | `QWEN_CODE_MAX_OUTPUT_TOKENS` | Overrides the default maximum output tokens per response. When not set, Qwen Code defaults to the model's declared output limit and, if a response is truncated, automatically escalates (64K floor) and recovers across turns. Set this to a specific value (e.g., `16000`) to use a fixed limit instead — useful for capacity-constrained self-hosted backends that want a lower per-request slot reservation. | Takes precedence over the model-limit default but is overridden by `samplingParams.max_tokens` in settings. Disables automatic escalation when set. Example: `export QWEN_CODE_MAX_OUTPUT_TOKENS=16000` | | `QWEN_CODE_UNATTENDED_RETRY` | Set to `true` or `1` to enable persistent retry mode. When enabled, transient API capacity errors (HTTP 429 Rate Limit and 529 Overloaded) are retried indefinitely with exponential backoff (capped at 5 minutes per retry) and heartbeat keepalives every 30 seconds on stderr. | Designed for CI/CD pipelines and background automation where long-running tasks should survive temporary API outages. Must be set explicitly — `CI=true` alone does **not** activate this mode. See [Headless Mode](../features/headless#persistent-retry-mode) for details. Example: `export QWEN_CODE_UNATTENDED_RETRY=1` | -| `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` | Operator rollout mode for ACP repeated tool-execution failure protection. Accepts `off`, `shadow`, `warn`, or `enforce`; missing or invalid values default to `shadow`. | Applies only to interactive foreground ACP prompts; channel-driven and other automatic routes remain off. Non-empty invalid values produce a diagnostic. Because this is operator policy, project `.env`, project `.qwen/.env`, and workspace `settings.env` cannot set it; export it in the process environment or use a user-level environment file. Shadow, warn, and enforce require the ACP host's `craft/drainMidTurnQueue` response to include reliable queued-prompt state. | +| `QWEN_CODE_ACP_REPEATED_TOOL_FAILURE_GUARD` | Operator rollout mode for ACP repeated tool-execution failure protection. Accepts `off`, `shadow`, `warn`, or `enforce`; missing or invalid values default to `shadow`. | Applies only to interactive foreground ACP prompts; channel-driven and automatic routes remain off. Project and workspace environment files cannot set this operator policy. Shadow leaves model continuation and messages unchanged but adds the queued-prompt watch flag to `craft/drainMidTurnQueue`; every non-off mode requires reliable queued-prompt state. Non-empty invalid values emit a diagnostic; export the variable in the process environment or a user-level file. | | `QWEN_CODE_PROFILE_STARTUP` | Set to `1` to enable startup performance profiling. Writes a JSON timing report to `~/.qwen/startup-perf/` with per-phase durations. | Only active inside the sandbox child process (or with `QWEN_CODE_PROFILE_STARTUP_OUTER=1`). Zero overhead when not set. Example: `export QWEN_CODE_PROFILE_STARTUP=1` | | `QWEN_CODE_PROFILE_STARTUP_OUTER` | Set to `1` together with `QWEN_CODE_PROFILE_STARTUP=1` to also collect a startup profile in the outer (pre-sandbox) process. Outer-process reports get an `outer-` filename prefix to keep them distinct from the sandbox child's report. | Off by default — only the sandbox child collects, to avoid duplicate reports. Useful for local development where the cli isn't relaunched into a sandbox. | | `QWEN_CODE_PROFILE_STARTUP_NO_HEAP` | Set to `1` together with `QWEN_CODE_PROFILE_STARTUP=1` to skip the per-checkpoint `process.memoryUsage()` snapshots. Useful when measuring the profiler's own Heisenberg overhead. | Off by default. Heap snapshots cost ~50 µs each (well below 1% of total startup) so most users should leave this alone. | diff --git a/packages/channels/base/src/index.ts b/packages/channels/base/src/index.ts index c454c9393d5..877d25dafeb 100644 --- a/packages/channels/base/src/index.ts +++ b/packages/channels/base/src/index.ts @@ -17,6 +17,7 @@ export type { SessionDiedEvent, ToolCallEvent, } from './ChannelAgentBridge.js'; +export { CHANNEL_PROMPT_META_KEY } from './ChannelAgentBridge.js'; export type { AcpBridgeOptions } from './AcpBridge.js'; export { DaemonChannelBridge } from './DaemonChannelBridge.js'; export type { diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 2a639557b65..995f77e7577 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -6,7 +6,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { EventEmitter } from 'node:events'; -import { createHash } from 'node:crypto'; import * as fsSync from 'node:fs'; import * as fs from 'node:fs/promises'; import * as os from 'node:os'; @@ -51,6 +50,7 @@ import * as nonInteractiveCliCommands from '../../nonInteractiveCliCommands.js'; import { CommandKind } from '../../ui/commands/types.js'; import { MessageType } from '../../ui/types.js'; import { buildAcpModelOptions } from '../../utils/acpModelUtils.js'; +import { CHANNEL_PROMPT_META_KEY } from '@qwen-code/channel-base'; const debugLoggerWarnSpy = vi.hoisted(() => vi.fn()); const debugLoggerDebugSpy = vi.hoisted(() => vi.fn()); @@ -6550,7 +6550,7 @@ describe('Session', () => { session.prompt({ sessionId: 'test-session-id', prompt: [{ type: 'text', text: 'channel task' }], - _meta: { 'qwen.channel.prompt': true }, + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, }), ).resolves.toEqual({ stopReason: 'end_turn' }); @@ -6700,22 +6700,16 @@ describe('Session', () => { mockConfig, expect.objectContaining({ loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + prompt_id: 'test-session-id########1', }), + { recordToQwenLogger: false }, ); const telemetryPromptIds = logRepeatedToolFailureGuardSpy.mock.calls.map( ([, event]) => event.prompt_id, ); expect(new Set(telemetryPromptIds).size).toBe(1); - expect(telemetryPromptIds[0]).toBe( - createHash('sha256') - .update('test-session-id########1') - .digest('hex'), - ); - expect(logLoopDetectedSpy).toHaveBeenCalledWith( - mockConfig, - expect.objectContaining({ prompt_id: telemetryPromptIds[0] }), - ); + expect(telemetryPromptIds[0]).toBe('test-session-id########1'); expect(mockChat.addHistory).toHaveBeenCalledWith({ role: 'user', parts: expect.arrayContaining([ diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 2af32f957cc..88c064e96ee 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -5,7 +5,7 @@ */ import { Buffer } from 'node:buffer'; -import { createHash, randomUUID } from 'node:crypto'; +import { randomUUID } from 'node:crypto'; import { realpathSync, statSync } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -170,6 +170,7 @@ import { runWithInvocationContext, } from '@qwen-code/qwen-code-core'; import { NOT_CURRENTLY_GENERATING_CANCEL_MESSAGE } from '@qwen-code/acp-bridge/bridgeErrors'; +import { CHANNEL_PROMPT_META_KEY } from '@qwen-code/channel-base'; import { ENV_ACP_REPEATED_TOOL_FAILURE_GUARD } from '../../config/shared-env-keys.js'; // Single source of truth shared with the daemon-side answerer (BridgeClient), // so a rename can't desync caller and answerer into a silent -32601 latch. @@ -305,7 +306,6 @@ const USER_CANCEL_ABORT_REASON = 'qwen:user-cancel'; const SESSION_DISPOSE_ABORT_REASON = 'qwen:session-dispose'; const DAEMON_RETRY_META_KEY = 'qwen.daemon.retry'; const DAEMON_CONTINUE_META_KEY = 'qwen.daemon.continueLastTurn'; -const CHANNEL_PROMPT_META_KEY = 'qwen.channel.prompt'; const TODO_STOP_GUARD_PROMPT_PREFIX = '[Todo Stop Guard] '; const TODO_STOP_GUARD_PROMPT_BODY_SUFFIX = ' todo item(s) are still pending or in progress. Continue executing the current task now. Do not ask the user whether to continue. If progress requires user input, use the structured question or permission flow. If progress depends on external state, report the blocker explicitly.'; @@ -460,7 +460,6 @@ type DaemonToolLoopState = { loopDetected: boolean; repeatedToolFailureMode: RepeatedToolFailureGuardMode; repeatedToolFailureState: RepeatedToolFailureGuardState; - repeatedToolFailureTelemetryId?: string; }; const DAEMON_INVALID_TOOL_PARAMS_THRESHOLD = 3; @@ -476,8 +475,7 @@ const TOOL_POST_EXECUTION_CANCELLED_MESSAGE = 'The tool had already completed; its output was discarded.'; function createDaemonToolLoopState( - repeatedToolFailureMode: RepeatedToolFailureGuardMode = 'off', - telemetrySourceId?: string, + repeatedToolFailureMode: RepeatedToolFailureGuardMode, ): DaemonToolLoopState { return { totalToolCalls: 0, @@ -485,13 +483,6 @@ function createDaemonToolLoopState( loopDetected: false, repeatedToolFailureMode, repeatedToolFailureState: createRepeatedToolFailureGuardState(), - ...(repeatedToolFailureMode === 'off' || telemetrySourceId === undefined - ? {} - : { - repeatedToolFailureTelemetryId: createHash('sha256') - .update(telemetrySourceId) - .digest('hex'), - }), }; } @@ -514,17 +505,13 @@ function repeatedToolFailureBatchBucket(count: number): '0' | '1' | '2' | '3+' { function recordRepeatedToolFailureDecision( config: Config, - telemetryPromptId: string | undefined, + promptId: string, mode: RepeatedToolFailureGuardMode, previousState: RepeatedToolFailureGuardState, decision: RepeatedToolFailureGuardDecision, batch: RepeatedToolFailureBatch, ): void { - if ( - mode === 'off' || - decision.kind === 'none' || - telemetryPromptId === undefined - ) { + if (mode === 'off' || decision.kind === 'none') { return; } @@ -556,7 +543,7 @@ function recordRepeatedToolFailureDecision( logRepeatedToolFailureGuard( config, new RepeatedToolFailureGuardEvent({ - prompt_id: telemetryPromptId, + prompt_id: promptId, route: 'acp_foreground', mode, phase_before: previousState.phase, @@ -593,12 +580,17 @@ function recordDaemonLoopDetected( loopType: LoopType, message: string, loopState: DaemonToolLoopState, + options: { recordToQwenLogger?: boolean } = {}, ): true { if (!loopState.loopDetected) { loopState.loopDetected = true; debugLogger.warn(message); try { - logLoopDetected(config, new LoopDetectedEvent(loopType, promptId)); + logLoopDetected( + config, + new LoopDetectedEvent(loopType, promptId), + options, + ); } catch (error) { debugLogger.debug( '[Session] Failed to record loop detection telemetry', @@ -3283,7 +3275,6 @@ export class Session implements SessionContext { promptMetadata?.[CHANNEL_PROMPT_META_KEY] === true ? 'off' : this.repeatedToolFailureGuardMode, - promptId, ); // conversation_finished must fire on every terminal path of the @@ -3661,7 +3652,7 @@ export class Session implements SessionContext { if (this.todoStopGuard.needsStopInspection) { const drained = await this.#drainMidTurnInput(pendingSend.signal, { - watchQueuedPromptForTodoStopGuard: true, + watchQueuedPrompt: true, onFullTurnModel, }); if (drained.parts.length > 0) { @@ -3760,7 +3751,7 @@ export class Session implements SessionContext { if (this.todoStopGuard.needsStopInspection) { const drained = await this.#drainMidTurnInput(pendingSend.signal, { - watchQueuedPromptForTodoStopGuard: true, + watchQueuedPrompt: true, onFullTurnModel, }); if (drained.parts.length > 0) { @@ -3944,7 +3935,7 @@ export class Session implements SessionContext { ): Promise { let nextMessage: Content | null = { role: 'user', parts }; let nextGuardContinuation = options.guardContinuation; - const toolLoopState = createDaemonToolLoopState(); + const toolLoopState = createDaemonToolLoopState('off'); let initialSend = true; let automaticContinuationValidated = false; let supersededAutomaticContinuation = false; @@ -4027,7 +4018,7 @@ export class Session implements SessionContext { const drained = await this.#drainMidTurnInput( pendingSend.signal, { - watchQueuedPromptForTodoStopGuard: true, + watchQueuedPrompt: true, onFullTurnModel: options.onFullTurnModel, }, ); @@ -4507,15 +4498,6 @@ export class Session implements SessionContext { options.onFullTurnModel, ); nextMessage = nextAfterTools.message; - if (nextAfterTools.stoppedByRepeatedToolFailure) { - return { - kind: 'terminal', - stopReason: 'end_turn', - ...(supersededAutomaticContinuation - ? { supersededAutomaticContinuation: true } - : {}), - }; - } if (nextAfterTools.hadMidTurnUserInput) { nextGuardContinuation = undefined; continue; @@ -4965,8 +4947,7 @@ export class Session implements SessionContext { return { message: null, hadMidTurnUserInput: false }; } const drained = await this.#drainMidTurnInput(abortSignal, { - watchQueuedPromptForTodoStopGuard: - toolLoopState.repeatedToolFailureMode !== 'off', + watchQueuedPrompt: toolLoopState.repeatedToolFailureMode !== 'off', onFullTurnModel, }); const hadMidTurnUserInput = drained.parts.length > 0; @@ -5000,7 +4981,7 @@ export class Session implements SessionContext { ); recordRepeatedToolFailureDecision( this.config, - toolLoopState.repeatedToolFailureTelemetryId, + promptId, toolLoopState.repeatedToolFailureMode, previousRepeatedToolFailureState, repeatedToolFailureDecision, @@ -5037,10 +5018,11 @@ export class Session implements SessionContext { await this.messageRewriter?.waitForPendingRewrites(); recordDaemonLoopDetected( this.config, - toolLoopState.repeatedToolFailureTelemetryId ?? promptId, + promptId, LoopType.REPEATED_TOOL_EXECUTION_FAILURE, REPEATED_TOOL_FAILURE_STOP_MESSAGE, toolLoopState, + { recordToQwenLogger: false }, ); try { await this.messageEmitter.emitAgentMessage( @@ -5180,7 +5162,7 @@ export class Session implements SessionContext { async #drainMidTurnInput( abortSignal: AbortSignal, options: { - watchQueuedPromptForTodoStopGuard?: boolean; + watchQueuedPrompt?: boolean; onFullTurnModel?: (model: string) => boolean; } = {}, ): Promise { @@ -5203,7 +5185,8 @@ export class Session implements SessionContext { try { drainPromise = this.client.extMethod(MID_TURN_QUEUE_DRAIN_METHOD, { sessionId: this.sessionId, - ...(options.watchQueuedPromptForTodoStopGuard + // Keep the legacy wire name for ACP host compatibility. + ...(options.watchQueuedPrompt ? { todoStopGuardWatchQueuedPrompt: true } : {}), }); @@ -5223,7 +5206,7 @@ export class Session implements SessionContext { this.midTurnDrainTimeoutStrikes = 0; const reliable = isValidMidTurnDrainResponse( response, - options.watchQueuedPromptForTodoStopGuard === true, + options.watchQueuedPrompt === true, ); return { parts: await this.#buildMidTurnParts( @@ -5810,7 +5793,7 @@ export class Session implements SessionContext { { text: modelText }, ], }; - const toolLoopState = createDaemonToolLoopState(); + const toolLoopState = createDaemonToolLoopState('off'); while (nextMessage !== null) { turnCount++; @@ -5980,9 +5963,6 @@ export class Session implements SessionContext { toolLoopState, ); nextMessage = nextAfterTools.message; - if (nextAfterTools.stoppedByRepeatedToolFailure) { - return; - } if (toolRun.loopDetected) { this.todoStopGuard.suspend(); await this.#preserveStoppedToolRun(toolRun, ac.signal); @@ -6341,7 +6321,7 @@ export class Session implements SessionContext { ...notificationParts, ], }; - const toolLoopState = createDaemonToolLoopState(); + const toolLoopState = createDaemonToolLoopState('off'); while (nextMessage !== null) { if (ac.signal.aborted) { @@ -6499,10 +6479,6 @@ export class Session implements SessionContext { toolLoopState, ); nextMessage = nextAfterTools.message; - if (nextAfterTools.stoppedByRepeatedToolFailure) { - await this.#emitBackgroundNotificationEndTurn('end_turn'); - return; - } if (toolRun.loopDetected) { this.todoStopGuard.suspend(); await this.#preserveStoppedToolRun(toolRun, ac.signal); @@ -9349,7 +9325,8 @@ export class Session implements SessionContext { persistedOutputFiles: toolResult.persistedOutputFiles, policyToolName, toolType, - executionErrorType, + executionErrorType: + executionStatus === 'error' ? executionErrorType : undefined, metadata: { callId, status, diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts index 9dbdc2ca0e9..39de3008fc5 100644 --- a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts @@ -14,7 +14,7 @@ import { reduceRepeatedToolFailureGuard, REPEATED_TOOL_FAILURE_REMINDER, REPEATED_TOOL_FAILURE_STOP_MESSAGE, - resolveRepeatedToolFailureGuardMode, + parseRepeatedToolFailureGuardMode, type RepeatedToolFailureGuardMode, type RepeatedToolFailureObservation, type RepeatedToolFailureTerminalStatus, @@ -58,10 +58,10 @@ function reduce( } describe('repeated tool failure guard', () => { - it('defaults invalid or missing deployment modes to shadow', () => { - expect(resolveRepeatedToolFailureGuardMode(undefined)).toBe('shadow'); - expect(resolveRepeatedToolFailureGuardMode('invalid')).toBe('shadow'); - expect(resolveRepeatedToolFailureGuardMode(' WARN ')).toBe('warn'); + it('parses valid deployment modes and rejects missing or invalid values', () => { + expect(parseRepeatedToolFailureGuardMode(undefined)).toBeUndefined(); + expect(parseRepeatedToolFailureGuardMode('invalid')).toBeUndefined(); + expect(parseRepeatedToolFailureGuardMode(' WARN ')).toBe('warn'); }); it('does no work when the deployment mode is off', () => { @@ -327,6 +327,57 @@ describe('repeated tool failure guard', () => { }); }); + it.each([ + ['cancelled first', ['cancelled', 'unknown']], + ['unknown first', ['unknown', 'cancelled']], + ] as const)( + 'classifies mixed ineligible batches deterministically when %s', + (_label, order) => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const byOutcome = { + cancelled: observation({ + callId: 'cancelled', + terminalStatus: 'cancelled', + executionStatus: 'cancelled', + executionErrorType: undefined, + }), + unknown: observation({ + callId: 'unknown', + policyToolName: undefined, + }), + }; + const result = reduce( + tracked.state, + order.map((outcome) => byOutcome[outcome]), + ); + + expect(result).toMatchObject({ + kind: 'reset', + reason: 'cancelled', + state: { enforcementDisabled: true }, + }); + }, + ); + + it.each(['error', 'success'] as const)( + 'fails open when a future execution status reaches a terminal %s observation', + (terminalStatus) => { + const result = reduce(createRepeatedToolFailureGuardState(), [ + observation({ + terminalStatus, + executionStatus: 'timeout' as ToolExecutionStatus, + }), + ]); + + expect(result).toMatchObject({ + reason: 'unknown', + state: { enforcementDisabled: true }, + }); + }, + ); + it('uses fixed privacy-safe reminder and stop text', () => { expect(REPEATED_TOOL_FAILURE_REMINDER).toBe( 'System: the same tool execution has failed repeatedly for the same classified reason. Do not repeat the same approach. Inspect the returned result, change the approach or required preconditions, or explain the blocker.', diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts index 48f9ad87f90..9d8ae710226 100644 --- a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts @@ -78,6 +78,15 @@ export type RepeatedToolFailureResetReason = | 'unreliable_input' | 'contract_violation'; +const INELIGIBLE_RESET_REASON_PRECEDENCE = [ + 'contract_violation', + 'cancelled', + 'unknown', + 'not_started', + 'post_execution_failure', + 'success', +] as const satisfies readonly RepeatedToolFailureResetReason[]; + export type RepeatedToolFailureGuardDecision = | { kind: 'none'; state: RepeatedToolFailureGuardState } | { @@ -128,12 +137,6 @@ export function parseRepeatedToolFailureGuardMode( } } -export function resolveRepeatedToolFailureGuardMode( - value: string | undefined, -): RepeatedToolFailureGuardMode { - return parseRepeatedToolFailureGuardMode(value) ?? 'shadow'; -} - function resetState( state: RepeatedToolFailureGuardState, enforcementDisabled = state.enforcementDisabled, @@ -205,38 +208,59 @@ export function reduceRepeatedToolFailureGuard( } const eligible: FailureKey[] = []; + const resetReasons = new Set(); + let enforcementDisabled = state.enforcementDisabled; for (const observation of observations) { const { terminalStatus, executionStatus } = observation; - if ( - terminalStatus === 'success' && - (executionStatus === 'error' || executionStatus === 'cancelled') - ) { - return reset(state, 'contract_violation', true); - } if (terminalStatus === 'cancelled') { - return reset(state, 'cancelled'); + resetReasons.add('cancelled'); + continue; } if (executionStatus === undefined || executionStatus === 'unknown') { - return reset(state, 'unknown', true); - } - if (executionStatus === 'cancelled') { - return reset(state, 'cancelled'); + resetReasons.add('unknown'); + enforcementDisabled = true; + continue; } if (terminalStatus === 'success') { - return reset(state, 'success'); + if (executionStatus === 'error' || executionStatus === 'cancelled') { + resetReasons.add('contract_violation'); + enforcementDisabled = true; + } else if ( + executionStatus === 'success' || + executionStatus === 'not_started' + ) { + resetReasons.add('success'); + } else { + resetReasons.add('unknown'); + enforcementDisabled = true; + } + continue; + } + if (executionStatus === 'cancelled') { + resetReasons.add('cancelled'); + continue; } if (executionStatus === 'not_started') { - return reset(state, 'not_started'); + resetReasons.add('not_started'); + continue; } if (executionStatus === 'success') { - return reset(state, 'post_execution_failure'); + resetReasons.add('post_execution_failure'); + continue; + } + if (executionStatus !== 'error') { + resetReasons.add('unknown'); + enforcementDisabled = true; + continue; } if ( !observation.policyToolName || observation.executionErrorType === undefined || observation.executionErrorType === ToolErrorType.UNKNOWN ) { - return reset(state, 'unknown', true); + resetReasons.add('unknown'); + enforcementDisabled = true; + continue; } eligible.push({ policyToolName: observation.policyToolName, @@ -244,6 +268,13 @@ export function reduceRepeatedToolFailureGuard( }); } + const resetReason = INELIGIBLE_RESET_REASON_PRECEDENCE.find((reason) => + resetReasons.has(reason), + ); + if (resetReason !== undefined) { + return reset(state, resetReason, enforcementDisabled); + } + const key = eligible[0]; if (!key) { return reset(state, 'unknown', true); diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index cd6199401d1..ac65d702fb8 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -354,7 +354,7 @@ describe('loggers', () => { }); describe('logRepeatedToolFailureGuard', () => { - it('emits a privacy-safe transition log and low-cardinality metric', () => { + it('emits a data-minimized transition log and low-cardinality metric', () => { const config = makeFakeConfig({ sessionId: 'test-session-id' }); vi.spyOn( metrics, @@ -438,7 +438,7 @@ describe('loggers', () => { }); describe('logLoopDetected', () => { - it('keeps repeated execution failure stops out of session-scoped RUM', () => { + it('does not infer telemetry destinations from the loop type', () => { const config = makeFakeConfig({ sessionId: 'test-session-id' }); const logLoopDetectedEvent = vi.fn(); const getInstanceSpy = vi @@ -454,14 +454,36 @@ describe('loggers', () => { try { logLoopDetected(config, event); + expect(logLoopDetectedEvent).toHaveBeenCalledWith(event); + } finally { + getInstanceSpy.mockRestore(); + } + }); + + it('supports explicitly keeping a loop event out of QwenLogger', () => { + const config = makeFakeConfig({ sessionId: 'test-session-id' }); + const logLoopDetectedEvent = vi.fn(); + const getInstanceSpy = vi + .spyOn(QwenLogger, 'getInstance') + .mockReturnValue({ + logLoopDetectedEvent, + } as unknown as QwenLogger); + const event = new LoopDetectedEvent( + LoopType.REPEATED_TOOL_EXECUTION_FAILURE, + 'prompt-id', + ); + + try { + logLoopDetected(config, event, { recordToQwenLogger: false }); + expect(logLoopDetectedEvent).not.toHaveBeenCalled(); expect(mockLogger.emit).toHaveBeenCalledWith({ body: `Loop detected. Type: ${LoopType.REPEATED_TOOL_EXECUTION_FAILURE}.`, - attributes: event, + attributes: { + 'session.id': 'test-session-id', + ...event, + }, }); - expect( - mockLogger.emit.mock.calls.at(-1)?.[0].attributes, - ).not.toHaveProperty('session.id'); } finally { getInstanceSpy.mockRestore(); } diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index 5d026a9a76e..a39f61c1650 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -135,7 +135,6 @@ import type { MemoryRecallEvent, MemoryRecallDeliveryEvent, } from './types.js'; -import { LoopType } from './types.js'; import type { HookCallEvent } from './types.js'; import type { UiEvent } from './uiTelemetry.js'; import { uiTelemetryService } from './uiTelemetry.js'; @@ -639,18 +638,15 @@ export function logApiResponse(config: Config, event: ApiResponseEvent): void { export function logLoopDetected( config: Config, event: LoopDetectedEvent, + options: { recordToQwenLogger?: boolean } = {}, ): void { - const privacyRestricted = - event.loop_type === LoopType.REPEATED_TOOL_EXECUTION_FAILURE; - // This loop type uses its guard-generated opaque prompt correlation ID and - // intentionally omits the session-scoped RUM and common session attribute. - if (!privacyRestricted) { + if (options.recordToQwenLogger !== false) { QwenLogger.getInstance(config)?.logLoopDetectedEvent(event); } if (!isTelemetrySdkInitialized()) return; const attributes: LogAttributes = { - ...(privacyRestricted ? {} : getCommonAttributes(config)), + ...getCommonAttributes(config), ...event, }; From 8cb87ea5477a795b27792132fcda6bb40ab0638f Mon Sep 17 00:00:00 2001 From: "jinye.djy" Date: Thu, 6 Aug 2026 15:10:33 +0800 Subject: [PATCH 4/5] fix(acp): improve repeated failure guard recall Co-authored-by: Qwen-Coder --- .../acp-repeated-tool-call-protection.md | 57 +++++-- .../channels/base/src/ChannelAgentBridge.ts | 1 + .../acp-integration/session/Session.test.ts | 16 +- .../src/acp-integration/session/Session.ts | 66 ++++---- .../repeated-tool-failure-guard.test.ts | 148 ++++++++++++++++++ .../session/repeated-tool-failure-guard.ts | 26 ++- packages/core/src/telemetry/loggers.test.ts | 6 +- packages/core/src/telemetry/loggers.ts | 3 +- 8 files changed, 255 insertions(+), 68 deletions(-) diff --git a/docs/design/acp-repeated-tool-call-protection.md b/docs/design/acp-repeated-tool-call-protection.md index 51939039aa3..1d6abe4c671 100644 --- a/docs/design/acp-repeated-tool-call-protection.md +++ b/docs/design/acp-repeated-tool-call-protection.md @@ -9,8 +9,9 @@ Area: Interactive ACP foreground prompt loop ACP should stop an automatic model loop when the same resolved tool repeatedly reaches the same trusted execution failure. The protection is conservative: it observes only finalized, fully settled tool batches; gives the model one -fixed corrective reminder per candidate streak; and stops only if the next -batch repeats the same failure. +fixed corrective reminder per candidate streak; and stops only if a later +matching batch repeats the same failure without that tool succeeding in +between. The first version is an in-memory, per-prompt semantic guard. It does not replace the existing protections for duplicate provider call IDs, invalid @@ -87,8 +88,8 @@ frames, or spans. | Terminal `status` | `executionStatus` | Meaning for this guard | | ----------------- | -------------------- | ---------------------------------------------------------------------- | -| `success` | `success` | Reset | -| `success` | `not_started` | Reset; protocol-level synthetic result | +| `success` | `success` | Reset a candidate for the same resolved tool | +| `success` | `not_started` | Reset the same-tool candidate; protocol-level synthetic result | | `error` | `not_started` | Reset; validation, permission rejection, hook block, or lookup failure | | `error` | `error` | Eligible only with a trusted frozen `executionErrorType` | | `error` | `success` | Reset; execution succeeded and later processing failed | @@ -179,18 +180,24 @@ After every `runToolCalls` batch has fully settled: leave the existing input and FIFO behavior authoritative. If the drain is unreliable, reset and disable enforcement for the rest of the prompt. 3. Reset to `idle` if the batch is incomplete, violates the outcome contract, - or contains any reset-class outcome. -4. Collect eligible failure keys from the batch. If there is not exactly one - unique key, reset to `idle`. + or contains cancellation, unknown, not-started, or post-execution failure. +4. Collect eligible failure keys from the batch. If there is more than one + unique key, reset to `idle`. A successful observation resets only a + candidate for the same resolved tool; successful observations from other + tools neither advance nor reset the candidate. A success has no execution + error type, so it clears every failure classification for that tool before + the remaining unique failure key is selected. 5. If the key differs from the tracked key, begin a new streak from this batch. Otherwise add the batch's eligible failure count and increment the batch - count. + count. A complete batch containing only unrelated successful tools leaves + the current streak unchanged. 6. When `failureCount >= 8` and `batchCount >= 2`, transition to `warned` and request the mode-specific reminder action shown below. -7. If the immediately following complete batch contains the same eligible key - and no reset condition, execute and record the whole batch, then transition - to `latched`. The configured mode determines whether another model request - is sent. +7. If a later complete batch contains the same eligible key and no reset + condition, execute and record the whole batch, then transition to `latched`. + Intervening successes from other tools do not hide the repeated failure; a + success from the candidate tool resets it. The configured mode determines + whether another model request is sent. The state transition and control action are separate: @@ -206,6 +213,13 @@ Resetting a candidate and later tracking a different failure key may produce a new reminder; the one-reminder guarantee is per candidate streak, not per top-level prompt. +The first release intentionally retains one active candidate instead of a map +of independent streaks. After same-tool successes are removed, multiple +remaining failure keys reset because choosing which concurrent failure should +own a reminder or stop is ambiguous. This keeps enforcement conservative while +still detecting the dominant interleaved shape in which one tool keeps failing +as read, edit, or inspection tools succeed between attempts. + The reminder is: > System: the same tool execution has failed repeatedly for the same classified @@ -286,8 +300,8 @@ their prompts explicitly, and Session forces those marked prompts to `off` even when the process is configured for enforcement. It is not process-global and must not fall back to a legacy or primary runtime when workspace ownership is unknown. The marker is client-asserted ACP metadata, so another client can -also opt its prompt out of this conservative protection; it is a routing trust -signal, not an authorization boundary. +also opt its prompt out of this conservative protection; it is a routing hint, +not a trust signal or authorization boundary. Modes: @@ -400,6 +414,13 @@ Shadow mode advances a virtual warned state without injecting the reminder, so `would_warn` and `would_stop` estimate volume only. They cannot establish how a model behaves after seeing the reminder. +Before the baseline starts, each deployment must configure a stable +OpenTelemetry Resource attribute such as `deployment.environment` that +distinguishes internal from public cloud. The SDK always supplies +`service.version`; it deliberately does not invent a deployment environment. +Without that Resource dimension, Phase 1 cannot produce a per-environment +conclusion and rollout must not advance. + The default shadow mode does not change model continuation or injected messages. It does add `todoStopGuardWatchQueuedPrompt: true` to the existing mid-turn drain request so the reducer can prove that no full prompt is queued. @@ -500,9 +521,11 @@ Automated unit tests for the pure reducer cover: - eight failures in one batch do not warn; - eight failures across two batches warn; - the next matching batch stops only after it settles; -- success, cancellation, `not_started`, `unknown`, post-processing failure, - mixed keys, incomplete batch, unreliable drain, queued prompt, and new input - reset; +- same-tool success, cancellation, `not_started`, `unknown`, post-processing + failure, mixed failure keys, incomplete batch, unreliable drain, queued + prompt, and new input reset; +- successes from other tools are ignored both within a matching failure batch + and in intervening successful batches; - duplicate provider events are ignored rather than counted or reset; - a new key starts a new streak; - warning and stop text are fixed and contain no tool data; and diff --git a/packages/channels/base/src/ChannelAgentBridge.ts b/packages/channels/base/src/ChannelAgentBridge.ts index f5b5464e94c..53bf9609351 100644 --- a/packages/channels/base/src/ChannelAgentBridge.ts +++ b/packages/channels/base/src/ChannelAgentBridge.ts @@ -3,6 +3,7 @@ import type { RequestPermissionResponse, } from '@agentclientprotocol/sdk'; +// Client-supplied routing hint only; never use it as an authorization boundary. export const CHANNEL_PROMPT_META_KEY = 'qwen.channel.prompt'; export interface AvailableCommand { diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index db6c863d63b..85997847f82 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -7126,7 +7126,7 @@ describe('Session', () => { expect(mockClient.extMethod).toHaveBeenCalledTimes(1); expect( logRepeatedToolFailureGuardSpy.mock.calls.some( - ([, event]) => event.reset_reason === 'unreliable_input', + ([event]) => event.reset_reason === 'unreliable_input', ), ).toBe(true); expect(logLoopDetectedSpy).not.toHaveBeenCalledWith( @@ -7162,7 +7162,7 @@ describe('Session', () => { expect(mockChat.sendMessageStream).toHaveBeenCalledTimes(4); expect( logRepeatedToolFailureGuardSpy.mock.calls.some( - ([, event]) => event.reset_reason === 'queued_prompt', + ([event]) => event.reset_reason === 'queued_prompt', ), ).toBe(true); expect(sentText()).not.toContainEqual( @@ -7236,7 +7236,7 @@ describe('Session', () => { ); const telemetryPromptIds = logRepeatedToolFailureGuardSpy.mock.calls.map( - ([, event]) => event.prompt_id, + ([event]) => event.prompt_id, ); expect(new Set(telemetryPromptIds).size).toBe(1); expect(telemetryPromptIds[0]).toBe('test-session-id########1'); @@ -7277,6 +7277,12 @@ describe('Session', () => { try { const execute = installFailingTool(); let drainCount = 0; + const reminder = + 'unfinished todo: inspect the failure'; + vi.mocked(mockConfig.takeActiveTodoReminder).mockImplementation( + (_promptId, force = false) => + !force && drainCount === 3 ? reminder : undefined, + ); mockClient.extMethod = vi.fn().mockImplementation(async () => { drainCount++; if (drainCount === 3) { @@ -7301,6 +7307,10 @@ describe('Session', () => { loop_type: core.LoopType.REPEATED_TOOL_EXECUTION_FAILURE, }), ); + expect(mockChat.addHistory).toHaveBeenCalledWith({ + role: 'user', + parts: expect.arrayContaining([{ text: reminder }]), + }); } finally { restoreGuardMode(); } diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 25c23978423..1b59e586f99 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -522,7 +522,6 @@ function repeatedToolFailureBatchBucket(count: number): '0' | '1' | '2' | '3+' { } function recordRepeatedToolFailureDecision( - config: Config, promptId: string, mode: RepeatedToolFailureGuardMode, previousState: RepeatedToolFailureGuardState, @@ -557,39 +556,29 @@ function recordRepeatedToolFailureDecision( const toolType = matchingToolTypes.size === 1 ? [...matchingToolTypes][0] : undefined; - try { - logRepeatedToolFailureGuard( - config, - new RepeatedToolFailureGuardEvent({ - prompt_id: promptId, - route: 'acp_foreground', - mode, - phase_before: previousState.phase, - phase_after: decision.state.phase, - decision: telemetryDecision, - failure_count_bucket: repeatedToolFailureCountBucket( - countState.failureCount, - ), - batch_count_bucket: repeatedToolFailureBatchBucket( - countState.batchCount, - ), - candidate_ordinal: countState.candidateOrdinal, - ...(decision.kind === 'reset' - ? { reset_reason: decision.reason } - : { - terminal_status: 'error', - execution_status: 'error', - execution_error_type: key?.executionErrorType, - tool_type: toolType, - }), - }), - ); - } catch (error) { - debugLogger.debug( - '[repeated-tool-failure-guard] Failed to record telemetry', - error, - ); - } + logRepeatedToolFailureGuard( + new RepeatedToolFailureGuardEvent({ + prompt_id: promptId, + route: 'acp_foreground', + mode, + phase_before: previousState.phase, + phase_after: decision.state.phase, + decision: telemetryDecision, + failure_count_bucket: repeatedToolFailureCountBucket( + countState.failureCount, + ), + batch_count_bucket: repeatedToolFailureBatchBucket(countState.batchCount), + candidate_ordinal: countState.candidateOrdinal, + ...(decision.kind === 'reset' + ? { reset_reason: decision.reason } + : { + terminal_status: 'error', + execution_status: 'error', + execution_error_type: key?.executionErrorType, + tool_type: toolType, + }), + }), + ); } function recordDaemonLoopDetected( @@ -5205,11 +5194,16 @@ export class Session implements SessionContext { if (hadMidTurnUserInput) { this.todoStopGuard.acceptMidTurnUserInput(); } + const activeTodoReminder = this.config.takeActiveTodoReminder(promptId); if (abortSignal.aborted) { return { message: { role: 'user', - parts: [...toolRun.parts, ...drained.parts], + parts: [ + ...toolRun.parts, + ...(activeTodoReminder ? [{ text: activeTodoReminder }] : []), + ...drained.parts, + ], }, hadMidTurnUserInput, }; @@ -5231,7 +5225,6 @@ export class Session implements SessionContext { }, ); recordRepeatedToolFailureDecision( - this.config, promptId, toolLoopState.repeatedToolFailureMode, previousRepeatedToolFailureState, @@ -5245,7 +5238,6 @@ export class Session implements SessionContext { `[repeated-tool-failure-guard] mode=${toolLoopState.repeatedToolFailureMode} decision=${repeatedToolFailureDecision.kind} phase=${state.phase} candidate=${state.candidateOrdinal} failures=${state.failureCount} batches=${state.batchCount}`, ); } - const activeTodoReminder = this.config.takeActiveTodoReminder(promptId); const parts = [ ...toolRun.parts, ...(activeTodoReminder ? [{ text: activeTodoReminder }] : []), diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts index 39de3008fc5..77dec7b1494 100644 --- a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.test.ts @@ -180,6 +180,154 @@ describe('repeated tool failure guard', () => { }, ); + it('keeps counting a failure key when other tools succeed in the same batch', () => { + const failingShell = (callId: string) => + observation({ + callId, + policyToolName: 'run_shell_command', + executionErrorType: ToolErrorType.EXECUTION_FAILED, + }); + const first = reduce( + createRepeatedToolFailureGuardState(), + Array.from({ length: 4 }, (_, index) => failingShell(`first-${index}`)), + ); + const second = reduce(first.state, [ + ...Array.from({ length: 4 }, (_, index) => + failingShell(`second-${index}`), + ), + observation({ + callId: 'read-success', + policyToolName: 'read_file', + terminalStatus: 'success', + executionStatus: 'success', + executionErrorType: undefined, + }), + ]); + + expect(second).toMatchObject({ + kind: 'warn', + state: { failureCount: 8, batchCount: 2 }, + }); + }); + + it('preserves a failure streak across successful batches from other tools', () => { + const failingShell = (callId: string) => + observation({ + callId, + policyToolName: 'run_shell_command', + executionErrorType: ToolErrorType.EXECUTION_FAILED, + }); + const first = reduce( + createRepeatedToolFailureGuardState(), + Array.from({ length: 4 }, (_, index) => failingShell(`first-${index}`)), + ); + const unrelatedSuccess = reduce(first.state, [ + observation({ + callId: 'edit-success', + policyToolName: 'replace', + terminalStatus: 'success', + executionStatus: 'success', + executionErrorType: undefined, + }), + ]); + const second = reduce( + unrelatedSuccess.state, + Array.from({ length: 4 }, (_, index) => failingShell(`second-${index}`)), + ); + + expect(unrelatedSuccess).toEqual({ kind: 'none', state: first.state }); + expect(second).toMatchObject({ + kind: 'warn', + state: { failureCount: 8, batchCount: 2 }, + }); + }); + + it('resets when the failing tool also succeeds in the same batch', () => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const result = reduce(tracked.state, [ + observation({ callId: 'failure' }), + observation({ + callId: 'success', + terminalStatus: 'success', + executionStatus: 'success', + executionErrorType: undefined, + }), + ]); + + expect(result).toMatchObject({ kind: 'reset', reason: 'success' }); + }); + + it('does not let another tool self-recovery reset the tracked candidate', () => { + const tracked = reduce(createRepeatedToolFailureGuardState(), [ + observation(), + ]); + const result = reduce(tracked.state, [ + observation({ + callId: 'other-failure', + policyToolName: 'run_shell_command', + executionErrorType: ToolErrorType.EXECUTION_FAILED, + }), + observation({ + callId: 'other-success', + policyToolName: 'run_shell_command', + terminalStatus: 'success', + executionStatus: 'success', + executionErrorType: undefined, + }), + ]); + + expect(result).toEqual({ kind: 'none', state: tracked.state }); + }); + + it('counts one failure key when another failing tool succeeds in the batch', () => { + const result = reduce(createRepeatedToolFailureGuardState(), [ + observation({ callId: 'tracked-failure' }), + observation({ + callId: 'other-failure', + policyToolName: 'run_shell_command', + executionErrorType: ToolErrorType.EXECUTION_FAILED, + }), + observation({ + callId: 'other-success', + policyToolName: 'run_shell_command', + terminalStatus: 'success', + executionStatus: 'success', + executionErrorType: undefined, + }), + ]); + + expect(result).toMatchObject({ + kind: 'tracked', + state: { + key: { + policyToolName: 'read_file', + executionErrorType: ToolErrorType.FILE_NOT_FOUND, + }, + failureCount: 1, + batchCount: 1, + }, + }); + }); + + it('downgrades enforcement for a successful observation without tool identity', () => { + const result = reduce(createRepeatedToolFailureGuardState(), [ + observation({ + policyToolName: undefined, + terminalStatus: 'success', + executionStatus: 'success', + executionErrorType: undefined, + }), + ]); + + expect(result).toMatchObject({ + kind: 'reset', + reason: 'unknown', + state: { enforcementDisabled: true }, + }); + }); + it('ignores provider duplicates without advancing or resetting', () => { const tracked = reduce(createRepeatedToolFailureGuardState(), [ observation(), diff --git a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts index 9d8ae710226..731fe1a2845 100644 --- a/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts +++ b/packages/cli/src/acp-integration/session/repeated-tool-failure-guard.ts @@ -84,7 +84,6 @@ const INELIGIBLE_RESET_REASON_PRECEDENCE = [ 'unknown', 'not_started', 'post_execution_failure', - 'success', ] as const satisfies readonly RepeatedToolFailureResetReason[]; export type RepeatedToolFailureGuardDecision = @@ -208,6 +207,7 @@ export function reduceRepeatedToolFailureGuard( } const eligible: FailureKey[] = []; + const successfulPolicyToolNames = new Set(); const resetReasons = new Set(); let enforcementDisabled = state.enforcementDisabled; for (const observation of observations) { @@ -229,7 +229,12 @@ export function reduceRepeatedToolFailureGuard( executionStatus === 'success' || executionStatus === 'not_started' ) { - resetReasons.add('success'); + if (observation.policyToolName) { + successfulPolicyToolNames.add(observation.policyToolName); + } else { + resetReasons.add('unknown'); + enforcementDisabled = true; + } } else { resetReasons.add('unknown'); enforcementDisabled = true; @@ -275,11 +280,20 @@ export function reduceRepeatedToolFailureGuard( return reset(state, resetReason, enforcementDisabled); } - const key = eligible[0]; + const matchingFailures = eligible.filter( + (failure) => !successfulPolicyToolNames.has(failure.policyToolName), + ); + const key = matchingFailures[0]; if (!key) { + if (state.key && successfulPolicyToolNames.has(state.key.policyToolName)) { + return reset(state, 'success'); + } + if (successfulPolicyToolNames.size > 0) { + return { kind: 'none', state }; + } return reset(state, 'unknown', true); } - if (eligible.some((entry) => !keysEqual(entry, key))) { + if (matchingFailures.some((entry) => !keysEqual(entry, key))) { return reset(state, 'mixed'); } @@ -287,7 +301,7 @@ export function reduceRepeatedToolFailureGuard( const nextState: RepeatedToolFailureGuardState = { phase: 'tracking', key, - failureCount: eligible.length, + failureCount: matchingFailures.length, batchCount: 1, candidateOrdinal: state.nextCandidateOrdinal, nextCandidateOrdinal: state.nextCandidateOrdinal + 1, @@ -296,7 +310,7 @@ export function reduceRepeatedToolFailureGuard( return { kind: 'tracked', state: nextState }; } - const failureCount = state.failureCount + eligible.length; + const failureCount = state.failureCount + matchingFailures.length; const batchCount = state.batchCount + 1; if (state.phase === 'warned') { const nextState: RepeatedToolFailureGuardState = { diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index ac65d702fb8..146b7192f91 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -355,7 +355,6 @@ describe('loggers', () => { describe('logRepeatedToolFailureGuard', () => { it('emits a data-minimized transition log and low-cardinality metric', () => { - const config = makeFakeConfig({ sessionId: 'test-session-id' }); vi.spyOn( metrics, 'recordRepeatedToolFailureGuardMetrics', @@ -376,7 +375,7 @@ describe('loggers', () => { tool_type: 'mcp', }); - logRepeatedToolFailureGuard(config, event); + logRepeatedToolFailureGuard(event); expect(mockLogger.emit).toHaveBeenCalledWith({ body: 'Repeated tool failure guard decision: would_warn.', @@ -406,7 +405,6 @@ describe('loggers', () => { }); it('isolates transition log and metric sink failures', () => { - const config = makeFakeConfig({ sessionId: 'test-session-id' }); const event = new RepeatedToolFailureGuardEvent({ prompt_id: 'prompt-id', route: 'acp_foreground', @@ -428,7 +426,7 @@ describe('loggers', () => { throw new Error('log unavailable'); }); - expect(() => logRepeatedToolFailureGuard(config, event)).not.toThrow(); + expect(() => logRepeatedToolFailureGuard(event)).not.toThrow(); expect(event).not.toHaveProperty('reset_reason'); expect(event).not.toHaveProperty('terminal_status'); expect(event).not.toHaveProperty('execution_status'); diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index a39f61c1650..23d826c3071 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -659,9 +659,10 @@ export function logLoopDetected( } export function logRepeatedToolFailureGuard( - _config: Config, event: RepeatedToolFailureGuardEvent, ): void { + // Deployment cohort and service version come from the OpenTelemetry + // Resource, which is attached to both the logger and meter providers. runToolTelemetrySink(() => { if (isTelemetrySdkInitialized()) { const logger = logs.getLogger(SERVICE_NAME); From 41289f6c8960421240bd234f9dbf9bb16834d2b6 Mon Sep 17 00:00:00 2001 From: "jinye.djy" Date: Sun, 9 Aug 2026 16:41:31 +0800 Subject: [PATCH 5/5] docs(acp): clarify review and rollout gates --- .../acp-repeated-tool-call-protection.md | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/design/acp-repeated-tool-call-protection.md b/docs/design/acp-repeated-tool-call-protection.md index 1d6abe4c671..b53dd2b74b5 100644 --- a/docs/design/acp-repeated-tool-call-protection.md +++ b/docs/design/acp-repeated-tool-call-protection.md @@ -1,7 +1,8 @@ # ACP Repeated Tool-Call Protection Date: 2026-07-31 -Status: Implemented, pending shadow rollout; revised for PR #8176 and PR #8180 +Status: Implemented on the PR branch; pre-merge manual ACP validation and post-merge shadow rollout pending +Revision: Updated for PR #8176 and PR #8180 Area: Interactive ACP foreground prompt loop ## Summary @@ -551,12 +552,17 @@ Telemetry tests cover: preserving standard OpenTelemetry correlation fields, plus exclusion of sensitive tool fields from guard-specific telemetry. -The behavioral change also has an E2E plan under `.qwen/e2e-tests/`. Its manual -ACP fixture run remains required before promotion out of Draft. It covers one -typed failing tool, permission cancellation, successful recovery after the -reminder, a repeated failure that stops, concurrent siblings, unsupported -hosts, channel exclusion, reconnect and restart behavior, history replay, a -fresh prompt after stop, and both internal and public-cloud policy modes. +The behavioral change will use a local E2E plan under `.qwen/e2e-tests/`. +Preparing that plan and completing its manual ACP fixture run are still pending +and are required before merge. Ready for review means maintainer review may +proceed; it does not claim that the pre-merge manual ACP fixture validation is +complete. The fixture must cover one typed failing tool, permission +cancellation, successful recovery after the reminder, a repeated failure that +stops, concurrent siblings, unsupported hosts, channel exclusion, reconnect and +restart behavior, history replay, a fresh prompt after stop, and shadow-mode +non-interference. Separate seven-day internal and public-cloud +shadow baselines start after merge and deployment; they gate later promotion to +warn or enforce rather than merge. Before delivery, run targeted Core and CLI Vitest files from their package directories, then `npm run build`, `npm run typecheck`, and `npm run lint`.