3 Commits

Author SHA1 Message Date
alanhuangyoo
0f2813f6b9
fix(agents): keep queued guard warnings when a model call is retried (#5433)
* fix(agents): keep queued guard warnings when a model call is retried

LoopDetectionMiddleware, TokenBudgetMiddleware and ToolProgressMiddleware
pop their queued warning/hint before calling the model. When the call
raises, LLMErrorHandlingMiddleware (outside them) retries by running
their wrap_model_call again, and by then the queue is empty, so the
retried request goes out without the warning. Loop detection and the
token budget have already marked it as sent, so it is never queued
again, and a loop runs on to the hard stop unwarned.

Put the drained items back in front of the queue when the handler
raises, then re-raise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(agents): trim restored loop warnings from the tail and drop a dead helper

_restore_pending_warnings put the restored warnings at the front and then
trimmed the front, so if the cap ever fired it would drop exactly what it
restored. Trim the tail, as tool progress does. _augment_request had no
callers after the wrap_model_call change. Add the sync twin of the tool
progress retry test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* refactor(agents): drop tool progress's unused _augment_request

Its only remaining reference was a test name; the dedup that test checks lives
in _inject_hints.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-09-16 15:52:17 +08:00
PeaceMaker-best
14c9d44440
feat(runtime): persist tool-progress phase transitions (#5214)
* feat(runtime): persist tool-progress phase transitions

Record bounded warn, block, and recover decisions for lead and task subagent runs while preserving event-loop isolation, fail-open behavior, and concurrent transition order.

* fix(runtime): trust server-owned tool progress attribution

* fix(runtime): centralize trusted audit attribution

* fix(runtime): preserve complete tool progress audit state

* docs: trim tool progress guidance to pass size check

* fix(runtime): fence subagent audit recorder loop

* docs(readme): sync tool-progress event coverage

Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com>

---------

Signed-off-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com>
Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com>
Co-authored-by: 嗜鵼 <hy2010hy2010@qq.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-09-15 08:45:08 +08:00
He Wang
fd41fdb065
feat(middleware): add structured tool result meta and tool-progress state machine (#3601)
* feat(middleware): add structured tool result meta and tool-progress state machine

feat:
- Add tool_result_meta.py: ToolResultMeta dataclass (status/error_type/retryable/
  recoverable_by_model/recommended_next_action/source) + normalize_tool_result and
  stamp_exception_meta utilities; classifies every ToolMessage regardless of path
- Add ToolProgressMiddleware: per-(thread_id, tool_name) state machine ACTIVE →
  WARNED (hint injected as HumanMessage) → BLOCKED (call short-circuited); Jaccard
  near-duplicate detection for repeated successful results; auth/config/internal
  errors bypass WARNED and go directly to BLOCKED; LRU-bounded thread state store
- Add ToolProgressConfig: all thresholds configurable (stagnation_threshold,
  warn_escalation_count, jaccard_similarity_threshold, exempt_tools, etc.);
  disabled by default (enabled: false)
- Wire ToolProgressMiddleware as outer wrapper around ToolErrorHandlingMiddleware
  in _build_runtime_middlewares so it receives results already carrying
  deerflow_tool_meta

fix:
- ToolErrorHandlingMiddleware now calls stamp_exception_meta on exception path and
  normalize_tool_result on success path so every ToolMessage carries deerflow_tool_meta

test:
- Add test_tool_result_meta.py: 26 cases covering all classification paths,
  stamp_exception_meta, and normalize_tool_result Command passthrough
- Add test_tool_progress_middleware.py: 27 cases including full async paths,
  Jaccard duplicate detection, LRU eviction, hint injection, and malformed meta
  passthrough
- Extend test_tool_error_handling_middleware.py: middleware ordering invariant and
  meta stamping on exception

docs:
- Add tool_progress section to config.example.yaml with all fields and descriptions
- Update CLAUDE.md middleware chain documentation (entries 8-9)

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

* fix(middleware): recoverable errors stay WARNED; fix auth keyword shadowing

fix:
- WARNED is terminal for recoverable_by_model=True errors (no_results, not_found,
  permission); hint re-injected on each problem call instead of escalating to
  BLOCKED, so the model can retry with different parameters (e.g. fresh query,
  new URL) without being hard-blocked by a prior stagnation count.
  Non-recoverable (rate_limited, transient) still escalate WARNED → BLOCKED
  after warn_escalation_count more problems; auth/config/internal remain
  immediately BLOCKED.
- Remove bare "api key" keyword from auth classification rule so "no api key
  configured" correctly classifies as config (not auth), producing the accurate
  block-reason text for the model.

docs:
- CLAUDE.md: document all three ToolProgressMiddleware transition paths
- config.example.yaml: update inline state-machine comment to match new paths

test:
- test_recoverable_errors_stay_warned_indefinitely: WARNED never escalates for
  recoverable errors regardless of how many problem calls accumulate
- test_recoverable_error_re_injects_hint_past_escalation: hints continue past
  the escalation zone for recoverable errors
- test_no_api_key_is_config_not_auth: regression guard for keyword shadowing fix

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

* fix(tool_result_meta): add JSON error extraction and fix source classification

fix:
- Fix non-standard error path: source was "exception" but should be "tool_return"
- Add _extract_json_error_text to isolate JSON error fields from noisy JSON bodies
  (e.g. Brave Search {"error": "...", "query": "..."} — query keywords no longer
  pollute error classification)
- Add success-path JSON extraction to catch tools that return HTTP 200 with a JSON
  error body (status="success" but {"error": "API key not configured"})
- Add _SEMANTIC_ZERO_ERROR_STRINGS frozenset to suppress false positives from tools
  that use {"error": "none"} / {"error": "null"} / {"error": "ok"} as success signals
- Document that stamp_exception_meta always overwrites existing TOOL_META_KEY
  (exception-derived classification is authoritative over tool return-time stamps)

test:
- Add parametrized regression tests for all semantic-zero error strings
- Add tests for non-standard error path source field
- Add tests for JSON error extraction (nonstd, success-path, numeric, falsy values)
- Correct test comment for test_no_api_key_is_config_not_auth

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

* fix(tool_progress_middleware): fix 6 bugs, add terminal guard and structured logging

fix:
- H1: fix exempt_tools empty-set silently ignored — use `is not None` instead of
  truthiness check so ToolProgressConfig(exempt_tools=set()) correctly disables all
  exemptions
- Fix _get_block_reason creating phantom LRU entries via _get_state (write path);
  now uses dict.get + explicit move_to_end on read path only
- Fix _pending memory leak: LRU eviction of _phase_states now synchronously removes
  all (evicted_thread, *) keys from _pending
- Fix _assess_and_transition missing terminal guard for blocked state — a recoverable
  error result could silently demote blocked → warned in concurrent-race scenarios;
  early return preserves terminal semantics
- Fix recent_word_sets window: stored [-5:] but is_near_duplicate only compared [-3:];
  align to [-3:] and change type list→tuple (prevents accidental in-place mutation
  across dataclasses.replace shallow copies)
- Fix _format_hint missing "success" key and "continue" action: Jaccard near-duplicate
  results produced the generic fallback instead of a specific actionable message

feat:
- Add structured state-transition logging (ACTIVE/WARNED/BLOCKED transitions, blocked
  intercepts, hint injection debug log)

test:
- Add regression tests for all 6 bug fixes (H1, phantom LRU, pending leak, terminal
  guard, window alignment, format_hint near-dup)
- Add Jaccard near-threshold boundary test (7/9 vs 8/9 Jaccard)
- Add production min_words=10 skip test for short content
- Add exempt_tools empty-set and None round-trip tests
- Add _augment_request deduplication test
- Add before_agent current-run preservation test
- Add structured logging tests (WARNED/BLOCKED/ACTIVE/intercepted/debug)

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

* chore(config): remove unused backward-compat fields from ToolProgressConfig

Remove max_calls_per_intent and window_size fields that were marked
"Retained for backward compatibility; not used by the current state machine"
when the state machine was introduced.  Pydantic v2 ignores unknown fields
by default, so existing config.yaml files with these keys remain valid.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>

* fix(tool_progress): address PR review and multi-agent review findings

fix:
- Remove <80-char length gate for partial_success; only _PARTIAL_MARKERS now
- Add word-boundary regex for numeric HTTP codes (401/403/404/500) to avoid
  false positives like "500ms" or "4010 rows" triggering hard-block
- Add "task" to default exempt_tools (delegation primitive, not a search tool)
- Remove move_to_end() from _get_block_reason read path; blocked threads were
  permanently warm in LRU, starving active threads of eviction slots
- Add _reset_blocked_states in before_agent: scope BLOCKED and WARNED states
  to a single run; clear recent_word_sets so stale Jaccard windows don't cause
  false near-duplicate detections in the next run
- Compute word_set() lazily (only for success results); cap content at 8192
  chars to bound memory and CPU cost on large tool results
- Remove unused retryable field from ToolResultMeta (no consumer existed)
- Add isinstance-based ordering guard and warning log for missing meta
- Fix JSON-without-error-key fallback: use _UNKNOWN_ERROR instead of
  classifying incidental field values (e.g. {"user_id": 401} → auth → stop)
- Fix _extract_json_error_text: use json.dumps for dict/list error fields
  instead of str() which produced Python repr matching config rules spuriously
- Add "no results found"/"no content found"/"no images found" to _PARTIAL_MARKERS
  so success responses with empty results trigger stagnation detection
- Fix immediate-block path to increment consecutive_problems (was left at 0)
- Fix _queue_assessment: skip phantom _pending entries for evicted threads
- Bump config_version 13→16 (upstream added 14/15; tool_progress is additive)

test:
- Update test_short_content_is_partial → test_short_terse_success_is_not_partial
- Add parametrized test_numeric_keyword_word_boundary (8 positive + negative cases)
- Add test_before_agent_resets_blocked_states_for_new_run (strengthened assertions)
- Add test_before_agent_resets_warned_states_for_new_run
- Add test_missing_meta_on_non_exempt_tool_emits_warning
- Add test_middleware_ordering_guard_raises_when_progress_is_inner
- Add test_auth_error_immediately_blocked asserts consecutive_problems == 1
- Add tests for JSON-without-error-key, dict error field, no-results partial_success

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(tool_progress): address second PR review — perf, architecture doc, concurrency note

fix:
- Extract content.lower() once before _PARTIAL_MARKERS check in normalize_tool_message;
  previously computed up to 7× per call inside the generator (once per marker)

docs:
- Add division-of-labor paragraph to ToolProgressMiddleware module docstring explaining
  coexistence with LoopDetectionMiddleware: result-quality guard (per-tool BLOCK) vs
  call-pattern guard (whole-turn hard-stop); no shared state, no double-stop risk
- Add threading.Lock comment explaining why asyncio.Lock is not used (short critical
  sections, must also protect sync wrap_tool_call path from subagent executor threads)
- Update backend/CLAUDE.md entry 8 with division-of-labor summary; fix entry 9
  (remove stale retryable field reference, add missing recoverable_by_model/source)

test:
- Add test_tool_progress_and_loop_detection_coexist_without_interfering: drives both
  middlewares to WARNED state simultaneously, verifies independent state, independent
  hint queues, and no cross-contamination; uses snapshot copy for final assertion

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(tool_progress): reset all tool states at run boundary; fix semantic-zero test validity

fix:
- _reset_run_states (formerly _reset_blocked_states) drops the phase filter and
  resets all tracked (thread, tool) pairs unconditionally at before_agent; ACTIVE
  tools with sub-threshold consecutive_problems or cached recent_word_sets no longer
  bleed into the next run, preventing spurious WARNED transitions on clean R2 calls
- test_normalize_json_semantic_zero_error_string_not_treated_as_error: replace
  {error_value!r} f-string (produces invalid JSON with single quotes) with
  json.dumps so _extract_json_error_text actually parses the payload and the
  _SEMANTIC_ZERO_ERROR_STRINGS guard is exercised, not bypassed at json.loads

test:
- add test_before_agent_resets_active_state_consecutive_problems_and_word_sets to
  lock the ACTIVE-phase run-boundary reset: drives tool to active/cp=1/ws≠() in R1,
  asserts both fields are zero/empty after before_agent fires for R2

* docs(tool_progress): document intentional per-run reset vs LoopDetection thread-scoped retention

Addresses reviewer observation in PR #3601 that _reset_run_states diverges
from LoopDetectionMiddleware's cross-run scoping policy without explanation.

Expands the _reset_run_states docstring to record the intentional design
choice: ToolProgressMiddleware resets per-run because result-quality errors
(rate_limited, transient) are time-bound and may resolve between turns —
retaining stale counters would risk false-positive BLOCKED calls.
LoopDetectionMiddleware retains history across runs because call-pattern
loops are time-invariant. The divergence is by design, not oversight.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(middleware): restore ReadBeforeWriteMiddleware as outermost write gate

A merge conflict resolution had accidentally placed ReadBeforeWriteMiddleware
after ToolErrorHandlingMiddleware (inner), reversing the original intent from
b81334cc where it was the outermost write gate before ToolErrorHandling.

fix:
- Restore ReadBeforeWriteMiddleware to outer position: ReadBeforeWrite →
  ToolProgress → ToolErrorHandling. Blocked writes now return immediately
  without consuming a ToolProgress slot.
- Add normalize_tool_result call on blocked ToolMessages so they carry
  deerflow_tool_meta (recoverable_by_model=True) even though they bypass
  ToolErrorHandlingMiddleware.

test:
- Add test_blocked_write_has_deerflow_tool_meta (sync + async) to lock the
  normalize_tool_result behavior on blocked writes.
- Fix chain order assertions in TestChainWiring and
  test_build_lead_runtime_middlewares_chain_order_matches_agents_md.

docs:
- Renumber AGENTS.md items: 10→ReadBeforeWrite, 11→ToolProgress,
  12→ToolErrorHandling; update descriptions to reflect outermost-gate design.
- Fix stale cross-reference: LoopDetectionMiddleware (item 23) → (item 25).

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-07-06 20:57:41 +08:00