An away bar where the reader left off, and a login the client stopped breaking by HarryCordewener · Pull Request #21 · SharpMUSH/SharpMUTerm

HarryCordewener · 2026-08-10T21:02:46Z

Two independent pieces of work, kept as separate merge commits so either can be read or reverted on its own.

The away divider (`feat/away-divider`)

Coming back to the terminal now leaves a bar where you were — AwayBarRenderer + TerminalFocusWatcher, driven by real terminal focus reporting (CSI ?1004h). Third of the boundary bars, and it earns its row the same way FreezeBarRenderer and RestoreBarRenderer do: mark the boundary, never restyle the content. Both halves of getting the signal are workarounds against SharpConsoleUI, which is why they live in one file — no released version asks for focus, IConsoleDriver has no focus event, and focus-in arrives disguised as a Tab keypress. Details are in the file's own documentation and in CLAUDE.md.

Includes 0e2fad7, which scrolls the pane to the bar instead of leaving it above the fold.

The unsolicited telnet option (`fix/unsolicited-telnet-option`)

TelnetSession wrote IAC DO MSSP straight to the transport the moment it connected, so that a server which supports MSSP but never volunteers it would answer. Legal telnet — RFC 854 has either party initiating, and requires a response even to a refusal — and it cost the auto-login.

Refusing an option means consuming its three bytes. A server that implements neither leaves them in its line buffer, where they are prepended to the next line the client sends — and that line is always the login. The server reads \xFF\xFD\x46connect Name password, doesn't recognise it, redisplays its connect screen and logs nobody in. The transcript shows the welcome screen twice with no reason for it, because the login line is deliberately never echoed or logged. Only the first line after the request dies, which is why typing the login by hand always worked and the auto-login never did.

Measured on a live game rather than reasoned about:

bytes sent	result
`WHO\r\n`	WHO list, one connect screen
`IAC DO MSSP` + `WHO\r\n`	two connect screens, no WHO list
`IAC DO MSSP` + `WHO\r\n` + `WHO\r\n`	screen, screen, then the WHO list — only the first line dies
`IAC DO MSSP` + connect ×2	screen, then `Invalid credentials.` — the first was never evaluated
`IAC WILL NAWS` + connect	one screen + `Invalid credentials.` — the login survives

IAC DO GMCP and IAC WILL/DONT MSSP reproduce it on that server; NAWS and TTYPE, options it implements, do not — so it is every unknown option, an RFC 854 state-machine bug on their side. We are not in a position to know which servers parse telnet properly, and the one that does not is exactly the one whose login we break.

The mechanism is deleted rather than left empty — RequestOptions, MsspOption and RequestOptionsAsync all go — so there is no seam to reach for. Nothing is lost upstream: TelnetNegotiationCore would never have sent that DO (its client-side MSSP answers a server's WILL and initiates nothing), and the bytes were written around the library only to keep IAC from being escaped as data. Negotiation is the library's to conduct.

MSSP now arrives the way its own specification writes the handshake — the server offers IAC WILL MSSP, the library answers DO — and servers that never offer are what the INFO screen's dialled, publishes none state is for.

Tests

UnsolicitedNegotiationTests pins both halves, and the second dials a real loopback socket. An injected sessionFactory replaces the exact arm that carried the bug — WorldSession.DefaultSessionFactory — so a test built that way would have agreed with the code while every real connection carried the bytes. That is why nothing here caught it.

Verification

Build warning-free on the merged tree; all five suites green — Core 845, Graphics 83, Scripting 42, Web 37, Tui 1491. The away and away-scrollback snapshot views still render. Each branch also builds and tests green on main alone.

🤖 Generated with Claude Code

https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

Summary by CodeRabbit

New Features
- Added an AWAY divider showing where terminal reading stopped, missed-line count, and elapsed time.
- Added automatic focus-away and return handling, including marker visibility and scrollback behavior.
- Added away and away-scrollback snapshot views.
Bug Fixes
- Telnet connections no longer send unsolicited option requests; options are negotiated only when offered by the server.
Documentation
- Documented focus handling, away-divider behavior, and server-initiated MSSP negotiation.

A boundary row drawn where the reader left off when they tab away from the terminal, so returning does not mean guessing which lines are already read. The signal is terminal focus reporting (CSI ?1004h), which SharpConsoleUI neither asks for nor decodes. Both halves of the workaround are recorded: WriteClipboardOsc52 is a raw-escape writer wearing its first customer's name, and focus-in reaches us disguised as a Tab keypress because DispatchCsi reads a trailing I as Tab. Focus-out is discarded upstream and is not recoverable, so the boundary anchors to the last input event instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

AwayBarRenderer is a sibling of the freeze and restore bars and carries the same two figures the restore bar does, for the same reason: a returning reader asks both how much is in front of them and how far behind they are. TerminalFocusWatcher is the whole workaround, in one file so it is one file to delete. It asks for focus reporting through WriteClipboardOsc52 — named for its first customer, but a verbatim raw write under the renderer's own lock — and recognises focus-in in the Tab keypress the framework's parser mistranslates it into. Telling that Tab from a real one is a question about time, and the comparison has to be against the input before it: the disguised focus-in is itself a KeyPressed, raised before the global shortcuts run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

Tabbing away and coming back meant guessing which lines had already been read: every pane bottom-anchors through an absence, so what you land on is the newest output with no boundary in it, and the unread badges are silent about it — NoteActivity only counts a line while a window is *not* caught up, and the window you were looking at stays visible and at its tail the whole time. The boundary is tracked forward, on every input event, because it cannot be found afterwards: a PaneLine's stamp is formatted text, not a time. It is kept one input back, for the reason the watcher's clock is — the focus report is itself a keypress and has already moved the newer of the two by the time anything recognises it as a return. Consumption is not Workspace.IsCaughtUp. A bottom-anchored pane satisfies that the instant you return with two hundred unread lines above the fold. The bar goes when it has been inside the viewport, the pane is at its live tail, and one input has landed since it was drawn. Also fixes SimulateKey discarding a global shortcut's result: it swallowed the key whether or not the handler claimed it, which was invisible while every claim returned true and is not any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

… the fold Reported as "I saw no change". The mechanism was working — proven against a real kitty: the client emits ?1004h, a DECRQM probe answers ?1004;1 after the alternate-screen switch and every mode set behind it, the terminal writes ESC [ O and ESC [ I, and a 42-second absence was recognised. What was missing was any way to tell. Come back to more lines than the pane holds and the bar is drawn far above the viewport, so nothing on screen changes. Nothing else covers for it either: a window that is visible and at its live tail throughout an absence accrues no unread badge, because NoteActivity counts only what arrives while not caught up. The reader got no signal at all, which is indistinguishable from the terminal not reporting focus. RevealAwayBar scrolls each pane that gained a bar so the bar is at the top of its viewport. A bar already in view is left alone — scrolling a shallow absence would take a pane off its tail to reveal what is already on it. The first cut of that scroll was itself wrong, and the frame is what caught it: a buffer index is not a viewport row, a wrapped line occupies several, and in a narrow pane the scroll landed hundreds of rows adrift in content from a previous session. The tail height is measured through the framework's own MeasureDOM, so it wraps the way the real control will, and only the tail is measured. Consumption drops to two conjuncts. "At the live tail" now means something on its own, because the reveal has taken the pane off its tail whenever the bar was not on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

TelnetSession wrote IAC DO MSSP straight to the transport the moment it connected, so that a server which supports MSSP but never volunteers it would answer. It is legal telnet - RFC 854 has either party initiating, and requires a response even to a refusal - and it cost the auto-login. Refusing an option means consuming its three bytes. A server that implements neither leaves them in its line buffer, where they are prepended to the next line the client sends, and that line is always the login. The server reads \xFF\xFD\x46connect Name password, does not recognise it, redisplays its connect screen and logs nobody in - while the transcript shows the welcome screen twice with no reason for it, because the login line is deliberately never echoed or logged. Only the first line after the request dies, which is why typing the login by hand always worked and the auto-login never did. Measured on a live game rather than reasoned about. With the request the login line was never evaluated; without it the same line reached the game and was answered. IAC DO GMCP and IAC WILL/DONT MSSP reproduce it there; NAWS and TTYPE, options that server implements, do not. The mechanism is deleted rather than left empty - RequestOptions, MsspOption and RequestOptionsAsync all go - so there is no seam to reach for. Nothing is lost upstream: TelnetNegotiationCore would never have sent that DO, because its client-side MSSP answers a server's WILL and initiates nothing, and the bytes were written around the library only to keep IAC from being escaped as data. Negotiation is the library's to conduct. UnsolicitedNegotiationTests pins both halves, and the second one dials a real loopback socket. An injected sessionFactory replaces the exact arm that carried the bug - WorldSession.DefaultSessionFactory - so a test built that way would have agreed with the code while every real connection carried the bytes, which is why nothing here caught it. MSSP now arrives the way its own specification writes the handshake: the server offers IAC WILL MSSP and the library answers DO. Servers that never offer are what the INFO screen's "publishes none" state is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

…e terminal

…ble an option it has not offered

coderabbitai · 2026-08-10T21:03:22Z

Walkthrough

The TUI now detects focus returns, marks output received while away, reveals and consumes markers, and supports related snapshots. Telnet setup no longer sends client-initiated MSSP requests. Documentation and regression tests cover both changes.

Changes

Away divider

Layer / File(s)	Summary
Away marker rendering `src/SharpMUTerm.Tui/AwayBarRenderer.cs`, `src/SharpMUTerm.Tui/Glyphs.cs`, `tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs`	Adds the `Away` glyph and localized `AWAY` bar rendering with line-count, duration, styling, and validation behavior.
Terminal focus return detection `src/SharpMUTerm.Tui/TerminalFocusWatcher.cs`, `docs/superpowers/specs/...`, `tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs`	Adds supported-driver focus reporting, delayed bare Tab detection, input events, lifecycle handling, and watcher tests.
Away marker lifecycle and integration `src/SharpMUTerm.Tui/SharpMUTermApp.cs`, `tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs`, `CLAUDE.md`	Tracks per-window boundaries, inserts and removes markers, reveals deep markers, adjusts for trimming, preserves normal Tab handling, and adds snapshot scenarios and integration tests.

Server-led MSSP negotiation

Layer / File(s)	Summary
Server-led MSSP connection setup `src/SharpMUTerm.Core/Session/WorldSession.cs`, `src/SharpMUTerm.Core/Telnet/TelnetSession.cs`, `tests/SharpMUTerm.Core.Tests/Telnet/*`, `CLAUDE.md`	Removes client-initiated MSSP requests. Connection setup now relies on server offers. Loopback and parsing tests verify negotiation and login ordering.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
participant TerminalFocusWatcher
participant SharpMUTermApp
participant ScrollbackBuffer
participant AwayBarRenderer
TerminalFocusWatcher->>SharpMUTermApp: Returned event
SharpMUTermApp->>ScrollbackBuffer: Read output boundary and line count
SharpMUTermApp->>AwayBarRenderer: Render away marker
AwayBarRenderer-->>SharpMUTermApp: AWAY bar markup
SharpMUTermApp->>ScrollbackBuffer: Insert marker and reveal viewport

Possibly related PRs

SharpMUSH/SharpMUTerm#8: The away-divider logic uses scrollback line counts, trimming, and positioning.
SharpMUSH/SharpMUTerm#13: Both changes modify MSSP negotiation and TelnetSession behavior.
SharpMUSH/SharpMUTerm#20: Covers the same away-divider and focus-watcher implementation.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies both main changes: the away bar and the fix for login corruption caused by unsolicited negotiation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

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

coderabbitai

Actionable comments posted: 13

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 264-270: Update the stale long-output view description near the
existing “three scroll* views” statement to explicitly name all deep-scrollback
views, including away-scrollback and freeze-scrollback, rather than relying on
the scroll* prefix or outdated count.
- Around line 115-119: Rewrite the sentence around “focus-in” and
“AnsiInputParser.DispatchCsi” so it is grammatically complete: add the missing
relative connector and place the parenthetical explanation after a clear
introductory clause. Preserve the existing technical meaning about focus-in
recognition, bare Tab translation, and why Tab is claimed through
RegisterGlobalShortcut rather than MacroKeys.AppShortcuts.
In `@docs/superpowers/specs/2026-08-10-away-divider-design.md`:
- Around line 8-10: Update the prose around “Every pane has bottom-anchored” to
use grammatical wording that states each pane stayed bottom-anchored while the
reader was away, without changing the surrounding meaning.
- Around line 3-4: Update the status line in the design document to indicate
that the design has been implemented, removing the “proposed — design only,
nothing implemented” wording while preserving the existing date.
In `@src/SharpMUTerm.Core/Telnet/TelnetSession.cs`:
- Around line 390-391: Update the comment near BuildInterpreterAsync to remove
the inaccurate claim that every enabled option was first offered by the server.
Document only the intended invariant that ConnectAsync does not initiate MSSP
negotiation or send a direct IAC DO MSSP, while preserving the existing
behavior.
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs`:
- Around line 532-542: The TerminalFocusWatcher event handlers must marshal
driver-thread callbacks onto the UI thread before mutating application state. In
src/SharpMUTerm.Tui/SharpMUTermApp.cs lines 532-542, wrap both _focus.Input and
_focus.Returned subscriptions with OnUiThread. In
src/SharpMUTerm.Tui/TerminalFocusWatcher.cs lines 209-213, update the XML
documentation to state that Input may be raised on the driver's mouse thread,
while preserving the existing Returned UI-thread documentation.
- Around line 2456-2478: Ensure the no-new-lines path after RemoveAwayBar
repaints the affected pane before continuing. In the missed <= 0 branch of the
surrounding away-bar update flow, invoke RepaintPane(windowId) after the removal
so the control reflects the buffer and removed _awayMarks entry.
In `@tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs`:
- Around line 156-158: Replace the raw frame substring assertions in both tests
with the shared FrameGrid.Decode and Visible helpers, asserting against the
decoded painted cells so cursor-addressed SGR output is interpreted correctly.
Preserve the existing AwayBarRenderer.Label expectation while using the suite’s
established FrameGrid inspection pattern.
- Around line 286-296: Strengthen both Tab tests in
tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs:286-296 and
tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs:298-311 by asserting that
SimulateKey(Key(ConsoleKey.Tab, '\t')) declines the chord, or that the command
line receives the Tab, in addition to the existing AwayBarIndex(Main) null
assertion. Ensure ATabFromAReaderWhoIsSittingThereIsATab and
AnAppWithNoFocusReportingClaimsNoTabAtAll cannot pass when the watcher silently
consumes the Tab.
- Around line 219-230: Strengthen TheBarIsNotCountedAsUnread by asserting that
SimulateReturnFromAway renders a bar, using the existing bar-detection assertion
or helper. Capture the unread count after the bar is confirmed, then assert the
return-from-away operation leaves that recorded value unchanged.
- Around line 313-342: Update the test setup around Bound to provide teardown
that disposes SharpMUTermApp and its HeadlessConsoleDriver, and restores the
original process-wide Console.In after each test. Preserve the existing returned
app/session/time usage while ensuring cleanup runs even when the test fails.
In `@tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs`:
- Around line 188-210: Add a test covering Tab handling after the focus watcher
has been stopped or disposed, asserting that TryTakeAsReturn declines the key
and does not raise Returned. Update TryTakeAsReturn to require both _enabled and
_started before consuming Tab, preserving normal behavior while the watcher is
active.
- Around line 99-109: Update StartAsksTheTerminalToReportFocusAndStopTurnsItOff
to use ordered collection assertions, such as CollectionOrdering.Matching or
positional checks, for driver.Written. Preserve the expected enable-then-disable
sequence so reversed terminal writes cause the test to fail.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 463478c8-bbeb-4a97-aefe-1cabafd19f7f

📥 Commits

Reviewing files that changed from the base of the PR and between 41c7b8d and 2afa612.

📒 Files selected for processing (14)

CLAUDE.md
docs/superpowers/specs/2026-08-10-away-divider-design.md
src/SharpMUTerm.Core/Session/WorldSession.cs
src/SharpMUTerm.Core/Telnet/TelnetSession.cs
src/SharpMUTerm.Tui/AwayBarRenderer.cs
src/SharpMUTerm.Tui/Glyphs.cs
src/SharpMUTerm.Tui/SharpMUTermApp.cs
src/SharpMUTerm.Tui/TerminalFocusWatcher.cs
tests/SharpMUTerm.Core.Tests/Telnet/LoopbackServer.cs
tests/SharpMUTerm.Core.Tests/Telnet/MsspParsingTests.cs
tests/SharpMUTerm.Core.Tests/Telnet/UnsolicitedNegotiationTests.cs
tests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cs
tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs
tests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs

…ved bar off the screen Two defects from review, both in the away divider. TerminalFocusWatcher subscribes to the driver's KeyPressed, Paste and MouseEvent directly, and the driver raises those from the reader task it starts - not the UI thread. The framework's own key path does not run there: its driver handler only enqueues, and InputCoordinator drains on the main loop. So every other key handler in this client is on the UI thread and this one was not, while its handler iterated the pane buffers and mutated the away boundaries, the marks and the controls they repaint. A window opening during that foreach throws. The subscription cannot move: a queued key arrives too late to measure the gap in front of a focus-in Tab, which is the whole rule. NoteInput therefore keeps its timestamps on the reader thread and both handlers go through OnUiThread - which runs inline when it already is one, so headless and the harness are unchanged. Marshalling reorders, and that has a consequence worth naming. An input raised before the return that drew a bar can be delivered after it, and would then set InputSince on a bar nobody has looked at - retiring it to the very keystroke that produced it, which is precisely what the third consumption conjunct exists to prevent. Input now carries a count and AwayMark records the count it was drawn after, so a late note is recognisable as an early input. The second: MarkWhereTheReaderLeft removes the previous bar first (it has to, removal shifts every index after it) and then returns early for a window that gained nothing, without the repaint that the insert branch does. RemoveAwayBar deliberately does not repaint, on the understanding that its caller either follows with one or inserts over it - and this arm did neither, so the row left the buffer and stayed on the control, with no mark left to consume it. Reachable without touching the keyboard: a deep absence, then a second return to a quiet window. Tests: the ghost bar has one of its own. Two more stopped being able to pass for the wrong reason - TheBarIsNotCountedAsUnread never asserted a bar was drawn, and neither Tab test could tell a Tab that travelled on from one that was swallowed and drew nothing, which they now do by raising the second command line and watching the Tab cycle it. Frame assertions go through FrameGrid rather than searching the escape stream, and the enable/disable writes are asserted in order, since IsEquivalentTo ignores it. Docs: the design spec said nothing was implemented; the stale "three scroll* views" claim already missed freeze-scrollback and now misses away-scrollback; two sentences were broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX

HarryCordewenerand others added 7 commits August 10, 2026 14:37

Merge feat/away-divider: a bar where the reader was when they left th…
3d64660
…e terminal

Merge fix/unsolicited-telnet-option: the client asks no server to ena…
2afa612
…ble an option it has not offered

coderabbitaiBot reviewed Aug 10, 2026
View reviewed changes

HarryCordewener merged commit 72bb802 into mainAug 10, 2026
2 checks passed

HarryCordewener deleted the integrate/away-and-telnet-fix branch August 10, 2026 22:37

coderabbitaiBot mentioned this pull request Aug 11, 2026
Per-character pane tints: identity in hue, focus in luminance #22
Merged

Navigation Menu

Uh oh!

An away bar where the reader left off, and a login the client stopped breaking - #21

An away bar where the reader left off, and a login the client stopped breaking#21
HarryCordewener merged 8 commits into
mainfrom
integrate/away-and-telnet-fix

HarryCordewener commented Aug 10, 2026•
edited by coderabbitai Bot
Loading

Uh oh!

coderabbitaiBot commented Aug 10, 2026•
edited
Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitaiBot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

HarryCordewener commented Aug 10, 2026• edited by coderabbitai BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The away divider (feat/away-divider)

The unsolicited telnet option (fix/unsolicited-telnet-option)

Tests

Verification

Summary by CodeRabbit

Uh oh!

coderabbitaiBot commented Aug 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

coderabbitaiBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

HarryCordewener commented Aug 10, 2026•
edited by coderabbitai Bot
Loading

The away divider (`feat/away-divider`)

The unsolicited telnet option (`fix/unsolicited-telnet-option`)

coderabbitaiBot commented Aug 10, 2026•
edited
Loading