fix(desktop): treat interrupted completions as metadata - #63292
fix(desktop): treat interrupted completions as metadata#63292yingliang-zhang wants to merge 101 commits into
Conversation
26b1862 to
c1d79bf
Compare
|
Rebased onto current main lineage and resolved the interruption-path conflicts. Follow-up |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the cancellation path across the core loop and Desktop. The premise is confirmed on current main: agent/conversation_loop.py:2352 returns interruption prose when no text streamed, and Desktop currently treats every message.complete as successful at apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts:384-403.
Problems
agent/agent_runtime_helpers.py:2670insertsOperation interrupted.for everytool → useradjacency, with no interruption provenance. Current main explicitly treats anassistant(tool_calls) → tool → usersequence as a valid normal redirect (agent/agent_runtime_helpers.py:394-399;tests/run_agent/test_message_sequence_repair.py:108-129). This would inject false cancellation context into the next model request for that valid path.
Suggested changes
- Scope API-only closure to an explicitly cancellation-originated tail, and add a negative sanitizer test for the normal redirect sequence.
Automated hermes-sweeper review.
2e1003e to
ae5dd4f
Compare
24dbf94 to
f4fd162
Compare
9ef5f2d to
27463c4
Compare
27463c4 to
2078a62
Compare
2078a62 to
b1ba8ba
Compare
…side import block)
…ith hybrid-routing-v1 callers)
The union merge dropped the single call-site wiring line for the already-configured option (patch 360's only material gap); compress options read the value but never passed it.
…hropic wire MiniMax-M3 rides MiniMax's server-side automatic prefix cache; explicit cache_control is documented for the M2 family only, so markers on M3 are dead weight (cache_creation always 0, never billable). Restore the exclusion (provider-id or host match) checked BEFORE the native-Anthropic return, so provider="anthropic" pointed at a MiniMax /anthropic proxy is also caught. The later M2 opt-in reuses the hoisted route predicate. Fixes tests/run_agent/test_anthropic_prompt_cache_policy.py ::TestMiniMaxAnthropicWire (4 candidate-only failures).
switch_model snapshots _reasoning_echo_flag into _primary_runtime and a fallback entry overwrites the live flag, but restore_primary_runtime never reverted it, so the fallback's opt-in leaked into the restored primary. Restore the flag from the snapshot (default False for snapshots taken before the field existed). Also tolerate a missing context_compressor when rebinding context-engine state; gateway/CLI paths may construct it lazily. Fixes tests/agent/test_message_sanitization_policy.py ::TestPerProviderReasoningEcho::test_restore_primary_reverts_flag.
With the inline dispatch branch amputated, setup_mcp escaped to handle_function_call while the post-hook ownership predicate also dropped its membership, breaking the emit-once contract on both executor paths. Restore the invoke_tool branch and the AGENT_RUNTIME_POST_HOOK_TOOL_NAMES entry. Fixes tests/run_agent/test_run_agent.py::TestAgentRuntimePostHookOwnershipSync (2 candidate-only failures).
…ach, log close reason Three restorations for the slash-worker lifecycle: - Keep references to the drain threads (named slash-drain-stdout / slash-drain-stderr for leaked-thread attribution) and join them with a bounded 2s timeout in close(); previously each closed session leaked two daemon threads holding worker references (NousResearch#53303). - _close_sessions_for_transport now closes the session's slash worker immediately when detaching a non-close_on_disconnect session instead of letting it linger until the orphan/TTL reaper; tolerates worker.close() failure and a missing history_lock. The worker recreates lazily on next use. - _close_session_by_id logs "session closed: <sid> (end_reason=...)" at INFO; the teardown path was silent, making reap behavior undiagnosable. Fixes tests/tui_gateway/test_slash_worker_drain.py (2) and tests/tui_gateway/test_slash_worker_detach.py (3).
switch_model refreshed only model metadata on the compressor, so a move onto a custom codex_responses route never applied the per-route Codex threshold autoraise (0.85) and moving back never restored the preserved config baseline. Resolve the new route's threshold the same way agent_init does at startup — _compression_threshold_for_model gated by _codex_gpt55_autoraise, combined with the raise-only resolver semantics — and pass it through update_model(default_threshold_percent=...). The agent-side _compression_global_threshold attribute is optional; the compressor's immutable config baseline substitutes when the agent wasn't fully constructed. Also tolerate partially-initialised ContextCompressor instances in _effective_max_tail_message_floor (fixtures built via __new__ set only the attributes under test). Fixes tests/run_agent/test_switch_model_context.py ::test_switch_model_custom_codex_threshold_uses_resolved_window, tests/agent/test_context_compressor_cross_session_guard.py (3) and tests/agent/test_compressor_image_tokens.py ::TestTokenBudgetWithImages::test_image_heavy_turns_count_toward_budget.
…d tool tails on the wire Restoration of three lost behaviors around durable prompt/transcript reuse: 1. _restore_or_build_system_prompt reused the persisted prompt verbatim but never seeded the frozen plugin-section snapshot, so the first invalidate/rebuild in the resumed process re-evaluated section callbacks and rewrote prompt bytes, breaking the byte-identical-resume contract and the cache prefix. Seed the snapshot from the persisted prompt; section callbacks evaluate only on new sessions. 2. sanitize_api_messages lost the NousResearch#48879/NousResearch#63292 closure pass: a durable _interrupted_tool_tail tool row followed by a user redirect reached the wire as a bare tool -> user alternation, which strict providers reject or hallucinate against. Close the sequence on the per-call API copy with the interim "Operation interrupted." assistant marker, and strip the internal provenance key from the wire copy. 3. Re-align the NousResearch#68454 rotation-flush control test with this stack's schema: the partial UNIQUE index idx_messages_active_dedupe(session_id, role, content, timestamp) WHERE active=1 makes a bare flush of unstamped cold-resume rows a DB-level no-op, so the control now forbids duplicates instead of asserting the upstream double-write. Production is unchanged; the boundary/no-op and tail-only sibling tests are untouched. Fixes tests/agent/test_plugin_prompt_sections.py ::test_fresh_process_resume_restores_identical_full_prompt_without_callback, tests/agent/test_session_rotation_flush_cold_resume_68454.py ::test_rotation_flush_without_history_boundary_duplicates, tests/agent/test_interrupt_tool_tail_api_sanitization.py ::test_user_after_interrupted_tool_tail_is_closed_only_in_api_copy, and tests/agent/test_turn_finalizer_interrupt_alternation.py ::test_interrupt_after_tool_keeps_transcript_clean_and_closes_api_copy.
…e-boundary submits The 63298 queue-boundary contract emits `queued: true` on re-submission and rebinds the queued entry's session identity to the recovered runtime before the first visible submit; update the two stale assertions (3 session.resume calls, recovered id on both submits).
The runtime updater pipeline's hindsight_post_sync verifier ran fully on the promoted branch state but rejected the 0.6.1 pin; bumping to the same 0.8.4 the server env already carries.
…lersistence) uv sync --locked prunes any package without a lock entry; main-venv embed package must be part of the resolving graph to survive dependency_sync before hindsight_post_sync.
…ddedClient) Restores plugins/memory/hindsight/embedded_runtime.py from the P0 hindsight-server isolation lineage (1660e5528). Required by the updater's verify_hindsight_runtime runtime-adapter contract: imports cleanly without the full hindsight namespace and exposes _ensure_started/close/__getattr__.
…ons call (rebase fix)
…ron tests to upstream tick architecture
…_cron_session_db The done-callback _close_late_session_db_result was registered before future.result() was called, so when SessionDB() completed on time (the normal case) the callback fired immediately in the calling thread and closed the connection — returning a SessionDB with _conn=None. This caused the stale-session reaper to silently return 0 (list_open_cron_sessions hit _conn.execute on None) and run_job to fail when titling sessions. Matching the upstream pattern: register the callback only inside the except TimeoutError block so it fires exclusively for a late result. Fixes 5 TestReaperFailClosed tests and test_monitor_kind failures.
The Hindsight plugin's _get_client() still used the legacy 'from hindsight import HindsightEmbedded' path requiring hindsight-all (torch/transformers/sentence-transformers ~2-3GB) in the agent venv. The split-runtime design (DedicatedEmbeddedClient in embedded_runtime.py) was committed (ec7ba1a) but the __init__.py wiring was lost in a subsequent rebase, leaving Hindsight silently offline since Aug 14. Changes: - _check_local_runtime: probe hindsight_client + hindsight_embed + embedded_runtime (lightweight, <1s) instead of hindsight + sentence_transformers (monolith, requires 134-package install) - _get_client: use DedicatedEmbeddedClient from .embedded_runtime instead of HindsightEmbedded; drop __del__ monkey-patch; add server_executable kwarg - _probe_url: read inner Hindsight._base_url (private attr) instead of .url (would trigger __getattr__ daemon start) - _local_runtime_hint: broadened trigger; new guidance points to hermes memory setup + HINDSIGHT_EMBED_API_EXECUTABLE, not hindsight-all - Version constants: restore _MIN=0.8.4, _MAX=0.10, _CLIENT_REQUIREMENT, _EMBED_REQUIREMENT (was regressed to 0.6.1) - Setup wizard: install [client, embed] not hindsight-all for local_embedded - embedded_runtime.py: add seal cache (keyed by candidate+fingerprint, 45000x speedup) + conditional LLM config (only set HINDSIGHT_API_LLM_* when truthy, preventing empty-string shadowing of hermes.env keys) - plugin.yaml: declare hindsight-embed>=0.8.4,<0.10 - Remove hindsight-all from memory_setup.py, web_server.py, update_cmd.py - Update tests to mock DedicatedEmbeddedClient; port test_hindsight_embedded_runtime.py (621 LOC) from reference; add seal-cache regression test - Update website docs (EN + zh-Hans) to split-runtime terminology Verified: hermes memory status → available; _check_local_runtime → (True, None) in 0.49s; seal cache 5.45s→0.0001s; live recall 26 facts in 3.68s; 154 passed / 3 pre-existing failures (confirmed on pristine HEAD).
Resolved conflicts: - cron/scheduler.py: kept both hashlib (ours) and errno (upstream) imports - cron/scheduler_provider.py: adopted upstream EMFILE self-heal backoff - pyproject.toml: kept our hindsight 0.8.4 + upstream mcp 2.0.0/httpx2 upgrade Auto-merged 23 overlapping files incl: agent/conversation_loop.py, gateway/run.py, hermes_state.py, run_agent.py, tools/*, tui_gateway/server.py, apps/desktop/src/app/session/hooks/*. Upstream highlights: Bot Mode builtin plugin (6935 lines), HUD surface, multi-gateway connections registry, session list density modes, transcript tail-page hydration, per-turn duration badges, find-bar positioning fixes, titlebar clusters, sidebar filter/inbox/density, running≠busy, composer settle-hold bound, session tile reclaim self-heal.
ef7fe8e to
f2c8b04
Compare
Resolved 4 conflicts in tui_gateway/ — preserved our submitted_at/message_id params alongside upstream's new display_kind param: - server.py: _run_prompt_submit + _compute_host_turn_frame signatures - compute_host.py: frame kwargs forwarding - methods_prompt.py: _run_prompt_submit call in run_after_agent_ready - test_tui_gateway_server.py: _run_inline mock accepts display_kind
…ration Address review feedback on NousResearch#62492 from @jrleal10: 1. _migrate_profile_config: print an actionable per-profile warning when missing required settings prevent silent auto-migration (previously silently skipped, leaving stale profiles undiagnosed). 2. cmd_update all-profiles loop: replace logger.debug (invisible to users) with a visible stderr warning when per-profile migration fails, including the profile name and remediation command. 3. Add behavioral test coverage for both entry points: - TestMigrateProfileConfig: helper unit tests (4 tests) - TestUpdateAllProfilesMigration: update-loop coverage (2 tests) - TestDashboardStartupMigration: serve/dashboard startup coverage (3 tests) Tests cover: version-bump-only silent path, missing-settings warning, exception surfacing, up-to-date skip, multi-profile iteration, and failure visibility.
Tri-model review (3/3 ACCEPT) flagged the profiles_env fixture as dead code — no test uses it (all 12 tests use mock.Mock() profiles). Removed fixture plus unused os/pathlib/yaml imports.
…ount publication The background prefetch worker in queue_prefetch inlined its own recall/reflect calls instead of honoring the shared _do_recall contract, so it bypassed two behaviors our tests encode: (1) the query was never clipped to recall_max_input_chars before reaching arecall/areflect, and (2) the discrete memory count was never stored in _prefetch_count, leaving prefetch() to publish count=0 in the deterministic recall indicator (recall_status) regardless of what was injected, and breaking the stale-count lifecycle. Fix: extract a shared _truncate_query() helper used by both the sync and background recall paths, and have the worker capture len(results) (0 for reflect synthesis) and store it alongside _prefetch_result under the same lock; the session-rotation reset now clears the count with the buffer so the pair stays atomic.
# Conflicts: # contributors/emails/agent@agents-Mac-mini.local
47862b2 to
441dccd
Compare
# Conflicts: # apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx
Summary
interruptedstatus with an empty final response instead of assistant prosetool -> usertails only in the per-request API copy so strict providers still receive valid orderingRoot cause
The core loop returned local cancellation status strings such as
Operation interrupted: waiting for model response (...)asfinal_response, and the turn finalizer could persist a syntheticOperation interrupted.assistant row after a tool result. Desktop then handled everymessage.completeas a successful completion, so cancellation metadata appeared as a reply and triggered completion UX.The fix makes structured
interruptedstate authoritative. Empty cancellations stay metadata-only; actual streamed partial text is still returned and persisted. Provider role repair now happens on the API copy rather than rewriting the durable transcript.Verification
pytest -q tests/run_agent/test_message_sequence_repair.py tests/agent/test_turn_finalizer*.py tests/agent/test_interrupt_tool_tail_api_sanitization.py tests/run_agent/test_run_agent.py -k 'interrupt or tool_tail or sanitize'— 37 passednpx vitest run src/app/session/hooks/use-message-stream --environment jsdom— 12 passednpm run typecheckinapps/desktop— passeduv run --frozen --extra dev --extra acp pytest -q tests/acp/test_server.py -k interrupt— 1 passednpx vitest run src/__tests__/createGatewayEventHandler.test.ts -t 'interrupt'inui-tui— 2 passedgit diff --check— passedRelated work
This is complementary to #55317: that PR surfaces non-interrupted tool-result-only endings, while this PR keeps interrupted cancellation paths metadata-only.