[pull] main from NousResearch:main - #162
Merged
Merged
Conversation
…t .env _is_sensitive_filename() only blocked .env / .env.<suffix>, but the dashboard Files tab's managed root is operator-configurable and, per the docker-mount scenario #57505 was filed against, can point directly at HERMES_HOME — where the canonical credential stores enforced elsewhere in the codebase (gateway.platforms.base._ROOT_CREDENTIAL_FILES, agent.file_safety.get_read_block_error) all live: auth.json, OAuth token stores, webhook HMAC secrets, the Bitwarden disk cache. None of those basenames were blocked, so the Files tab could still list, read, and download them. .envrc (direnv) also slipped past the old check since it doesn't equal ".env" or start with ".env.". Widen the basename set to mirror both existing guards so the dashboard doesn't lag behind them.
…anaged-files guard Follow-up to @srojk34's basename-denylist widening. Two gaps the basename-only guard left, both covered by the two canonical guards it mirrors: - Directory-tree stores mcp-tokens/ (live MCP OAuth tokens) and pairing/ are denied as whole trees by gateway.platforms.base._ROOT_CREDENTIAL_DIRS and agent.file_safety, but the dashboard files API descends into subdirs, so mcp-tokens/<server>.json (non-canonical basename) stayed listable/readable/downloadable. Add _is_sensitive_path(), a path-aware check that blocks any path with a credential-directory component, and route all three call sites (list/read/download) through it. - Add .git-credentials to the basename set (agent.file_safety blocks it too). - Correct the docstring: it now says it mirrors the credential-FILE basenames of the canonical guards, with the directory trees handled by the new path-aware helper (the prior wording overstated parity). Scope stays on the read/list/download exfil surface (#57505); the write endpoints (upload/mkdir/delete) are a separate threat and out of scope. Tests: dir-tree descent blocked (mcp-tokens/pairing per-server files), .git-credentials blocked, plus a positive control that a benign subdir file stays browsable. Mutation-checked (neuter _is_sensitive_path -> new tests fail). 39 web_server_files + fs tests pass, ruff clean.
…tial-guard security(dashboard): widen managed-files credential guard past .env + close dir-tree gap
…deadline helper Follow-up to @msh01's wall-deadline init-timeout fix. - Resource leak: on timeout the initialize() task is abandoned without awaiting its (shielded, possibly-never-completing) cancellation, so the half-built PTB app's httpx client / connection pool was never closed — up to 8x across the retry ladder. Add an optional on_abandon cleanup to _await_with_thread_deadline that best-effort app.shutdown()s the abandoned app, run detached + exception-swallowed so it can never re-block or re-hang the ladder (mirrors _close_client_on_timeout in agent/auxiliary_client.py). - Cover the helper itself: the salvaged test monkeypatched out the real _await_with_thread_deadline, so its abandonment/cleanup path was untested. Add direct tests for happy-path return, prompt-timeout-with-cleanup, and cleanup-error-swallowed; the wedged coroutines swallow cancellation for a bounded window (proving the helper returns before cancellation completes, the #58236 shielded-scope behavior) without leaving an immortal task that would wedge pytest teardown. Widen the salvaged stub to accept on_abandon. - Attribution: add yingwaizhiying@gmail.com -> msh01 to AUTHOR_MAP (bare gmail does not auto-resolve the check-attribution gate). Known follow-up (not addressed here): the retry ladder reuses the same self._app across all 8 attempts; a fresh app per attempt would fully close the coherence risk if an abandoned initialize() completes in the background. That is a larger restructure of the ~130-line builder+handler setup, left for a separate change.
…adline fix(telegram): wall-deadline init timeout + shut down abandoned init app
…t synthesis call 22c5048 restored Anthropic-style cache_control for two of MoA's three call paths: the acting aggregator (MoAChatCompletions.create, the persistent `provider: moa` model) and the advisor fan-out (_run_reference). aggregate_moa_context() -- the /moa <prompt> one-shot command's synthesis call -- is the third, independent call path and was never covered: its call_llm(task="moa_aggregator", ...) sent a single undecorated user message containing the full joined reference output, re-billing the entire input on every invocation even when the resolved aggregator slot is a cache-honoring route (Claude on OpenRouter/native Anthropic, MiniMax, Qwen/DashScope). - Generalize _maybe_apply_advisor_cache_control to _maybe_apply_moa_cache_control (it never had advisor-specific logic -- same policy function, same breakpoint layout as the main loop, judged purely on the passed-in runtime) and reuse it in aggregate_moa_context the same way _run_reference already does. - Compute _slot_runtime(aggregator) once and reuse it for both the decoration call and the call_llm kwargs, instead of calling it twice. Mutation-verified: reverting the moa_loop.py change makes the new regression test fail by asserting a plain string aggregator-message content where the cache-honoring case expects native cache_control content blocks.
…urn guard Replace the inline dict-copy + _db_persisted pop in _ensure_compressed_has_user_turn with the canonical _fresh_compaction_message_copy helper (the same primitive the compressor's own protected-head/tail assembly uses), so the persistence-marker strip stays consistent across all compaction copy sites (#57491). Expand the docstring to record the alternation-safety and end-placement rationale.
Contributor of the salvaged PR #58276 fix commit. Required so the contributor attribution CI check passes on the rebase-merge that preserves their authorship.
…58327) Strict providers (DeepSeek) reject a payload where the same tool_call_id appears more than once with HTTP 400 'Duplicate value for tool_call_id'. The issue was filed as an 'orphaned tool message' compression bug, but the pasted error is a DUPLICATE tool_call_id — orphans are already handled on main; duplicates were not. Reproduced live on main: both shapes leaked through repair_message_sequence and sanitize_api_messages. Two chokepoints, two shapes: - repair_message_sequence: consume the id from known_tool_ids on first match so a SECOND tool result reusing it falls into the drop branch (duplicate tool-result shape). This is @Robinlovelace's kernel from #55436 (applied manually — that PR was ~800 commits stale and bundled an unrelated duplicate-DB-write change for #860, which is dropped here). - sanitize_api_messages (final pre-API pass): add a dedup pass covering BOTH (a) duplicate tool_calls sharing an id WITHIN one assistant message (the message[6] shape) and (b) later tool result messages reusing an already-seen id. #55436 covered neither of these at this chokepoint. Tests: duplicate-tool-result dedup at both functions, duplicate-assistant- tool_call-id collapse, and a negative control proving distinct ids are never dropped (no over-dedup). Credit: @Robinlovelace (#55436) for the repair_message_sequence dedup kernel. Closes #58327.
…T reconnect hang When the TCP connection enters CLOSE-WAIT the PTB polling task is blocked on epoll on a dead socket and never wakes. updater.stop() awaits that task and therefore hangs indefinitely. Consequence: _polling_error_task stays alive-but-blocked forever; every subsequent heartbeat probe sees it as "in-flight" and skips triggering a new reconnect; the gateway silently drops messages for hours until a manual restart. Field incident: 11-hour outage on 2026-07-04 UTC despite the heartbeat loop firing a reconnect at 01:11 — stop() blocked the entire ladder. Fix: wrap the updater.stop() call inside asyncio.wait_for(timeout=15). On TimeoutError log a warning and continue to _drain_polling_connections() + start_polling() — same recovery path, just unblocked. The heartbeat loop (PR #48496) correctly detects the dead socket and fires _handle_polling_network_error. This commit is the missing second half: ensuring the reconnect itself always completes. Test: test_handle_polling_network_error_updater_stop_timeout() simulates a hang by making stop() sleep forever and verifies that drain + start_polling are still reached after the timeout. Fixes #58270
… CLOSE-WAIT timeout The salvaged fix (#58272) guarded the primary network-error reconnect path. Issue #58270's Scope section names three more unguarded await updater.stop() sites that can hang identically on a CLOSE-WAIT socket: - conflict handler (before the retry back-off sleep) - conflict-retries-exhausted teardown (before the fatal notify) - disconnect() teardown (would hang gateway shutdown/restart) Each is now wrapped in asyncio.wait_for(..., _UPDATER_STOP_TIMEOUT) with a warning on timeout, matching the primary path, so no reconnect/teardown ladder can wedge on a dead socket. Also hoist the shared 15.0s bound to a single module constant _UPDATER_STOP_TIMEOUT (self-documenting + DRY across all 4 sites), and update the CLOSE-WAIT regression test to patch that constant instead of monkeypatching asyncio.wait_for process-wide. Fatal-notify idempotency: bounding the conflict-exhausted teardown stop() adds an await AFTER _set_fatal_error, which yields the loop and lets a concurrent retry task (scheduled by an earlier conflict, already suspended past the entry guard) reach the fatal branch too — double-firing the fatal handler (surfaced as a Python 3.11 CI failure in test_polling_conflict_becomes_fatal_after_retries). Snapshot the pre-transition fatal state and only notify on the first transition.
The conflict-retry ladder schedules a background recovery task via loop.create_task(self._handle_polling_conflict(...)) on each failed start_polling. test_polling_conflict_becomes_fatal_after_retries never cancelled the last one, so under a loaded scheduler a leaked task could get a turn, re-drive the counter into the fatal branch, and fire _notify_fatal_error a second time — breaking assert_awaited_once() non-deterministically. The bounded updater.stop() guard added in this salvage introduced an extra await/scheduling yield that surfaced the latent leak in CI slice 3. Cancel the leaked task before the fatal assertions so the test is deterministic regardless of scheduler timing.
fix(agent): deduplicate tool_call_id across pre-API sanitizers (#58327)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )