Skip to content

fix(server): drop typing and keys whose connection closed while they waited behind a message - #235

Merged
devswha merged 3 commits into
mainfrom
fix/queued-input-after-close
Oct 1, 2026
Merged

devswha merged 3 commits into
mainfrom
fix/queued-input-after-close

Conversation

@devswha

@devswha devswha commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Closes #233.

What

input and keys that wait behind a composer message in the pane's queue are dropped when the connection that sent them has closed by the time their turn comes. submit is unchanged: a message sent just before the phone locks is still finished.

The path for a PC without terminal attach already had this check (#227); this adds it to the attached path and to keys.

Tests

  • server/submit.contract.test.ts: an attached connection sends a message, a Stop and an Enter, then closes. Only the message reaches the pane. The test fails with the two checks removed (the ESC and the Enter arrive).
  • bun run typecheck; bun test ./server/submit.contract.test.ts (18 pass).

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: bd7440bb-d0b8-477f-ba1b-e9ffeda6320e

📥 Commits

Reviewing files that changed from the base of the PR and between 4409103 and 049fd4f.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 34d2652f-3f5e-47e3-92bf-30fa51c55b00

📥 Commits

Reviewing files that changed from the base of the PR and between 162e5a5 and 4409103.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • server/index.ts
  • server/submit.contract.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Prevented queued Stop and Enter inputs from reaching a pane after the connection that sent them has closed. The message itself still completes, and later submissions from other connections continue normally.

Walkthrough

Queued input and key actions are now discarded if the originating client disconnects while they wait in the pane queue. The submitted message can still reach the pane. A contract test covers this behavior and a subsequent submit from another connection.

Changes

Queued client actions

Layer / File(s) Summary
Active-client checks and queued-action contract
server/index.ts, server/submit.contract.test.ts, CHANGELOG.md
Queued input and key callbacks return without sending actions when the client is no longer active. The contract test verifies message delivery, discarded queued actions, and a later successful submit. The changelog records the behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: yoonwoo-ha

Merge Risk: ⚪ Minimal · up to 44091

No actionable merge risk is identified in the supplied change context.

Architecture Summary

Architecture risk: 🔵 Low · up to 44091

The change affects 2 systems.

Changed systems: server, CHANGELOG.md

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — server (service) was modified; 2 changed files map to changed impact.
  • observed — CHANGELOG.md (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in CHANGELOG.md: Adds an Unreleased fixed entry describing the post-close input behavior: Stop, Enter, and other immediately typed text no longer reach the pane after the sending connection closes, while the message itself can still finish for a locked phone.
  • observed — Modified behavior in server/index.ts: The queued input callback now returns without sending text when its client is no longer active.
  • observed — Modified behavior in server/index.ts: The queued key callback now returns without sending keys when its client is no longer active.
  • observed — Modified behavior in server/submit.contract.test.ts: Adds a contract test for queued work from a disconnected connection. It verifies that the submitted text still reaches the pane, queued input and key events are discarded, and a subsequent submit from another connection succeeds.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary server fix: queued typing and key actions are dropped when their connection closes.
Description check ✅ Passed The description explains the problem, expected behavior, scope, regression coverage, and validation results. The headings differ from the template, but the required information is present.
Linked Issues check ✅ Passed Issue #233 requires queued attached-pane input and keys actions to be discarded when their WebSocket closes, while queued submit actions still finish. server/index.ts now checks that the origi…
Out of Scope Changes check ✅ Passed The changes remain within Issue #233. They modify the attached-pane queue behavior, add a regression test, and document the fix in CHANGELOG.md. The no-terminal-attach path remains unchanged, as req…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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

@devswha
devswha merged commit bf9fc63 into main Oct 1, 2026
11 checks passed
@devswha
devswha deleted the fix/queued-input-after-close branch October 1, 2026 08:08
devswha added a commit that referenced this pull request Oct 1, 2026
Release v0.3.39: moves the Unreleased notes under 0.3.39 and bumps
`package.json` and `herdr-plugin.toml`.

In this release, all fixes: a password sent right after a reconnect
(#244), typing and keys outliving their connection (#235), panning a
terminal whose grid is not the browser's own on a phone (#241), omo's
question form in the chat (#237 by @nahwan-kim), and a held pane resumed
by a late herdr answer plus a new shell taken for OmO (#243). No remote
bundle change: `REMOTE_BUNDLE_VERSION` stays 9.

Changelog housekeeping that was waiting for a release:
- Compare links at the bottom now cover 0.3.31 to 0.3.39 (they stopped
at 0.3.30), and `[Unreleased]` compares from v0.3.39.
- Five references that were a bare `#N` are full PR links with their
author: #164 is the issue PR #199 fixed, #154 the issue PR #155 closed,
and #45 and #6 are PRs. No other wording of released entries changed.

`bun scripts/release-notes.ts 0.3.39` prints the notes and exits 0.
devswha added a commit that referenced this pull request Oct 1, 2026
Fixes #256.

Raises `REMOTE_BUNDLE_VERSION` from 9 to 10, so that remote PCs (Windows
PCs included) get the bridge-side fixes merged since `remote-v9`: #235,
#243, #249, #251, #264, #265, #267, #278, #281 and the rest of `git log
remote-v9..main -- server shared`.

`server/machines.test.ts` named the `remote-v9` directory literally and
failed on the new number; it now reads the constant.

## Order, from docs/remote-pcs.md
1. Merge this.
2. Push the `remote-v10` tag on `main`: the Remote PC bundles workflow
builds, smoke-tests and publishes the five archives and `manifest.json`.
3. Only then release the app: an app that carries the new number finds
no `remote-v10` manifest before step 2, and every connected PC's bridge
counts as incompatible until it is updated.

## Checked
`bun run typecheck`, `bun run test:unit` (734 pass). The bundle build
and smoke run in this PR's Remote PC bundles checks.
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.

Typing and keys queued behind a message are still sent after their connection closed

1 participant