Conversation
🔎 Lint report:
|
| Rule | Count |
|---|---|
unsupported-operator |
14 |
unresolved-attribute |
10 |
unresolved-import |
9 |
invalid-argument-type |
3 |
invalid-assignment |
3 |
invalid-parameter-default |
2 |
unresolved-reference |
1 |
First entries
tests/gateway/test_discord_voice_mixer.py:14: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/gateway/test_slack_stream.py:1044: [unresolved-attribute] unresolved-attribute: Unresolved attribute `response` on type `Exception`
tests/hermes_cli/test_kanban_core_functionality.py:3375: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
tests/cli/test_resume_display.py:716: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["resume_display"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
tests/cli/test_fast_command.py:484: [invalid-argument-type] invalid-argument-type: Argument to bound method `TestCase.assertIn` is incorrect: Expected `Iterable[Any] | Container[Any]`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
gateway/run.py:17971: [invalid-assignment] invalid-assignment: Object of type `Unknown | None` is not assignable to attribute `tool_progress_callback` on type `(Any & ~None) | AIAgent`
tests/tools/test_web_providers.py:218: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["search_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
gateway/run.py:17971: [unresolved-attribute] unresolved-attribute: Attribute `on_tool_progress` is not defined on `GatewayStreamConsumer` in union `(Unknown & ~None) | GatewayStreamConsumer`
tests/hermes_cli/test_aux_config.py:37: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["title_generation"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
gateway/restart.py:10: [invalid-argument-type] invalid-argument-type: Argument to constructor `float.__new__` is incorrect: Expected `str | Buffer | SupportsFloat | SupportsIndex`, found `Unknown | int | str | ... omitted 14 union elements`
tests/hermes_cli/test_aux_config.py:41: [unsupported-operator] unsupported-operator: Operator `>` is not supported between objects of type `Unknown | int | str | ... omitted 5 union elements` and `Literal[0]`
tests/tools/test_browser_lightpanda.py:242: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["engine"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
gateway/platforms/slack_stream.py:352: [invalid-parameter-default] invalid-parameter-default: Default value of type `None` is not assignable to annotated parameter type `str`
gateway/platforms/slack_stream.py:353: [invalid-parameter-default] invalid-parameter-default: Default value of type `None` is not assignable to annotated parameter type `dict[Unknown, Unknown]`
tests/agent/test_auxiliary_config_bridge.py:284: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["provider"]` and `Unknown | int | str | ... omitted 14 union elements`
tests/tools/test_web_providers.py:219: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["extract_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
hermes_cli/telegram_managed_bot.py:87: [unresolved-import] unresolved-import: Cannot resolve imported module `qrcode`
hermes_cli/config.py:4602: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
tests/gateway/test_whatsapp_reply_prefix.py:119: [unsupported-operator] unsupported-operator: Operator `>=` is not supported between objects of type `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements` and `int`
tests/hermes_cli/test_aux_config.py:47: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["session_search"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
gateway/run.py:17980: [invalid-assignment] invalid-assignment: Object of type `None | (def _stream_delta_cb(text: str) -> None) | (def _stream_delta_cb(text: str) -> None)` is not assignable to attribute `stream_delta_callback` on type `(Any & ~None) | AIAgent`
gateway/platforms/slack.py:1317: [unresolved-reference] unresolved-reference: Name `SlackStreamConsumer` used when not defined
tests/agent/test_auxiliary_config_bridge.py:285: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["model"]` and `Unknown | int | str | ... omitted 14 union elements`
gateway/run.py:17799: [unresolved-attribute] unresolved-attribute: Attribute `on_delta` is not defined on `None` in union `Unknown | None | GatewayStreamConsumer`
tests/agent/test_curator.py:994: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["curator"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 30 union elements`
... and 17 more
✅ Fixed issues (25):
| Rule | Count |
|---|---|
unsupported-operator |
14 |
unresolved-attribute |
7 |
invalid-argument-type |
3 |
invalid-assignment |
1 |
First entries
tests/tools/test_browser_console.py:341: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["record_sessions"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/tools/test_browser_lightpanda.py:242: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["engine"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
hermes_cli/config.py:4573: [unresolved-attribute] unresolved-attribute: Attribute `items` is not defined on `int`, `str`, `list[Unknown]`, `float`, `None` in union `Unknown | int | str | ... omitted 13 union elements`
hermes_cli/config.py:4564: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/cli/test_resume_display.py:716: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["resume_display"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/gateway/test_whatsapp_reply_prefix.py:119: [unsupported-operator] unsupported-operator: Operator `>=` is not supported between objects of type `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements` and `int`
tests/agent/test_auxiliary_config_bridge.py:284: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["provider"]` and `Unknown | int | str | ... omitted 13 union elements`
tests/hermes_cli/test_aux_config.py:41: [unsupported-operator] unsupported-operator: Operator `>` is not supported between objects of type `Unknown | int | str | ... omitted 4 union elements` and `Literal[0]`
tests/cli/test_fast_command.py:484: [invalid-argument-type] invalid-argument-type: Argument to bound method `TestCase.assertIn` is incorrect: Expected `Iterable[Any] | Container[Any]`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/tools/test_web_providers.py:218: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["search_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/hermes_cli/test_destructive_slash_confirm_gate.py:32: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
gateway/run.py:17838: [invalid-assignment] invalid-assignment: Object of type `None | (def _stream_delta_cb(text: str) -> None)` is not assignable to attribute `stream_delta_callback` on type `(Any & ~None) | AIAgent`
tests/hermes_cli/test_aux_config.py:37: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["title_generation"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
hermes_cli/config.py:4554: [unresolved-attribute] unresolved-attribute: Attribute `items` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/tools/test_web_providers.py:219: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["extract_backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/hermes_cli/test_mcp_reload_confirm_gate.py:33: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/agent/test_curator.py:999: [unsupported-operator] unsupported-operator: Operator `>` is not supported between objects of type `Unknown | int | str | ... omitted 4 union elements` and `Literal[0]`
tests/agent/test_auxiliary_config_bridge.py:285: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["model"]` and `Unknown | int | str | ... omitted 13 union elements`
tests/agent/test_curator.py:994: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["curator"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/hermes_cli/test_aux_config.py:54: [unresolved-attribute] unresolved-attribute: Attribute `keys` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/hermes_cli/test_aux_config.py:47: [unsupported-operator] unsupported-operator: Operator `not in` is not supported between objects of type `Literal["session_search"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
gateway/restart.py:10: [invalid-argument-type] invalid-argument-type: Argument to constructor `float.__new__` is incorrect: Expected `str | Buffer | SupportsFloat | SupportsIndex`, found `Unknown | int | str | ... omitted 13 union elements`
tests/hermes_cli/test_kanban_core_functionality.py:3375: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/cli/test_reasoning_command.py:552: [invalid-argument-type] invalid-argument-type: Argument to bound method `TestCase.assertIn` is incorrect: Expected `Iterable[Any] | Container[Any]`, found `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
tests/tools/test_web_providers.py:217: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `Literal["backend"]` and `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 29 union elements`
Unchanged: 5084 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
jarvisxyz
force-pushed
the
2026-06-03.eizus.slack-stream-steps-fix
branch
3 times, most recently
from
June 4, 2026 18:41
ee7f769 to
43da907
Compare
…oss dashboard/Docker/Desktop docs (NousResearch#39633) * docs(dashboard): clarify auth provider suitability + document dashboard registration - Add a 'Registering a dashboard' subsection under the Nous Research provider covering both the 'hermes dashboard register' CLI command and the Portal /local-dashboards GUI page. - Note that the Nous provider is the one suitable for public-internet exposure (logins verified against your Nous account). - Add a warning that the username/password provider is for trusted networks / VPN only and is not suitable for direct public-internet exposure; point readers to the Nous / OIDC / custom OAuth providers. - Surface the same distinction in the two-provider intro list. * docs(dashboard): count three bundled auth providers, add self-hosted OIDC to intro 'Two providers ship in the box' undercounted — the bundled plugins/dashboard_auth/self_hosted (generic OpenID Connect) is a third. List all three in the gated-mode intro and link each to its section. * docs(dashboard): extend auth provider updates to Docker and Desktop pages - docker.md: list all three bundled gate providers (was username/password + OAuth only), adding the self-hosted OIDC provider and its env vars, and note username/password is not for public-internet exposure. - desktop.md: reframe the remote-backend connection so OAuth (Nous Portal) is the preferred option for any backend reachable beyond the local machine, with username/password positioned for local / trusted-network use only. Cover the 'Sign in with <provider>' OAuth flow in the in-app steps and scope the VPN warning to the password path. * docs(dashboard): align env-var, CLI, and remote-Desktop recipe with provider changes - environment-variables.md: reframe the Web Dashboard & Hermes Desktop intro (OAuth preferred for remote/public, username/password for trusted networks), add the self-hosted OIDC env vars (HERMES_DASHBOARD_OIDC_*) that were missing from the table, and note hermes dashboard register provisions the OAuth client_id. - cli-commands.md: document the 'hermes dashboard register' subcommand (flags, behavior, /local-dashboards GUI alternative). - web-dashboard.md: apply the OAuth-preferred reframe to the bottom 'Connecting Hermes Desktop to a remote backend' recipe and scope its VPN warning to the username/password path, matching desktop.md. * docs(dashboard): move 'recommended remote Desktop path' framing from username/password to OAuth The gated-mode intro list claimed the username/password provider was the recommended path for a remote Hermes Desktop connection, contradicting the OAuth-preferred framing established elsewhere. Move that recommendation onto the OAuth (Nous Portal) item so the docs are consistent: OAuth is the recommended provider for any remote/internet-facing backend; username/password is for trusted networks only. * docs(dashboard): drop unreleased managed/hosted-install provisioning notes Remove the 'not available in managed/hosted installs, where the client id is provisioned by the hosting platform' line from the dashboard register docs (web-dashboard.md, cli-commands.md) and the 'provisioned by the Nous Portal for hosted deploys' clause from the HERMES_DASHBOARD_OAUTH_CLIENT_ID env-var row — that platform-provisioning path is unreleased. * docs(dashboard): drop --portal-url / HERMES_DASHBOARD_PORTAL_URL from user docs The portal-URL override targets a non-production Nous Portal and only works for internal Nous usage — it won't function for end users (the access token must be issued by the same portal). Remove it from the register CLI flags, the Nous-provider config/env tables, and the verify-the-gate example so users aren't pointed at an option that can't work for them. * docs(dashboard): add worked examples for Nous and username/password providers The self-hosted OIDC provider already had a full 'Worked example: Keycloak' walkthrough; the Nous and username/password providers only had scattered config snippets. Add parallel '#### Worked example' sections for both (register/run/login + /api/status verification), mirroring the Keycloak example's structure so all three bundled providers read consistently. * docs(env): move HERMES_DESKTOP_REMOTE_URL to end of the dashboard auth table It was sitting between the HERMES_DASHBOARD_BASIC_AUTH_* block and the HERMES_DASHBOARD_OAUTH/OIDC block, splitting the dashboard-side vars. As the only desktop-side var in the table, it belongs at the end so the dashboard provider vars (basic, OAuth, OIDC) stay grouped together. * docs(dashboard): remove Fly.io references from dashboard auth docs Fly.io is the internal hosting implementation for hosted Hermes — it shouldn't leak into user-facing dashboard auth docs. Reword the OAuth provider intro, the env-var-path rationale, the public-URL-override section, the cookie Secure note, and the verify-the-gate example to generic 'hosting platform' / 'reverse proxy' / 'TLS terminator' phrasing. Left the legitimate user-facing Fly.io mentions in telegram.md (a deliberate cloud-deployment walkthrough) and work-with-skills.md (a generic example) untouched.
Add HERMES_DASHBOARD_SESSION_TOKEN to the Hermes-managed subprocess environment blocklist so dashboard authorization material does not propagate into shell, PTY, or background process launches. Extend the local environment blocklist regression coverage to prove the dashboard session token is stripped like other Hermes-managed secrets.
…utostashing it The Desktop bootstrap installer writes `.hermes-bootstrap-complete` into the managed git checkout root. Because it wasn't gitignored, `hermes update`'s `git stash push --include-untracked` treated it as a local change and created an autostash on every run — prompting the user to restore "local changes" that were really Hermes-managed runtime state (and risking the marker getting stranded in a stash, which re-triggers Desktop bootstrap). Add the marker to .gitignore; `git stash -u` and `git status --porcelain` both skip ignored files, so the updater now sees a clean tree. Fixes NousResearch#38529
When a provider is configured but no model is selected (e.g. a profile sets provider: nous with no model), the gateway/CLI fall back to get_default_model_for_provider(), which returned the first curated catalog entry. The Nous Portal list is ordered most-capable-first, so entry [0] is anthropic/claude-opus-4.8 — the single most expensive model ($5/$25 per Mtok). A misconfigured profile therefore silently routed every call to the flagship and billed it for traffic the user never opted into. Pin the silent (non-interactive) default for metered aggregators to the cheapest curated tier via _PROVIDER_SILENT_DEFAULT_OVERRIDES so a missing model can never auto-escalate to the flagship. The interactive default (GUI onboarding / `hermes model`) keeps using the richer free/paid-tier-aware resolver. Fixes the unexpected anthropic/claude-opus-4.8 charges reported for a free-tier Nous account whose new profile had no default model.
…calation
Assert get_default_model_for_provider("nous") never returns the priciest
catalog entry (anthropic/claude-opus-4.8) and that an override pointing at a
model absent from the catalog falls back to catalog order. Regression for the
silent flagship-billing footgun.
Follow-up on the salvaged fix: point the Nous silent-default override at deepseek/deepseek-v4-flash (a cheap chat model) instead of the nvidia nemotron entry. Keeps the no-model-configured fallback off the priciest flagship while landing on a low-cost, broadly-capable default.
…hat overlap TTS (NousResearch#39659) * feat(discord): voice-channel mixer — ambient idle bed + verbal acks that overlap TTS Discord voice mode can now feel conversational: the bot speaks a short acknowledgement before it starts working, and a subtle ambient 'thinking' bed plays underneath while tools run, ducking under speech and swelling back — the Grok-voice-mode feel. discord.py plays only one audio stream per voice connection, so this adds a software mixer (VoiceMixer, a discord.AudioSource) installed once per guild on join. It sums an ambient loop, verbal acks, and TTS replies into that single 20ms/48kHz/stereo stream (numpy int16 add + clip), so they overlap instead of stop-and-swap. Speech ducks the ambient gain down and releases it smoothly. - plugins/platforms/discord/voice_mixer.py: VoiceMixer + MixerChild (gain, loop, fade, duck/release), decode_to_pcm (ffmpeg), synth_ambient_pcm (no asset needed — synthesised pad). - adapter: install mixer on join, tear down on leave, route play_in_voice_channel through the mixer (legacy one-shot path kept as fallback), play_ack_in_voice, voice_mixer_active. Defensive getattr for the object.__new__ test helpers. - gateway/run.py: tool_start_callback fires a one-time verbal ack on the first tool call of a turn when in a voice channel (independent of the text tool-progress gate). No system-prompt or message-flow changes. - config: discord.voice_fx.* (OFF by default; ambient/duck/speech gains, ack phrases). All in config.yaml, not .env. - docs + tests (mixer unit + adapter integration). Verified: 19 new tests pass, existing voice suite green (2 pre-existing davey-module env failures unchanged), and a real-mixer E2E confirms ambient streams, TTS overlaps it, acks layer in, and teardown is clean. * fix(discord): make voice mixer numpy import lazy (numpy is voice-extra-only) numpy ships in the optional 'voice' extra, not [all,dev], so a module-level 'import numpy' broke CI test collection (and would break the always-imported Discord adapter on any install without the voice extra). Defer numpy to the functions that actually mix audio via _require_numpy(); guard the test module with pytest.importorskip('numpy').
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
… on Gemini endpoints (NousResearch#39730) * fix: respect disabled auto-compaction on context overflow Port from anomalyco/opencode#30749. When compression.enabled is false, NO automatic compaction trigger may fire. The proactive token-threshold paths (preflight + post-response should_compress gate) already honoured the setting, but the three provider-overflow recovery paths in the agent loop — long-context-tier 429, 413 payload-too-large, and context-overflow — called _compress_context() unconditionally, silently compressing and rotating the session against the user's explicit choice. Add a single guard at the top of the overflow-recovery dispatch: when compression is disabled and the error is one of those three overflow classes, surface a terminal error (compaction_disabled: True) telling the user to /compress manually, /new, switch to a larger-context model, or reduce attachments. Manual /compress (force=True) is unaffected — it never enters this loop. Tests: new TestOverflowWithCompactionDisabled (413 + 400 overflow don't compress when disabled; control case still compresses when enabled). Existing overflow-recovery tests updated to enable compaction explicitly (they verify the recovery fires); fixture defaults flipped to True to match production (compression.enabled defaults to True). * fix(gemini): default native maxOutputTokens + strip OpenAI extra_body on Gemini endpoints Two distinct failures hit users on the gemini provider with only Google AI Studio keys set. 1. Truncation loop: build_gemini_request() only set maxOutputTokens when max_tokens was non-None. Hermes passes None to mean "unlimited", but Gemini's native generateContent does NOT treat an absent maxOutputTokens as full budget — it applies a low internal default and stops early with finishReason=MAX_TOKENS, truncating tool calls. The agent then retries 3x and refuses the incomplete call. Now default to the published 65,535 ceiling (shared by all current Gemini text models) when max_tokens=None. 2. HTTP 400 on Gemini endpoint: the chat_completions transport assembles profile extra_body (Nous portal 'tags', reasoning, provider prefs) and sends it via the OpenAI client to whatever base_url is resolved. When a profile that emits extra_body (e.g. Nous) is active but the endpoint is a native Gemini base_url — typical when only Google creds exist and a fallback/aux call lands on Gemini — Google rejects the unknown 'tags' field with a non-retryable 400. Strip all non-thinking_config extra_body keys when the resolved endpoint is native Gemini. Verified E2E against real transport code: tags stripped on native Gemini, preserved on Nous and the /openai compat endpoint; maxOutputTokens=65535 on None, explicit values respected.
…9766) A bare /voice silently toggled on/off with a one-line result, leaving users with no idea what the modes mean or that Discord also supports TTS-all and live voice-channel join/leave. Bare /voice now still toggles but appends a usage explainer covering on/off/tts/status, with the Discord voice-channel lines shown only on adapters that support them. Adds gateway.voice.help + gateway.voice.help_channels across all 16 locales (placeholders {toggle}/{channels}).
…search#39772) * fix: respect disabled auto-compaction on context overflow Port from anomalyco/opencode#30749. When compression.enabled is false, NO automatic compaction trigger may fire. The proactive token-threshold paths (preflight + post-response should_compress gate) already honoured the setting, but the three provider-overflow recovery paths in the agent loop — long-context-tier 429, 413 payload-too-large, and context-overflow — called _compress_context() unconditionally, silently compressing and rotating the session against the user's explicit choice. Add a single guard at the top of the overflow-recovery dispatch: when compression is disabled and the error is one of those three overflow classes, surface a terminal error (compaction_disabled: True) telling the user to /compress manually, /new, switch to a larger-context model, or reduce attachments. Manual /compress (force=True) is unaffected — it never enters this loop. Tests: new TestOverflowWithCompactionDisabled (413 + 400 overflow don't compress when disabled; control case still compresses when enabled). Existing overflow-recovery tests updated to enable compaction explicitly (they verify the recovery fires); fixture defaults flipped to True to match production (compression.enabled defaults to True). * feat(delegation): uncap max_spawn_depth to match max_concurrent_children Removed the hard ceiling of 3 on delegation.max_spawn_depth. Depth now has a floor of 1 and no upper limit, mirroring max_concurrent_children. Cost (each level multiplies API spend) is the practical limiter, not a constant. - delegate_tool.py: drop _MAX_SPAWN_DEPTH_CAP, _get_max_spawn_depth() floors at 1 instead of clamping to [1,3]; depth-limit error string reworded - config.py / cli-config.yaml.example: doc comments say floor 1, no ceiling - docs (configuration, delegation, delegation-patterns): range 1-3 -> >=1 - tests: convert clamp-above-3 change-detector into a no-ceiling invariant, drop the _MAX_SPAWN_DEPTH_CAP==3 snapshot assert, fix warning-text assert
When the agent's reply references a deliverable file path that does not exist on disk, extract_local_files dropped it from native delivery with no log line — the most common reason a promised file never arrives over a messaging platform. Add an INFO log at that drop point so the gap is visible in gateway.log instead of vanishing. Also convert the two print() calls in Telegram's send_document / send_video exception handlers to logger.warning(exc_info=True). print() writes to stdout, which 'hermes logs' never captures, so outbound upload failures (oversized files, Bot API rejections) were invisible.
…eractive updates (reverts NousResearch#38542, NousResearch#39568) (NousResearch#39645) * Revert "fix(update): require managed marker before destructive clean" This reverts commit c8e80cd. * Revert "fix(update): stop stash/restore from clobbering desktop source on managed clones (NousResearch#38542)" This reverts commit 8a19884. * chore(install): keep npm ci desktop-build fix after stash revert The destructive-clean reverts (NousResearch#38542/NousResearch#39568) pulled the desktop workspace install back to bare `npm install`. The npm ci -> npm install fallback is orthogonal build-correctness (avoids the Windows workspace-hoisting flake where install reports up-to-date against a stale marker while node_modules is empty, breaking tsc -b). Preserve it. * feat(update): settable stash-or-discard for non-interactive local changes Adds updates.non_interactive_local_changes (stash | discard, default stash). Governs ONLY non-interactive updates (desktop/chat app, gateway, --yes) — interactive terminal updates always stash-and-ask, unchanged. - config.py: new key under existing updates section; _config_version 26->27. - main.py: _cmd_update_impl detects non-interactive (gateway/--yes/no-TTY), reads the setting; new _discard_stashed_changes() drops the stash (stash-and-drop, never reset --hard/clean -fd, so ignored paths survive). Post-pull restore site branches on it; the bail-out and up-to-date restores always preserve work. - web_server.py + apps/desktop settings: exposes it as a stash/discard select (Advanced section, In-App Update Local Changes). - docs + tests (discard drops, stash restores, interactive ignores setting, missing section defaults to stash). * fix(install.ps1): stash/restore instead of reset --hard on Windows update The PR reverted the destructive update path to stash/restore everywhere except scripts/install.ps1, whose managed-clone update path still ran `git reset --hard HEAD` before checkout — silently destroying agent-edited tracked source on Windows (the same NousResearch#38542 data-loss class the PR fixes). - Replace `git reset --hard HEAD` with stash-before-checkout + restore-after-checkout, mirroring install.sh. Untracked files are included so agent-created dirs (e.g. tinker-atropos/) survive. - Keep `core.autocrlf false` (it prevents the phantom CRLF dirt that made the stash necessary; it's also load-bearing for a clean restore). - Wrap all three checkout modes (Commit/Tag/Branch); Branch case now uses `git pull --ff-only` so local commits are never clobbered. - Only prompt to restore when a real console is attached (UserInteractive + non-redirected stdin/stdout + ConsoleHost); the desktop Update button and bootstrap have no usable console, so they default to restore and never hang on Read-Host. - On restore conflict or a failed update, the stash is preserved with recovery instructions — work is never silently dropped. Validated on Windows (PowerShell 5.1, git 2.54): AST parse clean; E2E non-conflicting restore applies+drops cleanly with ignored paths (node_modules) untouched; conflicting restore preserves the stash. --------- Co-authored-by: alt-glitch <balyan.sid@gmail.com>
…un crash) (NousResearch#39780) * fix(update): make ensure_uv() survive the update boundary (no first-run crash) `hermes update` runs the `ensure_uv()` call site from the old, already-imported `hermes_cli.main` against the *freshly pulled* `managed_uv` (managed_uv is only ever lazily imported, so it loads from disk post-pull). `ensure_uv()`'s return arity flipped from a single path string to `(path, fresh_bootstrap)` (4df280d) and back to a single string (fb853a1). Installs parked on a 2-tuple release unpack `uv_bin, fresh_bootstrap = ensure_uv()` against the new single-value module and crash the first update with `ValueError: not enough values to unpack (expected 2, got 1)` — inside the dependency-install step, *before* the PR NousResearch#39763 subprocess hand-off can run. Return a `_UvResult` (a `str` subclass) that is usable as the bare path AND unpackable as `(path|None, fresh_bootstrap)`. Missing uv is `""` (falsy) instead of `None` so legacy 2-target call sites can unpack a failure without raising, while `if not uv_bin` keeps working for single-value callers. fresh_bootstrap is always False (the rebuild-venv path it gated was scrapped in fb853a1). * docs(update): correct the verified error string + mechanism for ensure_uv() A hermetic repro (old 2-target call site vs the freshly-pulled single-value module) shows the first-update crash is exactly the string from PR NousResearch#39763's report: `ValueError: too many values to unpack (expected 2)` — not "not enough". The returned path is a plain `str`, which is iterable, so `uv_bin, fresh = ensure_uv()` walks its characters; the failure path's `None` return raises `TypeError: cannot unpack non-iterable NoneType`. Both are fixed by `_UvResult`. Comment/test wording updated to match; no behavior change.
* feat(desktop): per-profile remote gateway hosts Profile switching silently failed whenever the desktop was connected to a remote backend: the rail routed non-active profiles to a local pool backend, but spawnPoolBackend hard-threw "Profiles are unavailable when connected to a remote Hermes backend", and the renderer swallowed the error into an infinite reconnect backoff while still marking the profile active. Remote was also a single app-global setting, so there was no way to give a profile its own host. Add per-profile remote hosts so each profile can point at its own backend: - connection.json gains a validated `profiles` map; profileRemoteOverride() (pure, unit-tested) selects an explicit per-profile remote. - resolveRemoteBackend(profile) precedence: per-profile override → env override → global remote → local spawn. spawnPoolBackend now connects to a profile's remote (no local child) instead of throwing; startHermes resolves the primary profile's remote. - coerce/sanitize connection config are scope-aware (global vs named profile) and preserve each other's entries; IPC get/save/apply/test thread an optional profile. Per-profile apply drops only that profile's pool backend. - Settings → Gateway adds an "Applies to" scope selector reusing the existing URL/token/OAuth/test UX per profile. Tests: connection-config pure suite (+6) and desktop platform suite pass; tsc/eslint/vitest clean. * refactor(desktop): DRY per-profile remote helpers Share connectionScopeKey + normAuthMode from connection-config.cjs (drop the main.cjs copy), collapse the scope/auth ternaries, route the env remote through buildRemoteConnection, and fold the duplicated remote-block validation into buildRemoteBlock. No behavior change; pure suite + live E2E still green.
…ousResearch#39819) Two switch-time regressions from the multi-profile rail work: - "Session not found" (4007): pruneSecondaryGateways idle-reaps a non-active profile's backend; switching back respawns a *fresh* backend that mints new runtime ids, but runtimeIdByStoredSessionId is never pruned. resumeSession's cache fast-path then makes a dead runtime id active and returns, so session.usage + the next prompt 404. Probe the cached id; on rejection drop the stale mapping and fall through to a full resume that rebinds a live id. - "Forgets the LLM setting": $currentModel is a nanostore set only by refreshCurrentModel (gatewayState->open, etc). A swap fires invalidateQueries() (react-query only) and keeps the socket 'open', so the model/pill kept showing the previous profile. Re-pull both when $activeGatewayProfile changes.
…gv crash) (NousResearch#39820) PR NousResearch#39780 made ensure_uv() return a _UvResult — a str subclass whose __iter__ yields (path, fresh_bootstrap) so old `uv_bin, fresh = ensure_uv()` call sites survive the update boundary. That trick is unsafe on Windows. The dependency installer passes uv straight into the command list (`[uv_bin, "pip", "install", ...]`). On Windows, subprocess serializes argv via subprocess.list2cmdline, which iterates every entry *as a string* (`for c in arg`). Because _UvResult overrides __iter__, that iteration yields (path, fresh_bootstrap) instead of characters, injecting the bool into the command line and crashing the first update with: TypeError: sequence item 1: expected str instance, bool found This bites the common single-assignment caller (`uv_bin = ensure_uv()`) on its first update after NousResearch#39780: the freshly pulled _UvResult flows into the old in-memory call site and into the argv. Reported in the field on a ~10-commits-behind Windows install. A single return value cannot satisfy both legacy 2-target unpacking and Windows char-iteration — both use the iterator protocol with contradictory results. So gate the wrapper to POSIX: Windows returns a plain str/None (the historical, subprocess-safe contract). POSIX keeps _UvResult and the NousResearch#39780 update-boundary fix. Tests: list2cmdline canary proving _UvResult breaks Windows, plus Windows returns-plain-str and POSIX dual-contract coverage.
NousResearch#39828) * fix(desktop/windows): stop racing our own backend during in-app update The Windows in-app update (Update button -> hermes-setup.exe --update handoff) bricked because it raced a still-locked hermes.exe: the desktop quit fire-and-forget without reaping its backend child + grandchildren, so when the updater ran `hermes update`, the venv shim was still open. The quarantine rename then failed, uv's `pip install -e .` hit "Access is denied", the git path bailed to a full ZIP re-download, and the deps still couldn't write the locked shim -- leaving a half-applied install. macOS is fine because it never blocks REPLACE on a running executable. Three coordinated fixes restore Mac-style parity (click Update -> progress -> relaunch, no terminal): A. Desktop (main.cjs): before spawning the updater, releaseBackendLockForUpdate() tree-kills the primary + pool backends (taskkill /T /F on Windows, to catch REPL/pty/gateway grandchildren that SIGTERM misses) and polls the venv shim until it is actually writable (bounded 15s) -- so the lock is gone before we hand off. Also fixes resolveHermesCliBinary to use venv\Scripts\hermes.exe on Windows. B. Updater (update.rs): wait_for_venv_free no longer "proceeds anyway" on timeout -- it force-kills any lingering hermes.exe (excluding itself) and re-checks, so a straggler can't doom the install. C. Updater (update.rs): pass --force to `hermes update`. By contract the desktop has exited + waited, and the wait force-kills stragglers, so the running-exe guard would only produce a false "Hermes is still running" dead-end. Verified: node --check on main.cjs, cargo check on the updater (clean), and the Windows-gated taskkill body type-checks standalone. Field repro: ryanc's update.log (manual + handoff both hit the same lock cascade). * review: scope backend kill+wait to Windows; drop meaningless POSIX pgid kill
…crash (NousResearch#39831) The in-app updater (Hermes-Setup --update) runs `hermes update`, which lazily imports the freshly-pulled modules — but the dependency-install step runs the already-in-memory PRE-pull code for one invocation. When a release changes an updater-path contract across that boundary, the FIRST update on the parked population crashes even though the fix is already on disk. Concretely this is NousResearch#39780's `_UvResult`: its `__iter__` yields (path, bool), so Windows `subprocess.list2cmdline([uv_bin, "pip", ...])` injects the bool and dies with `TypeError: sequence item 1: expected str instance, bool found` (fixed in NousResearch#39820). A parked Windows user clicking Update pulls NousResearch#39820 to disk, then still crashes on the in-memory pre-merge module; only the SECOND click runs clean. Field repro: ryanc's bootstrap.log (2026-06-05 12:41:41). Fix: when the first `hermes update` exits non-zero (and it isn't the concurrent-instance guard, exit 2, which a retry can't fix), retry once automatically. The retry loads the now-current module from the start and succeeds — so the parked user gets a working one-click update instead of a scary crash + manual second attempt. Verified: cargo check clean.
…esearch#39727) * fix: respect disabled auto-compaction on context overflow Port from anomalyco/opencode#30749. When compression.enabled is false, NO automatic compaction trigger may fire. The proactive token-threshold paths (preflight + post-response should_compress gate) already honoured the setting, but the three provider-overflow recovery paths in the agent loop — long-context-tier 429, 413 payload-too-large, and context-overflow — called _compress_context() unconditionally, silently compressing and rotating the session against the user's explicit choice. Add a single guard at the top of the overflow-recovery dispatch: when compression is disabled and the error is one of those three overflow classes, surface a terminal error (compaction_disabled: True) telling the user to /compress manually, /new, switch to a larger-context model, or reduce attachments. Manual /compress (force=True) is unaffected — it never enters this loop. Tests: new TestOverflowWithCompactionDisabled (413 + 400 overflow don't compress when disabled; control case still compresses when enabled). Existing overflow-recovery tests updated to enable compaction explicitly (they verify the recovery fires); fixture defaults flipped to True to match production (compression.enabled defaults to True). * fix(completion): remove /model <arg> autocomplete from CLI/TUI The TUI frontend already suppressed /model argument completion in favor of the two-step ModelPicker (useCompletion.ts), but the CLI prompt_toolkit completer and the gateway-backed complete.slash RPC (TUI + desktop) still emitted model aliases and probed LM Studio on every keystroke. Drops the /model branch in SlashCommandCompleter.get_completions, the _model_completions method, and the LM Studio probe/cache helper that only fed it. Command-name completion (/mod -> model) and sibling arg completers (/skin, /personality) are untouched. Removes the now-dead TestModelTabCompletion tests.
* docs: remove --include-desktop install instructions Drop the --include-desktop curl one-liner from the desktop app docs. The flag remains in scripts/install.sh; these docs now point to the desktop installer / website and the 'hermes desktop' path instead. * docs: remove --include-desktop from install docs Drop the redundant 'Hermes Desktop installer on Linux' block (which used --include-desktop) from quickstart, installation, and index docs. The website installer covers macOS/Windows desktop; the CLI-only path covers Linux. Removes the flag from all user-facing docs.
…sResearch#39847) * fix: respect disabled auto-compaction on context overflow Port from anomalyco/opencode#30749. When compression.enabled is false, NO automatic compaction trigger may fire. The proactive token-threshold paths (preflight + post-response should_compress gate) already honoured the setting, but the three provider-overflow recovery paths in the agent loop — long-context-tier 429, 413 payload-too-large, and context-overflow — called _compress_context() unconditionally, silently compressing and rotating the session against the user's explicit choice. Add a single guard at the top of the overflow-recovery dispatch: when compression is disabled and the error is one of those three overflow classes, surface a terminal error (compaction_disabled: True) telling the user to /compress manually, /new, switch to a larger-context model, or reduce attachments. Manual /compress (force=True) is unaffected — it never enters this loop. Tests: new TestOverflowWithCompactionDisabled (413 + 400 overflow don't compress when disabled; control case still compresses when enabled). Existing overflow-recovery tests updated to enable compaction explicitly (they verify the recovery fires); fixture defaults flipped to True to match production (compression.enabled defaults to True). * perf(/model): prewarm picker provider-models cache in background The no-args /model picker calls list_authenticated_providers(), which fetches each authenticated provider's live /v1/models list serially. On a cold or stale (>1h TTL) cache that blocks ~1.5s on the user's critical path the first time /model is opened in a session. Warm that exact path off-thread during the idle window right after the CLI banner is shown: a once-per-process daemon thread runs list_authenticated_providers() to populate provider_models_cache.json for every authed provider. By the time the user types /model, the picker hits the warm disk cache (~136ms vs ~1500ms). Process-level Event guard (mirrors run_agent's _openrouter_prewarm_done) ensures at most one thread per process; fully exception-isolated so an offline/no-creds provider can never affect the session.
… backend Per-profile remote hosts (NousResearch#39778) wired the chat/resume socket to a profile's remote backend, but session list + transcript reads still assumed every profile's state.db is a local file the primary can open. For a remote profile the local file is absent or stale, so the IDs the sidebar shows 404 the moment resume runs against the remote -- the "session not found -> new session" bug. Intercept the three session-read GETs in the hermes:api handler and route them to the owning remote backend (which serves its own state.db natively): GET /api/profiles/sessions -> splice each remote profile's real rows in GET /api/sessions/{id}[/messages] -> read from the remote for remote profiles No remote profiles configured -> untouched local fast path. A dead remote contributes nothing rather than breaking the sidebar. Verified end-to-end against a live remote backend: a remote-profile session resumes from remote history and continues on the remote across turns (history grows in place, no new session spawned).
…st pagination
Follow-up to the read-routing fix: make remote-profile sessions fully
first-class, not just resumable.
Mutations (rename/archive/delete) went through the same hermes:api handler but
never carried the owning profile, so they hit the local primary's state.db --
which has no row for a remote session. Deleting/archiving/renaming a remote
session silently no-op'd or 404'd, and the row reappeared on next refresh.
- hermes.ts: setSessionArchived/deleteSession/renameSession take the owning
profile and pass it as request.profile so Electron routes to that profile's
backend (matching the read path). Callers now forward session.profile.
- main.cjs: generalize the intercept (read -> request) to also reroute
DELETE/PATCH on /api/sessions/{id} for remote profiles, stripping the profile
param (the remote serves its own state.db; no cross-profile semantics there).
- web_server.py: DELETE /api/sessions/{id} gains a profile param for parity with
GET/PATCH (local cross-profile delete).
Also fix the unified-list merge: it concatenated each remote's page onto the
primary's without re-windowing, so a limit=N request could return up to
N*(1+remotes) rows and report the primary's (stale) total. Now it over-fetches
limit+offset from each remote (from offset 0), re-sorts by recency, re-windows
to the page, and recomputes total/profile_totals from the remote counts.
Verified live against a remote backend: rename/archive/delete mutate the remote
db; page 1 windows to limit, profile_totals reflect remote counts, page 2 has no
overlap with page 1. tsc -b clean; connection-config tests pass.
… rename/archive/delete) (NousResearch#39894) * fix(desktop): route remote-profile session reads to the owning remote backend Per-profile remote hosts (NousResearch#39778) wired the chat/resume socket to a profile's remote backend, but session list + transcript reads still assumed every profile's state.db is a local file the primary can open. For a remote profile the local file is absent or stale, so the IDs the sidebar shows 404 the moment resume runs against the remote -- the "session not found -> new session" bug. Intercept the three session-read GETs in the hermes:api handler and route them to the owning remote backend (which serves its own state.db natively): GET /api/profiles/sessions -> splice each remote profile's real rows in GET /api/sessions/{id}[/messages] -> read from the remote for remote profiles No remote profiles configured -> untouched local fast path. A dead remote contributes nothing rather than breaking the sidebar. Verified end-to-end against a live remote backend: a remote-profile session resumes from remote history and continues on the remote across turns (history grows in place, no new session spawned). * fix(desktop): route remote-profile session mutations + fix unified-list pagination Follow-up to the read-routing fix: make remote-profile sessions fully first-class, not just resumable. Mutations (rename/archive/delete) went through the same hermes:api handler but never carried the owning profile, so they hit the local primary's state.db -- which has no row for a remote session. Deleting/archiving/renaming a remote session silently no-op'd or 404'd, and the row reappeared on next refresh. - hermes.ts: setSessionArchived/deleteSession/renameSession take the owning profile and pass it as request.profile so Electron routes to that profile's backend (matching the read path). Callers now forward session.profile. - main.cjs: generalize the intercept (read -> request) to also reroute DELETE/PATCH on /api/sessions/{id} for remote profiles, stripping the profile param (the remote serves its own state.db; no cross-profile semantics there). - web_server.py: DELETE /api/sessions/{id} gains a profile param for parity with GET/PATCH (local cross-profile delete). Also fix the unified-list merge: it concatenated each remote's page onto the primary's without re-windowing, so a limit=N request could return up to N*(1+remotes) rows and report the primary's (stale) total. Now it over-fetches limit+offset from each remote (from offset 0), re-sorts by recency, re-windows to the page, and recomputes total/profile_totals from the remote counts. Verified live against a remote backend: rename/archive/delete mutate the remote db; page 1 windows to limit, profile_totals reflect remote counts, page 2 has no overlap with page 1. tsc -b clean; connection-config tests pass.
Replace legacy postMessage → chat.update edit loop with Slack's native chat.startStream / chat.appendStream / chat.stopStream streaming APIs. This produces native collapsible step cards with checkmarks, chevrons, and status indicators — the same UI pattern used by Slack AI and Highbeam's Luma. Key changes: - New SlackStreamConsumer (slack_stream.py) that uses task_start and task_update chunks for tool progress, and markdown_text chunks for streaming tokens - Slack adapter: added set_thread_title(), set_suggested_prompts(), and create_stream_consumer() methods - run.py: when platform is Slack with a thread_ts, automatically use SlackStreamConsumer instead of GatewayStreamConsumer; falls back to legacy edit-based streaming on failure - Tool progress callback routed through on_tool_progress() for native step indicators when Slack native streaming is active - 20 unit tests for SlackStreamConsumer lifecycle, all passing - Existing 283 Slack tests still passing
… raw dicts Prefer slack_sdk.models.messages.chunk.TaskUpdateChunk for type-safe chunk construction. Falls back to raw dicts for slack_sdk < 3.35.
…gh Response step Slack's streaming API enforces mode isolation: streams started with task_display_mode='plan' can only accept chunks, not markdown_text. Mixing them raises 'streaming_mode_mismatch'. Refactored the consumer to: - Use plan mode exclusively for the entire interaction - Create a 'Response' step whose TaskUpdateChunk.output field carries the LLM's text output (buffered and flushed periodically) - Never use markdown_text in appendStream calls E2E tested against live Slack API — all streaming methods verified.
… titles Three fixes to SlackStreamConsumer: 1. Emit initial 'Processing' step immediately after startStream to replace Slack's built-in 'Gathering information...' placeholder. The step is completed as soon as the first real content (tool start or text delta) arrives, so the phantom message never appears. 2. Format tool names as human-readable step titles using a _TOOL_DISPLAY_NAMES mapping and Title Case fallback. Tool previews (primary args) are appended: 'Search files: *.py' instead of raw 'search_files'. This replaces the opaque blinking 'Response' with actual activity context. 3. Fix missing separator before 'Done' in completed step descriptions. The in-progress description (e.g. 'command: ls -la') is now preserved and separated from the completion status with a newline, producing 'command: ls -la\nDone (0.2s)' instead of concatenated gibberish.
…ndant Done - Always build desc from args dict (even when preview is provided) so the subtitle line shows context like 'command: ls -la' during and after tool execution - Remove 'Done' from completed step descriptions — the checkmark icon already indicates completion; just show the duration '(0.8s)' - Preserve the full title with args/preview on completion instead of reverting to bare tool name like 'Terminal'
…tails The newline separator wasn't rendering properly in Slack's step card details field, causing text like 'command: ls -la(0.8s)' instead of 'command: ls -la (0.8s)'. Use a space separator instead.
Slack's TaskUpdateChunk details field APPENDS across updates rather than replacing, so sending the full description on completion duplicated it. Now only the duration string is sent on completion (e.g. ' (0.8s)'), which appends cleanly to the in-progress description. Also: completed step title now uses the original preview (e.g. 'Terminal: ls -la') instead of the raw args description (e.g. 'Terminal: command: ls -la'). The preview is stored on tool.started and reused on tool.completed via _active_task_previews dict.
Three fixes for Slack native Steps API streaming: 1. Response text duplication: _flush_text and _finalize were sending the full text buffer each time, but Slack's chat.appendStream APPENDS the output field (same as details). Now sends only the delta since last flush via _total_text_sent tracking. 2. Phantom progress bubbles: When Slack native streaming is active, the legacy progress_callback was still queuing events to the send_progress_messages task, creating duplicate progress bubbles below the stream card. Now suppressed via _slack_native_stream_active. 3. Phantom status messages: The status_callback was still sending 'Analyzing...' / 'Summarizing findings...' as separate messages below the stream card. Now suppressed when native streaming is active.
…ream When Slack's server closes a stream (timeout or error), every subsequent appendStream call fails with 'message_not_in_streaming_state'. Previously each failure was logged as a WARNING, creating noise and no recovery. Changes: - Add _stream_broken flag: set on first message_not_in_streaming_state error, suppresses all further appendStream/stopStream calls - Validate startStream response: check ok=True, raise on failure so run() bails out early instead of silently proceeding - Fail fast on initial appendStream: if the very first task step after startStream fails, raise immediately rather than spamming failures - Extract _try_stop_stream helper: handles message_not_in_streaming_state gracefully at debug level - Short-circuit _finalize when stream is already broken
…te in _flush_text _flush_text caught the exception but only logged a warning — it never set _stream_broken=True, so subsequent flush calls kept retrying against a closed stream, producing hundreds of repeated warnings in gateway.log.
…id-delivery When Slack's chat.appendStream returns message_not_in_streaming_state (the stream was closed server-side), the SlackStreamConsumer sets _stream_broken=True and silently stops trying to deliver content. The user sees Slack's generic 'Something went wrong' step card with no actual response text. This commit adds a fallback path in _finalize: when the stream is broken, deliver the accumulated text via chat.postMessage so the user still receives the response content in the thread, even though the step card shows an error state. Also exposes stream_broken, accumulated_text, and final_content_delivered as public properties so the gateway can correctly detect when the stream failed and avoid duplicate sends (the gateway already checks final_content_delivered via getattr).
…stMessage fallback The Slack SDK's SlackApiError.__str__() does NOT include the actual error code (message_not_in_streaming_state) — it only shows a generic message like 'The request to the Slack API failed. (url: …)'. The real error code lives in e.response.data['error'], so all previous checks using 'message_not_in_streaming_state' in str(e)' were silently failing. This meant _stream_broken was never set, and the _fallback_post_message path (which delivers the response text via chat.postMessage when the stream dies) was never triggered. Users were left staring at Slack's 'Something went wrong' step card with no content. Changes: - Add _is_stream_closed_error() helper that checks both str(e) and e.response.data['error'] for robust detection across SDK versions - Replace all 5 str(e) checks with the helper function - When stream breaks during _finalize (task completion or text flush), immediately fall back to chat.postMessage instead of just logging - Add 9 new tests covering error detection and fallback behavior
…eam message Two fixes for the Slack Steps API streaming consumer: 1. **Markdown formatting in fallback postMessage**: The fallback chat.postMessage was sending raw markdown text without converting it to Slack mrkdwn. Now passes SlackAdapter.format_message as a callable to SlackStreamConsumer, which applies proper conversion (headers, bold/italic, links, code blocks) in the fallback path. 2. **Replace 'Something went wrong' stream card**: When the Slack stream breaks, the step card shows 'Something went wrong'. Now the fallback tries chat.update on the broken stream message first (Strategy 1) to replace the error card with properly formatted content. Only if chat.update fails does it fall through to chat.postMessage (Strategy 2). Tests: 50 pass (6 new), 194 Slack adapter tests still green.
…nation
Two improvements to prevent and gracefully handle Slack stream disconnections:
1. Keep-alive pings: When the stream is idle for keepalive_interval (default
120s), send a lightweight TaskUpdateChunk step ('Still working… (2m 30s)')
via appendStream to prevent Slack from closing the stream due to inactivity.
The step is updated on subsequent pings (not recreated) and completed when
real activity resumes. Configurable via keepalive_enabled and
keepalive_interval in SlackStreamConfig.
2. Fallback explanation: When the stream does break and content is delivered
via chat.postMessage (Strategy 2), prepend a brief explanatory message
('The streaming display disconnected, but the response was completed
successfully.') so users understand the 'Something went wrong' banner is
a display issue, not an actual failure. The explanation is skipped when
chat.update (Strategy 1) succeeds since the error card is replaced inline.
Configurable via fallback_explanation in SlackStreamConfig (set to '' to
disable).
Tests: 10 new tests (5 keep-alive + 4 fallback explanation + 1 config defaults).
All 59 stream tests + 283 Slack gateway tests pass.
…ring Switch task_display_mode from 'plan' to 'timeline' (now configurable). In timeline mode, response text is delivered via MarkdownTextChunk objects which render proper Slack mrkdwn (bold, italic, links, headers, code blocks). Plan mode only supports TaskUpdateChunk.output (256-char limit, no markdown rendering). Key changes: - SlackStreamConfig.task_display_mode: new field, default 'timeline' - _flush_text(): uses markdown_text chunks in timeline/dense mode, task_update.output only in plan mode - _finalize(): flushes remaining text via markdown_text in timeline mode - _make_markdown_text_chunk(): new helper building MarkdownTextChunk - _is_plan_mode(): helper checking display mode - Updated module docstring, start_stream docstring - Fixed importlib.util.find_spec ValueError in test skipif - New tests: TestMakeMarkdownTextChunk, TestIsPlanMode, test_plan_mode_uses_task_update_output
jarvisxyz
force-pushed
the
2026-06-03.eizus.slack-stream-steps-fix
branch
from
June 5, 2026 15:48
4209625 to
df1eabf
Compare
…sist steps after completion - Add plan_text_via_postmessage config (default True) to deliver response text via chat.stopStream's markdown_text param instead of separate postMessage, keeping step cards visible after completion - Add _skip_in_stream_text() to suppress TaskUpdateChunk.output in plan mode - Add _stop_stream_succeeded flag for fallback logic - Fix fallback condition: when stopStream rejects markdown_text (streaming_mode_mismatch), retry without text then fall back to postMessage for the response content - Add plan_text_via_postmessage to SlackStreamConfig dataclass - Wire task_display_mode from slack.extra config
jarvisxyz
pushed a commit
that referenced
this pull request
Jul 24, 2026
…e_check_xsrf pitfalls Add two pitfalls discovered when running the skill against a fresh Jupyter server: - Pitfall #9: When the websocket reply channel hangs on every execute even though the kernel actually ran (REST shows execution_state=idle and execution_count increments), force zmq transport with --transport zmq. The zmq transport uses jupyter_client directly and sidesteps the broken websocket layer. - Pitfall #10: A fresh ServerApp rejects POST /api/sessions with "_xsrf argument missing from POST" unless you start it with --ServerApp.disable_check_xsrf=True. Needed for REST-only flows where no browser/cookie is establishing the XSRF token.
jarvisxyz
pushed a commit
that referenced
this pull request
Aug 9, 2026
…lves A session title had no notion of who set it, so two bugs followed. An auto-generated title could clobber a name the user typed, and every compression rotation renumbered the conversation it forked - one piece of work reaching 'Smallville Map Architecture Plan #10' in the sidebar. Titles now carry a source (derived < llm < user) enforced by one compare-and-swap, so an automatic write can only ever replace a title of strictly lower authority. Compression carries the name across unchanged. Legacy NULL rows rank as user, so auto-titling only fills genuinely empty titles on existing data.
jarvisxyz
pushed a commit
that referenced
this pull request
Aug 21, 2026
… the relay (gateway half) (NousResearch#85796) * feat(relay): live-card ops — native draft streaming + task cards over the relay (gateway half) NS-658. Three additive ops within contract v1, emitted only when the connector's negotiated descriptor advertises them: {op: draft, chat_id, draft_id, content, final, metadata} {op: task_card, chat_id, card_id, chunks, metadata} {op: task_card_stop, chat_id, card_id, metadata} The gateway side is deliberately dumb: no platform API knowledge, no new config keys. Slack mechanics (chat.startStream/appendStream/stopStream, per-workspace feature-gate cache, send+edit fallback) live connector-side where the platform adapter lives in the relay model. Semantic bridge: base send_draft is Telegram-shaped (draft clears; final is a separate send). Slack native streaming makes the stream THE message. The adapter tracks the open draft per chat and converts the turn-final send() into draft(final=true) so the connector seals the stream instead of posting a duplicate; the stream ts returns as the message identity. A failed frame disarms interception so the edit-based fallback's real send goes through untouched. BEHAVIOR CHANGE (deliberate): relay supports_draft_streaming() now requires the descriptor flag AND the draft op. Flag-only was a latent lie — send_draft inherited NotImplementedError, so a connector setting the flag without the op would have crashed the stream consumer's draft path. supported_ops stays fail-open for legacy (pre-contract) ops; draft/task_card did not exist pre-contract and must not fail open. Task cards ride NousResearch#85476's adapter-agnostic TurnRunner seam (hasattr on send_native_task_card_progress); supports_native_task_cards() is the descriptor probe. Connector half + E2E harness pair follow in the gg repo. * fix(relay): expose native_task_cards_enabled() on the relay adapter Live-canary finding (Alice, staging): the TurnRunner's task-card lane probes adapter.native_task_cards_enabled() (the native Slack adapter's opt-in contract). The relay adapter only offered supports_native_task_cards(), so the hasattr gate failed silently and tool progress stayed on the text path — draft streaming worked, cards never rendered. Alias it to the descriptor probe. * fix(relay): match task-card methods to the TurnRunner's native keyword contract Live-canary finding #2 (Alice, staging): gateway/run.py's card lane calls send/stop_native_task_card_progress with the NATIVE Slack adapter's signature (tasks/title/reply_to/metadata/fallback_text, keyword-only) — PR 85796's relay methods took a positional card_id, so every call raised TypeError('unexpected keyword argument reply_to') in the progress task, repeatedly killing the card publisher (and the retry loop resent the final delivery 4-5x). Card id now derives per turn thread (turn:<reply_to>), thread_ts anchored like draft; title/fallback_text accepted for parity, not forwarded (plan-mode stream renders chunks). * fix(relay): one draft stream per turn for stream-is-the-message adapters Live-canary finding #4 (Alice, staging): the stream consumer bumps draft_id at every tool boundary so Telegram-shaped drafts animate each text segment as a fresh preview. On relay Slack NATIVE streaming a new draft_id opens a brand-new chat.startStream — the user saw one frozen message per segment (stuck streaming cursor ▉, never sealed: only the LAST stream gets the final=true seal) plus the real final; 5-6 cumulative snapshots per turn. Adapters that mark draft_stream_is_message keep ONE stream per turn: tool progress lives in the native task card, and the connector's suffix-delta falls back to whole-text append on prefix mismatch, so segments append cleanly. Telegram-shaped drafts keep the per-segment bump. * fix(relay): don't seal the native stream at tool boundaries — only the turn-final does Live-canary finding #5 (Alice; supersedes the incomplete #4 which was necessary but not sufficient). Root cause CONFIRMED by integration trace (test_live_cards_flow_trace.py, real consumer semantics + real adapter + stub transport): at every tool boundary the consumer calls _send_or_edit(finalize=True), which skips the draft path and issues a real send(); the relay adapter's seal-interception converts THAT into draft(final=true) — sealing the stream once per segment. Timeline showed 3 seals for a 3-segment turn: exactly the frozen cumulative ▉ snapshots seen live (the replaced stream never gets stopStream, keeping its cursor). Fix: for draft_stream_is_message adapters, a segment-break finalize (finalize=True, is_turn_final=False) stays ON the draft path as another cumulative frame; only got_done (is_turn_final=True) falls through to send() and seals. Telegram-shaped platforms unchanged. Trace test now pins the invariant: ONE user-visible message per turn. * fix(relay): strip the text cursor from native draft frames Live-canary finding #6 (Alice) — the ACTUAL duplicate-content mechanism, confirmed by full-flow scan of both sides' code + logs. The consumer appends its text cursor (▉) to every non-final display_text tick. The connector's stream sender diffs CUMULATIVE frames via prefix check: 'abc▉'.startsWith → 'abc def▉' is NEVER a prefix match (the cursor sits mid-string), so deltaFor falls back to whole-text append on EVERY tick — chat.appendStream stacks each full cumulative snapshot (cursor included) into the ONE stream message. Exactly the observed thread: repeated blocks, each ending in a frozen ▉, growing per tick. Fixes #4/#5 were real (one stream per turn now) but this was the last mechanism standing. Native streams render their own typing indicator, so the text cursor is pure noise on this path: strip it from draft frames. Prefix check now holds; every tick appends only its true suffix delta. * fix(relay): seal-interception covers EVERY egress door, not just send() Live-canary finding #7 (Alice): one duplication remained after #6 — the stream froze mid-word with the live indicator (never sealed) and the final posted as a separate message. Log receipt: 'Queued follow-up: final text delivery confirmed; delivering explicit media before continuing' — the turn's final went out via the DELIVERY RESOLVER lane (gateway/delivery.py), which calls send_for_platform() DIRECTLY, bypassing send() and its seal-interception. The open stream never absorbed the final; it arrived as a plain 'send' op → chat.postMessage. Fix: hoist the open-draft check to the top of send() (ahead of the explicit-platform branch) AND add it to send_for_platform() — an open native stream absorbs the turn-final regardless of which egress door it arrives through. The stream IS the message. * fix(relay): failed seal falls back to plain send (PR 85796 AI-review point 1) A turn-final seal that fails at the transport must never swallow the final answer: the stream consumer has already disabled the draft transport for the run, so a failed _seal_open_draft returning success=False meant the user got NOTHING. Both seal-interception sites (send + send_for_platform) now fall through to the regular plain-send path on seal failure, with a warning receipt. Also mitigates AI-review point 2 (sticky _open_draft_by_chat after an abandoned turn): a stale entry's failed seal no longer blocks the next turn's delivery. * fix(relay): arm seal-interception optimistically; never disarm on ambiguous failure (audit G-D1) Deep-audit defect G-D1 (HIGH): the outbound leg is at-most-once on the wire but its ack channel is lossy — send_outbound timeout (30s) and WS-drop 'failures' frequently mean the frame WAS delivered and the connector stream is open. send_draft popped _open_draft_by_chat on any failure, disarming seal-interception while the connector stream lived: the turn-final went out as a plain send → orphaned mid-word stream + complete duplicate final (intermittent; needs a drop/timeout inside the draft window). Fix: arm the entry BEFORE the transport call and keep it armed on failure/exception. Safe in every case: sealing a non-existent stream opens+seals a single complete message connector-side, and a truly failed seal already falls back to plain send at both interception sites. Stale-entry damage is self-healing (one warning + plain send). * fix(relay): gateway-side sealed-draft tombstone — G-D1 arming must not resurrect sealed streams Regression fix on G-D1 (live: 'worse than before' — escalating frozen prefixes). Optimistic arming had no seal-awareness: a straggler frame arriving AFTER the seal re-armed _open_draft_by_chat for the already- sealed draft_id; the next send was converted to draft(final=true) on the tombstoned connector key, which CLEARED the connector tombstone (final frame = new-turn signal), re-opened a stream with cumulative content, and left it frozen — repeating per straggler: 4-5 escalating frozen snapshots. Mirror the connector: _sealed_draft_by_chat records the sealed draft_id per chat (tombstoned BEFORE the seal's transport call); send_draft for a sealed draft_id is a success no-op (content already in the sealed message) and never arms. A new turn's fresh draft_id arms normally. * fix(relay): key stream/card state per (chat, turn anchor) — parallel turns must not collide (finding #10) Live finding #10 (Alice; three concurrent turns in one flat DM): all coordination state was keyed per CHAT on a one-active-turn assumption. Three parallel turns produced: turn B's task card merged into turn A's (both were card 'turn:root' — reply_to is None in flat DMs), B left cardless, and _open/_sealed_draft_by_chat clobbered across writers (3x duplicate finals on the last turn). Per-turn machinery was correct; the keys were not. Fix: _draft_key(chat, metadata) = chat + the turn's thread anchor (inbound stamps thread_ts = event.thread_ts or ts on every top-level message, so each turn has one even in flat DMs). draft arming, seal tombstones, both interception sites, and the task-card id all derive from the same anchor. New trace test pins two interleaved turns: distinct cards, own-stream seals, no leaked plain send, no cross-turn tombstone drops (289 tests green). * fix(gateway): preserve cumulative native stream across tools * fix(gateway): consumer-declared final — the seal carries the true final Three composed fixes for the Slack live-cards duplicate-final class: 1. finish(final_text): TurnRunner passes the completed final_response (verifier footer, completion explainer included) as the authoritative finalize payload. The native-stream seal delivers the TRUE final, so post-stream mutation no longer forks a corrective plain send (#11). 2. Interim-send contract: commentary and segment-tail sends carry a gateway-internal _interim_send marker; relay seal-interception skips them at both egress doors. A mid-turn interim send can no longer seal the live stream and orphan the real final into a duplicate. 3. Queued-follow-up lane reconciles an unconfirmed final by EDITING the consumer's delivered message in place (sealed stream = regular message, chat.update live-verified); plain send only as fallback. This was the actual duplicate lane in the parallel canaries — every duplicated turn logged 'final stream delivery not confirmed; sending first response' (subagent-completion queued inbound), not parallelism. Also: draft frames stay prefix-stable gateway-side (no fence-closing, no segment state reset, no commentary reset for stream-is-the-message adapters; MagicMock-safe 'is True' guards). * test+docs: streaming-contract coverage completeness + maintenance guidelines Coverage: two gaps closed on the consumer-declared-final contract — (1) send_for_platform (the delivery-resolver egress door) honors the _interim_send contract: no seal, marker stripped before the wire; (2) finish(final_text) on a turn that never streamed does not adopt the final (delivery ownership stays with the gateway's normal send path for non-streaming models / tool-only turns). Docs: AGENTS.md 'Known Pitfalls' gains the streaming delivery contract — the four invariants of stream-is-the-message adapters (prefix-stable frames, consumer-declared final, interim-send marker, reconcile-by-edit), each traced to its live incident, plus the live-probed Slack streaming API ground truth and the MagicMock 'is True' guard-style note. * fix(relay): seal transport failure must never silently lose the final (review B1) Two halves of one silent-loss path, live-probed on the review branch: 1. adapter: _seal_open_draft did not catch transport exceptions. A socket drop at seal time raised out of send(), skipping the fail-open plain send entirely. Now: retry the SAME idempotent final frame once (the connector's sealed-key tombstone returns the original stream ts for a repeated final — a retry can never open a second stream or duplicate), then report failure so the caller's fail-open path runs. 2. consumer: the turn-final retry (elif not _already_sent) called _send_or_edit with finalize=False, which re-entered the DRAFT-FRAME branch. Its no-op dedupe compared the adopted final against the last unsealed frame, matched, and returned True with ZERO transport calls — final_response_sent went green, delivered_final_matches reconciled, the gateway suppressed its fallback, and the user never received the answer. finalize=True keeps this retry out of the draft branch. Regression suite: tests/gateway/test_relay_seal_failure.py (3 tests). Mutation evidence in follow-up verification: reverting either half sends the suite red. * fix(relay): draft ids unique across gateway incarnations (review B3) The relay connector tombstones sealed streams by (channel, draft_id) and keeps up to 512 of them; they outlive the gateway process. Relay gateways are disposable BY DESIGN (scale-to-zero), and _draft_id_counter restarted at zero every incarnation — so the first turns after every scale-from-zero in a recently-active channel replayed already-sealed wire identities. The connector answered those frames straight out of the old tombstone: zero Slack API calls, the OLD message ts returned as the new turn's identity, the new answer silently dropped while gateway-side flags recorded success. Seed the counter from wall-clock milliseconds at process start. Ids stay plain ints within the existing contract op; incarnations cannot overlap for realistic turn counts and restart gaps. Regression: tests/gateway/test_draft_id_restart_uniqueness.py — the seed test fails on the old code (seed 0 is not epoch-scale). * fix(relay): stream/card state keyed per TURN, not per thread anchor (review B2) The thread anchor is the wrong coordination identity — simultaneously: - too coarse: two parallel turns replying INSIDE ONE Slack thread share thread_ts. Live-probed on the review branch: turn A's final sealed turn B's stream with A's content while A's own stream stayed open, and B's final degraded to a plain send. - too fragile: a flat DM with no thread metadata degraded to the bare chat id, re-creating the original finding-#10 collision the anchor was meant to fix. _draft_key now prefers the triggering inbound message id (message_id / reply_to_message_id — per-turn by construction; the gateway's Slack thread metadata and the consumer's send path both stamp it), falling back to the thread anchor, then the bare chat. The consumer stamps the same reply_to_message_id on draft frames so frames and the turn-final resolve to one key. Task-card ids share the derivation via _card_key (one helper for send AND stop, so the stop always hits the stream the send opened). Legacy resolver-lane callers with placement-only metadata still seal via _match_open_draft's fallback — but ONLY when exactly one stream is open. With several open, an identity-less send stays a plain send: a duplicate message is recoverable, sealing someone else's stream is not. Regression: tests/gateway/relay/test_relay_turn_keying.py (7 tests). * fix(relay): stream-is-the-message is a Slack semantic, gate it on the descriptor (review B4) draft_stream_is_message was hardcoded True on the relay adapter class, i.e. for EVERY relay platform. The base send_draft contract is Telegram-shaped — the draft clears client-side and the final arrives as a separate real send that becomes the history message. With the flag forced on, any non-Slack connector advertising the draft op had its turn-final intercepted into draft(final=true): probed on the review branch with a telegram descriptor, the op stream was [draft(final=false), draft(final=true)] and NO send — no history message would ever be posted. Gate the flag on the negotiated descriptor platform (slack), and skip arming seal-interception entirely when it is off. A future platform with genuine stream-is-the-message native streaming should advertise it via the descriptor rather than widening the platform check by guesswork. Regression: tests/gateway/relay/test_relay_stream_semantics_gating.py (4 tests: gating both ways, telegram final is a real send, slack final still seals). * fix(gateway): mark every mid-turn status lane interim — heartbeats must not seal the stream (review B5) Seal-interception treats the first unmarked send to an armed (chat, turn) key as the turn-final. The consumer's own interim lanes (commentary, tail flush) carry _interim_send, but four gateway-side lanes that fire DURING a streaming turn did not: - long-running heartbeat (default every 180s — probed live: at 3 minutes it sealed the live stream with '⏳ Working — 3 min', the real final posted as a duplicate, and later frames were silently swallowed by the seal tombstone) - inactivity warning - plain-text approval fallback (button lane failed) - background-review notice Add _interim_metadata() beside _non_conversational_metadata and wrap all four call sites. The marker is gateway-internal; the relay adapter strips it before the wire (existing behavior, pinned by test). Note for follow-up: the opt-out shape remains fragile — any FUTURE unmarked mid-turn send lane re-creates this bug. Inverting the contract (explicitly mark the one turn-final send) is the durable fix but touches every adapter's final-delivery path; deliberately kept out of this review-fix series. Regression: tests/gateway/test_interim_send_lanes.py (4 tests). * fix(gateway): interrupted/incomplete turns must not adopt the diagnostic as the stream final (review B6) The finish(final_text) adoption gate checked only 'not failed', but the interrupt/abort returns in agent/conversation_loop.py are {completed: False, interrupted: True, final_response: 'Operation interrupted during …'} with NO failed key. Adopting that diagnostic: 1. sealed the user's streamed partial answer over with the interrupt text (stream-is-the-message: the seal rewrites the whole message), and 2. recorded the diagnostic as the turn-final payload, so delivered_final_matches reconciled and the gateway suppressed its own error-delivery path — the diagnostic became the ONLY thing delivered. Enumerated all 27 final_response-bearing return shapes in conversation_loop.py: every non-happy-path shape carries completed: False (several with a diagnostic final_response and neither failed nor interrupted — retry exhaustion, truncation, codex-incomplete); the happy path routes through turn_finalizer.finalize_turn (completed=True). Gate is therefore: not failed AND not interrupted AND completed is not False. Results lacking the completed key entirely (older callers/test doubles) keep the previous behavior. Regression: tests/gateway/test_stream_final_adoption_gate.py (6 tests, incl. a source-level pin on the run.py call site). * fix(relay): task-card transport failures degrade to failed SendResults (review B7) send_native_task_card_progress and stop_native_task_card_progress let transport exceptions escape. The stop runs inside the progress loop's finally block on the turn-cleanup path, and the post-cancel awaits in gateway/run.py caught only CancelledError — a socket drop during a card publish/stop therefore aborted cleanup BEFORE the final-delivery bookkeeping ran. Three layers, outermost defends any adapter: - both adapter methods catch transport exceptions and return failed SendResults (progress is advisory; the TurnRunner's text fallback already handles failure results) - the progress loop's finally wraps the stop (best-effort; the connector seals orphaned card streams on its own via recycling/eviction) - the cleanup awaits log-and-continue on non-cancellation errors so final-delivery bookkeeping always runs Regression: tests/gateway/relay/test_relay_task_card_failures.py. * fix(relay): a dying turn seals its native stream instead of orphaning it (review B8) Stale-generation exits (/new, /stop mid-stream) and cancellations returned from the consumer's run() with the native stream still open: - the Slack message kept its live streaming indicator forever (the cancellation best-effort edit only runs when _message_id exists, and the native draft path deliberately keeps it None); - the adapter's armed interception state survived the turn, so the next turn on the same key could inherit it and seal a dead draft_id. New adapter op abandon_open_draft(chat, content): seals in place with the text already on screen (the consumer passes its last delivered frame) — the seal adds nothing and claims nothing; delivery flags are never set, so the gateway's normal paths still own whatever happens next. Best-effort by contract (failure reported, never raised); the connector reaps truly orphaned streams via recycling/eviction. The consumer calls it from both death paths: the stale-generation early return and the CancelledError handler. Regression: tests/gateway/test_stream_abandon_on_turn_death.py (4 tests, incl. the next-turn-inheritance hazard). * fix(relay): bound the draft/seal coordination dicts (review M1) _sealed_draft_by_chat's key embeds a per-turn identity, so every completed turn wrote a permanent entry — unbounded growth for the life of a long-running gateway process (the docstring said 'one entry per chat', which stopped being true when the key gained the turn anchor). _open_draft_by_chat could grow the same way via abandoned entries. FIFO-evict both at 512 entries — the same idiom as the sibling bounded cache (_auto_thread_by_chat, capped at 256) and the same size as the connector's own tombstone store. The straggler window the tombstone exists for is seconds long; FIFO is more than enough. Regression: tests/gateway/relay/test_relay_state_bounds.py. * fix(relay): explicit connector rejection disarms interception; exceptions stay armed (review P3) The G-D1 optimistic-arming change silently dropped disarm-on-failure entirely: after an EXPLICIT connector rejection (success=False result — not a transport ambiguity), interception stayed armed even though the stream consumer disables the draft transport on that failure and falls back to edit-based streaming. Its turn-final would then be converted into a seal on a stream the connector just told us is unusable. test_draft_failure_result_propagates claimed to cover this ('must NOT leave seal-interception armed') but passed for an unrelated reason: the stub's canned failure also failed the SEAL, whose fail-open path did the plain send. Split the two semantics and pin each honestly: - explicit rejection (result success=False): disarm — turn-final is a real send (test_draft_failure_result_propagates, now testing what its comment says) - transport exception: ambiguous, stay armed — turn-final still seals (test_draft_transport_exception_keeps_interception_armed, the G-D1 contract) Also corrects commit ba3a24a's claim ('a failed frame disarms interception so the edit-based fallback's real send goes through untouched') to hold again for the rejection case it described. * fix(relay): lost acks are ambiguous, not rejections — on the RESULT channel too (review r2, finding 1) The production ws transport does not raise on ack timeout — it returns {"success": False, "error": "relay outbound timed out"}. The round-1 ambiguity handling keyed entirely on the exception channel, so the shape production actually produces was misclassified as a definite connector rejection. Probed on the head: - lost SEAL ack: skipped the idempotent retry, fell straight to a plain send — duplicate final whenever the seal had actually applied; - lost FRAME ack: the round-1 disarm-on-rejection fired — interception disarmed, frozen native stream beside a plain final. This re-created the original G-D1 ambiguous-ack defect on the result channel. Contract now spans both channels: - transport: the ack-timeout branch tags ambiguous=True. The fail-fast branches (closing / not connected) never sent anything and stay unmarked — they are definite non-delivery. - adapter frame path: ambiguous results keep interception armed (same as exceptions); only definite rejections disarm. - adapter seal path: one shared _attempt() classifier — exception and ambiguous result both mean "unknown"; the SAME idempotent frame is retried once (connector tombstone returns the original stream ts for a repeated final). Only after both attempts stay ambiguous does the caller's fail-open plain send run: a possible duplicate after double ack loss beats a silent loss, and double ack loss on one socket almost always means the transport is down for the plain send too. Regression: tests/gateway/relay/test_relay_ack_ambiguity.py (6 tests, incl. a source-of-truth check that the transport tags the timeout branch and leaves fail-fast branches unmarked). * fix(relay): stream semantics + draft capability resolve per CHAT, not per primary (review r2, finding 2) One RelayAdapter fronts N platforms (Phase 1.5): descriptors accumulate per platform on the transport and egress is tagged per chat — but the round-1 gate keyed draft_stream_is_message and supports_draft_streaming() off the PRIMARY scalar descriptor. Probed on the head: - Slack primary + Telegram chat: the Telegram chat's turn-final was intercepted into draft(final=true) — no real Telegram history message; - Telegram primary + Slack chat: the Slack chat was denied native streaming entirely. Resolve both through _descriptor_for_chat — the same per-chat machinery max_message_length already uses (added for the identical class of bug: the primary's 39000-char cap over-sending into Discord 400s): - new stream_is_message_for_chat(chat_id) on the adapter; arming and NotImplementedError gating use it. The class attribute remains as the single-platform value and legacy-probe fallback. - supports_draft_streaming() gains an optional chat_id kwarg (base signature updated; single-platform adapters ignore it). The consumer passes chat_id with a TypeError fallback for out-of-tree adapters. - the consumer's four draft_stream_is_message reads collapse into one _stream_is_message() helper that prefers the per-chat probe (class-resolved, MagicMock-safe) over the attribute. Platform-name inference ("slack") stays deliberate: a descriptor-level semantic field is the right eventual contract but is a cross-repo wire change — noted for the gg follow-up so future platforms advertise the semantic explicitly. Regression: tests/gateway/relay/test_relay_multiplatform_semantics.py (5 tests: both starvation directions, scalar fallback, per-chat capability gate). * fix(gateway): split delivery + authoritative footer reconciles by suffix, not full resend (review r2, finding 3) The _FINAL_TEXT adoption guard refuses wholesale adoption on split turns — correct (NousResearch#78541: sealed heads would repeat inside the tail) but it was absolute: a post-split verifier footer never entered the ledger, delivered_final_matches() reported a mismatch, and the gateway resent the ENTIRE body+footer after the split chunks (the #11 duplicate class, one level up). When the authoritative final strictly prefix-extends the split ledger, the missing suffix is the only undelivered content: append it to the live tail and the ledger, so the finalize carries it and the recorded payload reconciles. Non-prefix rewrites keep the full-resend fallback — a rewrite cannot be patched onto sealed heads. Regression: tests/gateway/test_split_final_suffix_reconcile.py (3 tests: suffix rides the tail + reconciles, rewrite still mismatches, unsplit adoption unchanged). * fix(relay): cancellation mid-seal restores open state so abandon can close the stream (review r2, finding 4) _seal_open_draft pops the open entry and writes the local tombstone BEFORE awaiting transport I/O — correct ordering for the straggler race, but CancelledError is not an Exception: a cancel during the await bypassed all failure handling, leaving the remote stream live (visible streaming indicator until connector eviction) while the local state said 'nothing open'. The consumer's abandon pass — added for exactly this turn-death case — found nothing to close and no-oped. On CancelledError: restore the open entry, drop the premature tombstone (only if it is still ours), re-raise. The abandon path then seals the stream in place with the on-screen text. Regression: tests/gateway/relay/test_relay_seal_cancellation.py (2 tests: state restoration, and end-to-end cancel→abandon→remote seal). * fix(relay): thread anchors are placement, not turn identity — revive the placement-only fallback (review r2, finding 5) _match_open_draft's single-open-stream fallback was dead for its primary intended callers: metadata carrying thread_ts/thread_id (placement-only resolver lanes) was classified as having 'turn identity', so those sends never reached the fallback — probed: a plain final posted beside the still-open turn-keyed stream. Only per-turn MESSAGE ids are identity now. Thread-anchored and bare callers share the fallback: absorb into the chat's open stream when EXACTLY one is open; stay a plain send when several are (duplicate is recoverable, wrong-stream seal is not). Callers WITH a message id whose key misses never fall back — their identity is authoritative and a miss means the stream belongs to a different turn. Regression: 4 new tests in test_relay_turn_keying.py (thread-anchored seal, both ambiguous-stay-plain shapes, id-mismatch never steals). * fix(relay): random process nonce for draft-id seeding (review r2, follow-up 6) The epoch-millisecond seed (round-1 B3 fix) mitigates the restart-replay class but is not a uniqueness guarantee: two gateways starting in the same millisecond, a forked process inheriting the class state, or a clock step backwards can all mint colliding wire identities against the connector's per-(channel, draft_id) tombstone store. Seed from secrets.randbits(49) instead: collision probability negligible, no clock dependence, and ids + realistic per-process turn counts stay comfortably inside the connector's JS number range (draft_id?: number, 2^53). Regression test now spawns two real interpreters and asserts their seeds differ — the exact scale-to-zero restart shape, and both start within the same second so a clock-locked seed would fail it. * fix(relay): stamp per-turn Slack egress identity — cache is fallback only (R3-5) The connector (gateway-gateway#210) fills chat.startStream's recipient_user_id / recipient_team_id — required by Slack when streaming to a channel — from metadata.user_id / metadata.scope_id. The gateway stamped only slack_team_id per-turn and left user_id (and scope_id) to RelayAdapter._with_scope, whose per-chat caches are keyed on chat_id alone and overwritten by every inbound message: with users U1 and U2 running overlapping turns in one channel, U2's arrival overwrote the cache before U1's stream opened, and U1's stream carried U2 as recipient_user_id. _thread_metadata_for_source now stamps scope_id and user_id from the turn's OWN source (setdefault — explicit values win), so identity is turn-scoped data on the wire. _with_scope is unchanged and fill-only: the caches keep serving restart/synthetic sends that carry no per-turn identity, which is all they were ever safe for. Mutation evidence: reverting the run.py hunk sends test_thread_metadata_stamps_per_turn_user_and_scope and test_concurrent_turns_carry_their_own_identity red; restore returns green. The _with_scope fill-only tests pass on both trees (existing correct behavior, now pinned against regression). --------- Co-authored-by: Ben Barclay <ben@nousresearch.com>
jarvisxyz
pushed a commit
that referenced
this pull request
Sep 2, 2026
…ared registry A gateway process opened state.db from ~12 call sites, each minting its own writer connection, self._lock, close-time WAL checkpoint, and token-writer thread. With N independent writers on one WAL file, one connection's close-time checkpoint could race another's growth — the lost/reordered-page-write signature across 11+ incidents (NousResearch#90837). Adds hermes_state_registry.py: a process-wide, per-path, refcounted shared registry owning the writer boundary. - acquire(path): same resolved path returns the same instance (one writer connection, one lock, one token-writer thread) for every long-lived in-process caller (gateway runner, SessionStore, per-agent lazy recall, cron per-job, mirror, channel_directory, slash_commands, shutdown_flush, session_search, react_to_message, delegate, mcp_serve, auto_archive, tui_gateway). - close() on a shared instance is a NO-OP — the registry owns the lifecycle, so one caller's close can never tear down a writer other callers still hold. - Generation-aware retirement on inode change: a replaced state.db RETIRES the live generation (never lent again) but keeps it alive for existing holders; release is object-keyed so holders of the old generation drain it independently of the new one. The old generation's own write path still fails with the typed StateDbReplacedError (existing protection, unchanged). - Replacement-open failure leaves NO registry entry for the path — the next acquire retries fresh, never hands out a closed stale object. - All teardown runs OUTSIDE the registry lock: a final release's WAL checkpoint can never stall acquisition for every state.db. - close_shared_session_dbs() at gateway shutdown drains every generation (live + retired) as the final safety net. CLI one-shots, recovery flows, and read-only cross-profile opens keep using SessionDB() directly with their own close() — only long-lived in-process sites route through the registry. References NousResearch#90837 (root-cause tracker stays open: the #10 EOF signature and the WAL-lifecycle A/B verdict remain under investigation there).
This branch has not been deployed
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Native Slack AI Assistant Steps API Streaming
Replaces the edit-message polling approach with native Slack Assistant streaming using
chat.startStream/appendStream/stopStreamand typedTaskUpdateChunkfrom the Slack SDK.Architecture
When the gateway receives a Slack message in a thread, it detects the platform and thread context, creates a
SlackStreamConsumerinstead of the default edit-based consumer, and streams tokens through the Assistant Steps API:startStream— opens a plan-mode stream (task_display_mode="plan") with an initial "Processing" stepin_progress→completestep card withTaskUpdateChunkTaskUpdateChunk.outputstopStream— closes the stream; Slack renders the final UIUX Fixes Included
Phantom message eliminated — An initial "Processing" step is emitted immediately after
startStream, replacing Slack's built-in "Gathering information…" placeholder before it becomes visible. The step auto-completes when the first real content arrives.Human-readable step titles —
_format_tool_title()+_TOOL_DISPLAY_NAMESmapping converts raw snake_case tool names into friendly titles with context from the tool input:search_files→ "Search files: *.py"terminal→ "Terminal: ls -la"read_file→ "Read file: src/main.py"Fixed "Done" concatenation bug — Completed step descriptions preserve the in-progress details and separate them with a newline:
command: ls -la\nDone (0.2s)instead of the previouscommand: ls -laDone (0.2s).Key Technical Detail: Plan Mode
Slack enforces mode isolation — a stream started with
task_display_mode="plan"can only acceptchunks, nevermarkdown_text. Mixing the two returnsstreaming_mode_mismatch. All LLM text output is routed throughTaskUpdateChunk.outputon a dedicated "Response" step.Slack App Requirements
assistant:write,chat:write,channels:history,groups:history,im:historyassistant_thread_started,assistant_thread_context_changedFiles Changed
gateway/platforms/slack_stream.pySlackStreamConsumerclass with phantom fix, friendly titles, description preservationgateway/platforms/slack.pycreate_stream_consumer(),set_thread_title(),set_suggested_prompts()gateway/run.pytests/gateway/test_slack_stream.pyscripts/release.pyTest Results