Skip to content

fix: show prompts a server ends with IAC GA - #38

Merged
HarryCordewener merged 3 commits into
mainfrom
fix/ga-terminated-prompts
Aug 20, 2026
Merged

fix: show prompts a server ends with IAC GA#38
HarryCordewener merged 3 commits into
mainfrom
fix/ga-terminated-prompts

Conversation

@HarryCordewener

Copy link
Copy Markdown
Member

What

A character-creation prompt on tdome.nukefire.org:4000 never appeared — the session read as a server that had stopped answering. The prompt had arrived and was sitting in TelnetSession._pending with nothing to flush it.

OnPromptAsync is the only thing that flushes that buffer, and it only ever fired for IAC EOR. NukeFire is a default NVT: it negotiates neither EOR nor SUPPRESS-GO-AHEAD and ends every prompt with IAC GA, which is what RFC 854 requires of it —

when a process at one end of a TELNET connection cannot proceed without input from the other end, the process must transmit the TELNET Go Ahead (GA) command

TelnetNegotiationCore had no inbound handling for GA at 2.8.1 (it logged Critical and recovered through Trigger.Error on every prompt), and 2.10.0 accepted the byte but discarded it. Fixed upstream in PR #90 rather than worked around here, per the standing rule for this dependency.

Changes

  • Bump TelnetNegotiationCore 2.8.1 → 2.11.0. No client-side code change is needed: AddDefaultMUDProtocols(onPrompt: OnPromptAsync) already wires one callback to both plugins, so the signal arrives with the version. The bump also crosses 2.9.0 (IsNegotiated), 2.9.1 (StartNegotiation transitions) and 2.10.0 (raw MSSP bytes), all additive.
  • Three tests, one per RFC rule — only the EOR half was ever pinned, which is how this reached a release:
    • IAC GA is a prompt when nothing suppressed it (RFC 854)
    • IAC GA is a NOP once SUPPRESS-GO-AHEAD is negotiated (RFC 858)
    • IAC EOR is a NOP while the option is not in effect (RFC 885)
  • CLAUDE.md.Repository state claimed TelnetNegotiationCore 2.7.0 while the pin said 2.8.1; no paragraph names a version now — Directory.Packages.props is the single source of truth. Adds release notes for 2.9.0–2.11.0 and records the prompt-marker rules, including the part that stays ours: a server sending no boundary marker at all still leaves a prompt in _pending, because nothing here flushes on idle.

Verification

All five suites green against 2.11.0:

Core 941 failed: 0
Tui 1818 failed: 0
Graphics 83 failed: 0
Scripting 42 failed: 0
Web 37 failed: 0

The two library-dependent tests were confirmed to fail against the previously pinned 2.8.1 before the bump — they are not passing vacuously.

Not in scope

The prompt reads at right, Pemberton (<send>Y</send>/<send>N</send>)?. The ESC[1z/ESC[7z MXP line tags around it are discarded cleanly by AnsiParser, but ContentFormat defaults to Ansi, so the <send> tags themselves will now be visible as literal text. NukeFire never negotiated MXP — it sends the markup unsolicited. Deliberately left for separate work.

🤖 Generated with Claude Code

…t markers
The Repository state section said TelnetNegotiationCore 2.7.0 while the pin in
Directory.Packages.props said 2.8.1. A version in prose goes stale silently, so
neither paragraph names one now: the pin is the single source of truth and the
dependency notes say which release changed what.
Records what the GA investigation established, since the same gap will be read
for again: TelnetSession.OnPromptAsync is the only thing that flushes _pending,
a default NVT ends its prompts with IAC GA and nothing else (RFC 854), RFC 858
is the only thing that makes a GA meaningless, and an unnegotiated IAC EOR is a
NOP (RFC 885). Also notes the part that stays ours — a server sending no
boundary marker at all still leaves a prompt in the buffer.
Only the EOR half of the prompt boundary was ever pinned, which is how a login
screen that never appeared reached a release: a default NVT ends its prompts
with IAC GA and nothing else, nothing flushed _pending for it, and no test
noticed.
Three tests, matching the three RFC rules:
- IAC GA is a prompt when nothing suppressed it (RFC 854)
- IAC GA is a NOP once SUPPRESS-GO-AHEAD is negotiated (RFC 858)
- IAC EOR is a NOP while the option is not in effect (RFC 885)
The negative pair assert an absence, which WaitAsync cannot express, so they
wait for a later event they do expect and then read OutputWaiter.Snapshot() —
deterministic rather than a sleep. The SGA test feeds negotiation and GA in one
call: the interpreter reads bytes in order off one channel and awaits each
transition, so the negotiation has completed by the time the GA is processed.
Two of the three fail against the currently pinned 2.8.1, which is the point;
they pass on the bump in the next commit.
Takes the prompt-marker fix (upstream PR #90): IAC GA is now reported as the
prompt boundary RFC 854 makes it, so a server that negotiates neither EOR nor
SUPPRESS-GO-AHEAD — most MU* servers — no longer leaves its prompt sitting in
_pending with nothing to flush it.
No client-side change is needed for it. AddDefaultMUDProtocols(onPrompt:) already
wires one callback to both plugins, so the signal arrives with the version.
Also crosses 2.9.0 (IsNegotiated beside the always-true IsEnabled), 2.9.1 (IAC GA
and a bare IAC mid-negotiation get permitted transitions instead of hitting
OnUnhandledTriggerAsync, which logged Critical on every prompt) and 2.10.0 (raw
bytes beside each decoded MSSP value). All additive; nothing here had to move.
941 / 1818 / 83 / 42 / 37 across the five suites, all green.
@coderabbitai

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in:27 minutes

Limit details: You’ve used the included review currently available. Your 68 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4269fdec-3535-49c0-bbe0-c8bc853d68c7

📥 Commits

Reviewing files that changed from the base of the PR and between 77e9197 and 9f097ff.

📒 Files selected for processing (3)
  • CLAUDE.md
  • Directory.Packages.props
  • tests/SharpMUTerm.Core.Tests/Telnet/TelnetSessionTests.cs

Comment @coderabbitai help to get the list of available commands.

@HarryCordewener
HarryCordewener merged commit 4c2a331 into mainAug 20, 2026
3 checks passed
@HarryCordewener
HarryCordewener deleted the fix/ga-terminated-prompts branch August 20, 2026 19:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@HarryCordewener