Skip to content

feat(gateway): add per-platform terminal_progress knob to keep terminal commands compact in chat - #42638

Closed
GodsBoy wants to merge 3 commits into
NousResearch:mainfrom
GodsBoy:feat/gateway-terminal-progress-knob
Closed

feat(gateway): add per-platform terminal_progress knob to keep terminal commands compact in chat#42638
GodsBoy wants to merge 3 commits into
NousResearch:mainfrom
GodsBoy:feat/gateway-terminal-progress-knob

Conversation

@GodsBoy

@GodsBoy GodsBoy commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

On markdown-capable messaging platforms (Telegram, WhatsApp, Slack, Feishu, Matrix, Weixin), gateway tool progress for the terminal tool renders the full, untruncated shell command as a bare fenced code block. There is currently no way to keep it compact without turning off all tool progress, which also removes useful breadcrumbs like search_files, read_file, patch, and todo.

This is a regression:

Rather than revert #42576 again (which would just restart the back and forth), this PR makes the behavior configurable per platform. A new display.terminal_progress setting takes compact or code_block:

The setting is resolved through the existing per-platform display resolver, so a deployment can set it globally (display.terminal_progress) or per platform (display.platforms.telegram.terminal_progress). Only the terminal tool is affected.

Related Issue

Fixes #42637

Type of Change

  • ✨ New feature (non-breaking change that adds functionality)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • gateway/display_config.py: add terminal_progress to _GLOBAL_DEFAULTS (default code_block) and a _normalise branch that lower-cases the value and falls back to code_block for anything outside {compact, code_block}. OVERRIDEABLE_KEYS is derived from _GLOBAL_DEFAULTS, so the key becomes per-platform overrideable automatically.
  • gateway/run.py: resolve terminal_progress once per source in the existing display-settings setup block, then gate the fenced _code_block build on terminal_progress == "code_block". In compact, the callback falls through to the existing truncated preview in all/new modes, and the verbose branch also renders the truncated preview (with a 40-char cap) so the full command is not posted in verbose mode either.
  • cli-config.yaml.example: document the new terminal_progress setting with a per-platform override example.
  • tests/gateway/test_display_config.py: add TestTerminalProgress covering default, global, per-platform override precedence, case-insensitivity, unknown and non-string scalar normalisation, and OVERRIDEABLE_KEYS membership.
  • tests/gateway/test_run_progress_topics.py: add run-level regression tests that drive the real {"command": ...} path (the existing truncation tests pass {} args and never reach the code-block branch). They cover compact in all and verbose modes (no fence, truncated preview present, command tail absent) and an explicit code_block run that still fences. The existing test_terminal_progress_renders_fenced_code_block is left untouched as the default-preserved guard.

How to Test

  1. Configure a markdown-capable platform (for example Telegram) with tool progress on: display.platforms.telegram.tool_progress: all.
  2. Run a terminal command. With no terminal_progress set (default), the full command renders as a fenced code block, same as today.
  3. Set display.platforms.telegram.terminal_progress: compact and run a terminal command again. Now the progress shows the short truncated terminal: "..." preview, and other breadcrumbs (search_files, read_file, patch, todo) are unchanged.
  4. Run the tests:
pytest tests/gateway/test_display_config.py tests/gateway/test_run_progress_topics.py -q

All 74 tests pass. The new compact tests were verified to fail when the gate is removed, so they genuinely guard the leak path (including the verbose path).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q for the affected modules and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Ubuntu 24.04

Documentation & Housekeeping

  • I've updated relevant documentation (docstrings, comments) where applicable
  • I've updated cli-config.yaml.example because I added a config key
  • N/A architecture/workflow docs (no architecture change)
  • I've considered cross-platform impact (pure Python config resolution, no platform-specific code paths)
  • N/A tool descriptions/schemas (no tool behavior change)

Screenshots / Logs

Before this PR, a terminal progress bubble on Telegram renders the full multi-line command as a fenced code block, including the working directory:

Terminal command leaked as a fenced code block in Telegram

With display.platforms.telegram.terminal_progress: compact, the same command renders as the short terminal: "..." preview instead.

GodsBoy added 2 commits June 9, 2026 08:06
…de_block)

Terminal-command tool progress on markdown-capable platforms (Telegram,
WhatsApp, Slack, Feishu, Matrix, Weixin) currently renders the full shell
command as a bare fenced code block. There is no way to keep it compact
without turning off all tool progress.

Add a display.terminal_progress setting with values compact and code_block
(default code_block), resolved through the existing per-platform display
resolver. When set to compact, terminal progress falls back to the short
truncated `terminal: "..."` preview that every other tool already uses, so
the full command is not posted to the chat. Only the terminal tool is
affected; other breadcrumbs (search_files, read_file, patch, todo) are
unchanged, and the default preserves current behavior.

Regression tests drive the real {"command": ...} path so they exercise the
code-block branch (the existing truncation tests pass {} args and never did).
…act knob

Code review found that terminal_progress=compact only suppressed the fenced
block in all/new modes. In verbose mode the full command was still serialized
through the args branch (tool_preview_length defaults to 0 in verbose), so the
command could still reach the chat. Render the truncated terminal preview with
a 40-char cap in verbose mode too when the knob is compact, matching the
all/new path.

Strengthen the verbose compact test to assert the truncated preview is present
and the command tail is absent (it previously passed only by incidental
truncation), harden the all/new compact assertion against partial-token leaks,
and add a normalise edge test for non-string scalar values.
@liuhao1024

Copy link
Copy Markdown
Contributor

Thanks for this PR — the terminal_progress knob is well-structured:

  • Per-platform override resolution via resolve_display_setting follows the established pattern
  • The compact-mode 40-char cap fallback when tool_preview_length is unset covers the verbose-mode edge case
  • 7 config-unit tests + 3 integration tests with real config resolution (monkeypatched _hermes_home) — good coverage for a display setting
  • _normalise handles non-string scalars, case insensitivity, and unknown-value fallback — all the edge cases
  • OVERRIDEABLE_KEYS membership test ensures the key participates in validation

Clean implementation. 👍

@GodsBoy GodsBoy closed this Jun 9, 2026
@GodsBoy GodsBoy reopened this Jun 9, 2026
@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels Jun 9, 2026
@GodsBoy GodsBoy closed this Jun 9, 2026
@GodsBoy GodsBoy reopened this Jun 9, 2026
@GodsBoy

GodsBoy commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@teknium1 reopening this. The live repro shows the terminal progress leak is still present on a deployed Telegram gateway.

Evidence from the fresh smoke:

  • Platform: Telegram messaging gateway
  • Command path: user asked Gideon to apply Hermes local customisation after /update
  • Actual: the interim terminal progress bubble rendered a fenced shell block containing the full multi-line command
  • Expected: terminal progress should stay compact and truncated, while normal non-terminal tool breadcrumbs can remain visible

The earlier close was based on one live check that appeared compact, but this new screenshot proves the bug still reproduces. Reopening the issue and PR so #42638, or an equivalent upstream fix, can be considered again.

@GodsBoy

GodsBoy commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up on the red test (1) shard: it is not from this PR.

The 8 failures are all in tests/tools/test_terminal_config_env_sync.py with AssertionError: Could not find _config_to_env_sync = {...} literal in source. That is a pre-existing breakage on main: commit f8adefd (fix(tui): apply terminal backend config before launch) refactored hermes_cli/config.py:set_config_value to bridge terminal keys through the new terminal_config_env_var_for_key() helper and removed the old _config_to_env_sync dict, but the drift-guard test still introspects the source for that dict literal. main's own push run is red on the same 8 failures (run 27190977348), so every open PR's merge-with-main ref inherits it.

This PR's own changes are limited to gateway/display_config.py, gateway/run.py, cli-config.yaml.example, and two gateway test files; on its base all 74 tests in the two affected modules pass. The failure only surfaces here because the PR is tested merged with the currently broken main. It clears once the drift guard is pointed at the new terminal_config_env_var_for_key() mechanism. Happy to send that as a separate one-line test fix if useful.

Resolve gateway/run.py conflict with NousResearch#42634, which split the terminal
fenced block into a full (verbose) and a single-line capped (non-verbose)
variant. Keep the terminal_progress knob gate on the build condition so
compact still falls through to the truncated preview in both modes, and
adopt the _code_block_full / _code_block_short variables for rendering.
Update test_terminal_progress_explicit_code_block_still_renders to assert
the capped first-line fence in non-verbose mode rather than the full
command.
@GodsBoy

GodsBoy commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Update: test (1) is green now. The drift-guard breakage I flagged above was fixed upstream by #42695 (76f89d6, track TERMINAL_CONFIG_ENV_MAP after env-sync consolidation), so the earlier note no longer applies.

I also merged latest main into the branch, which resolved a conflict in gateway/run.py with #42634 (that PR split the terminal fenced block into a full variant for verbose and a single-line capped variant for non-verbose). The terminal_progress knob now layers cleanly on top: it gates whether either block is built, so compact falls through to the truncated terminal: "..." preview in both verbose and non-verbose modes, while code_block (default) preserves #42576's fenced output (full in verbose, capped first line in non-verbose). All test shards pass; only the docker build and nix-macos infra checks are still running.

@GodsBoy

GodsBoy commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing this for now after retesting current main without the local patch.

The remaining live issue I was seeing on Telegram is resolved by #42729: terminal progress in non-verbose mode now keeps the fenced code-block presentation but caps it to a compact single-line preview. That covers my deployed gateway configuration, since I am not running verbose tool progress.

The per-platform compact/code_block knob in this PR may still be useful as a preference later, but it is no longer needed for the original regression I was trying to fix.

Thanks for the follow-up fix.

@GodsBoy GodsBoy closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Terminal tool progress posts the full command as a fenced code block to messaging chats (regression after #42576)

3 participants