Skip to content

fix(cli): strip leaked Device Attributes (DA) terminal responses - #27172

Open
Ramadas108 wants to merge 1 commit into
NousResearch:mainfrom
Ramadas108:fix/cli-strip-da-terminal-responses
Open

fix(cli): strip leaked Device Attributes (DA) terminal responses#27172
Ramadas108 wants to merge 1 commit into
NousResearch:mainfrom
Ramadas108:fix/cli-strip-da-terminal-responses

Conversation

@Ramadas108

@Ramadas108 Ramadas108 commented May 16, 2026

Copy link
Copy Markdown

Problem

Terminal startup noise — visible "1c/2424" artifacts leaking into the CLI
input buffer. The "1c" fragment is a Device Attributes (DA) response
(ESC[?1;2c) from the terminal that the input sanitizer was not stripping.

Root Cause

The _strip_leaked_terminal_responses_with_meta() sanitizer in cli.py
(added for issue #14692) handled Cursor Position Report (CPR) responses
and SGR mouse reports, but was missing Device Attributes (DA) response
stripping entirely. DA responses leak through the same mechanism —
prompt_toolkit terminal-queries racing with the input parser under resize
storms or multiplexer tab switches.

Changes

File Change
cli.py:2124 Added _DA_ESC_RE — matches ESC[?c (primary DA), ESC[>c (secondary DA), and bare ESC[c
cli.py:2125 Added _DA_VISIBLE_RE — caret-escape visible form ^[[?c
cli.py:2229 Wired _DA_ESC_RE.sub() into the ESC sanitization branch
cli.py:2235 Wired _DA_VISIBLE_RE.sub() into the visible sanitization branch
tests/cli/test_cli_terminal_response_sanitizer.py 7 new regression tests

Testing

All 22 tests pass.

Related

Completes the coverage gap from issue #14692.

…sResearch#14692)

When prompt_toolkit's resize handler or terminal-query logic races with
the input parser under resize storms or multiplexer tab switches, the
terminal's Device Attributes response (ESC[?<params>c) can land in the
input buffer as literal text — visible as the '1c' fragment in terminal
startup noise like '1c/2424'.

The existing _strip_leaked_terminal_responses_with_meta() sanitizer
(added for issue NousResearch#14692) already handled Cursor Position Report (CPR)
responses (ESC[<row>;<col>R) and SGR mouse reports (ESC[<...M/m) but
was missing DA response stripping entirely.

This commit adds:

- _DA_ESC_RE — matches ESC[?<params>c (primary DA), ESC[><params>c
  (secondary DA), and bare ESC[c sequences at the byte level.
- _DA_VISIBLE_RE — matches the caret-escape visible form ^[[?<params>c
  that appears when the ESC byte was stripped by a prior filter.
- Both regexes wired into the ESC and visible branches of the sanitizer.

7 regression tests covering primary DA, secondary DA, bare DA, visible
form, combined DA+CPR (the exact '1c/2424' artifact), and a negative
test ensuring Cursor Forward (ESC[2C) is not confused with DA.

Closes issue NousResearch#14692 (completing the coverage gap).
@cardtest15-coder

This comment was marked as spam.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 16, 2026
@teknium1

teknium1 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused sanitizer extension. I found no blocking issue in the proposed approach.

Current main only strips CPR/DSR and SGR mouse response patterns (cli.py:3112-3120, cli.py:3298-3310), while all relevant classic CLI input paths pass through this sanitizer (cli.py:14145, cli.py:14305, cli.py:15201). PR commit 62dbc50cc1964bdeb5d32c4f4f8c9b6b332e2f7d extends both ESC and visible-form branches and adds targeted regression coverage without adding new configuration or surface area.

Automated hermes-sweeper review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants