Skip to content

Send the initial pane size when NAWS negotiation completes - #46

Merged
HarryCordewener merged 1 commit into
mainfrom
codex/fix-initial-naws
Oct 16, 2026
Merged

HarryCordewener merged 1 commit into
mainfrom
codex/fix-initial-naws

Conversation

@HarryCordewener

@HarryCordewener HarryCordewener commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

SharpMUTerm could cache its initial pane size as sent before the server's DO NAWS arrived. TNC correctly skipped the premature subnegotiation, leaving SharpMUSH's width at its 78-column fallback until a resize.

Retain the latest dimensions in a NAWS plugin and send them from TNC's negotiation-completion callback. Serialize reports, skip duplicates, and resend after DONT/DO even when the dimensions are unchanged. TNC continues to own binary framing and IAC escaping. The explicit plugin list preserves the existing default protocols and callbacks.

Validation: reproduced the failure before the fix; all 3,134 tests across Core, Tui, Graphics, Scripting, and Web pass. Full Release build, changed-file whitespace verification, and headless snapshot smoke pass. Build reports existing ImageSharp vulnerability warnings. Independent code review found no actionable issues.

Summary by CodeRabbit

  • New Features
    • Window-size updates are now retained until the server negotiates NAWS, then the latest size is reported automatically.
    • After NAWS is enabled, updated dimensions are reported; unchanged sizes are not sent again. If NAWS is disabled and later re-enabled, the latest size is reported again.
    • Window dimensions outside the supported range are rejected.
  • Documentation
    • Clarified how window-size updates behave before and after NAWS negotiation.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Essentials
  • Run ID: b9fa0d9f-5dae-46d5-b015-d0c5b795847a
📥 Commits

Reviewing files that changed from the base of the PR and between d4371a8 and 0be1a28.

📒 Files selected for processing: 4
  • src/SharpMUTerm.Core/Telnet/ITelnetSession.cs
  • src/SharpMUTerm.Core/Telnet/TelnetSession.cs
  • src/SharpMUTerm.Core/Telnet/WindowSizeProtocol.cs
  • tests/SharpMUTerm.Core.Tests/Telnet/TelnetSessionTests.cs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Walkthrough

The Telnet session now registers protocol plugins explicitly and routes window-size requests through `WindowSizeProtocol. The protocol retains the latest dimensions and reports them when NAWS negotiation and reporting settings allow.

Changes

NAWS window-size reporting

Layer / File(s) Summary
Window-size reporting state
src/SharpMUTerm.Core/Telnet/WindowSizeProtocol.cs, src/SharpMUTerm.Core/Telnet/ITelnetSession.cs
WindowSizeProtocol validates and retains requested dimensions. It reports dimensions when NAWS is negotiated and reporting is enabled, skips unchanged dimensions within an agreement, and permits retry after a failed send. The interface documentation describes deferred reporting and duplicate suppression.
Session integration and negotiation coverage
src/SharpMUTerm.Core/Telnet/TelnetSession.cs, tests/SharpMUTerm.Core.Tests/Telnet/TelnetSessionTests.cs
BuildInterpreterAsync registers protocol plugins explicitly. SetWindowSizeAsync reports through WindowSizeProtocol and throws if that plugin is unavailable or has a different type. Tests cover deferred reporting, fragmented negotiation input, escaped dimensions, duplicate sizes, and disable/re-enable behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 0be1a

The change sends the retained pane size once NAWS negotiation completes, which fixes the fallback to 78 columns. No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed 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 and concisely describes the main change: sending the initial pane size when NAWS negotiation completes.
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@HarryCordewener
HarryCordewener merged commit 0ab87d7 into main Oct 16, 2026
3 checks passed
Sign up for free to 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