Skip to content

feat(websh): announce idle-warning and name an idle detach - #675

Merged
Eunyoung Jeong (eunyoung14) merged 7 commits into
mainfrom
feat/420-idle-warning
Oct 31, 2026
Merged

Eunyoung Jeong (eunyoung14) merged 7 commits into
mainfrom
feat/420-idle-warning

Conversation

@eunyoung14

@eunyoung14 Eunyoung Jeong (eunyoung14) commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Summary

Websh idle handling disconnects the CLI while the shell keeps running on the server until the work session ends. Every client receives the same neutral warning, including shared joins and read-only watches.

  • Every Websh dial advertises idle-warning. Owner sessions retain websocket-reconnect on initial and reconnect dials.

  • A valid idle_warning frame prints the deadline in local time to stderr:

    [Idle: this session disconnects at 15:04:05 without input.]
    

    Notices remain deduplicated per deadline across reconnects, ignore invalid or elapsed deadlines, and leave terminal output and replay offsets untouched. The server's message field is never printed.

  • Close code 4000 with reason idle_detached ends the CLI without an automatic reconnect and prints:

    session disconnected after inactivity; the shell keeps running on the server until the work session ends
    
  • The obsolete Websh idle_timeout wording is removed. File transfers are unchanged.

  • Sessions without a work session close on idle_timeout and print session closed after inactivity; close code 4000 never reconnects automatically.

Reattach behavior

There is no owner reattach command after the CLI exits. alpacon websh SERVER creates a new session. alpacon websh join --url URL --password PASSWORD accepts a shared invite channel. Automatic recovery applies only while the owner client is running.

Validation

The updated warning assertions failed on the old wording before the fix. The regression suite covers local-time output, duplicate deadlines, history pruning, replay preservation, and a clean idle-detach exit with one dial and zero reconnect provisions.

  • go vet ./...: passed.
  • go test -race -count=1 ./...: passed.
  • gofmt -l .: empty.
  • golangci-lint run: passed.
  • go mod tidy: no dependency changes.

Pipeline results

  • Comment tidy: retained the three requested close-code explanations; no additional edits.
  • Test tidy: no consolidation needed for the review fixes.
  • Independent review: approved for the copy and comment fixes; three mechanical findings fixed (unused read-only state and Go comment form).
  • Config drift: no applicable changes.

Show a local-time warning on stderr before an idle close, ignore invalid or repeated warning deadlines, and explain how to reopen a session closed for inactivity.
Copilot AI lite review requested due to automatic review settings October 30, 2026 23:36
Copilot AI previously approved these changes Oct 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

The review status indicates that no active issues remain for the change.

No open issues 🎉

What changed in this PR

This pull request adds idle-timeout warnings for Websh sessions and updates the inactivity close message.

Changes:

  • Displays validated idle deadlines on stderr in the user’s local time.
  • Deduplicates warnings across reconnects without changing replay offsets.
  • Updates the idle_timeout close message.
  • Adds focused coverage for warning validation, output, replay behavior, and reconnects.

This pull request requires changes.

File Summary
api/​websh/​websh.go Handles idle-warning frames, validation, local-time formatting, deduplication, and the updated close message. Moderate: prune expired deadlines or otherwise bound retained warning history.
api/​websh/​websh_test.go Updates expectations for the inactivity close message.
api/​websh/​types.go Stores idle-warning deadlines across connections.
api/​websh/​idle_warning_test.go Tests warning output, validation, replay preservation, and reconnect deduplication.

Review effort: Lite

@eunyoung14

Copy link
Copy Markdown
Member Author

Moderate: prune expired deadlines or otherwise bound retained warning history.

Fixed in 8e03c87. api/websh/websh.go:726 now removes elapsed deadlines when a valid warning arrives and keeps future deadlines for deduplication. api/websh/idle_warning_test.go:102 verifies that expired history is removed, a future duplicate stays silent, a new future deadline prints once on stderr, and an expired frame stays silent.

Validation passed: vet, the full race suite, formatting, and lint. The first race run hit an event-listener timing assertion while lint was running; the full rerun without the linter passed.

Resolved this review-body finding with a regression test; reverting 8e03c87 restores the previous history retention behavior.

@alpacax-wall-d alpacax-wall-d Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WALL-D review: Approval withheld · head 8e03c87

Code review result: Comments only (1 P1/P2) · head 8e03c87

Review summary

  • Found 1 P1/P2 defect.

A maintainer will review this PR.

Re-review requests and limits

WALL-D reviews a PR opened ready after this rule took effect once on its own. After that it reviews only on request: comment @alpacax-wall-d review or mark a draft ready for review, up to 5 times per PR including the first review. A re-review reads only the lines changed since the last review, and from the second re-review on only P1 findings block approval. Answering the product review's questions reviews only the product side again.

Comment thread api/websh/websh.go Resolved
@alpacax-wall-d alpacax-wall-d Bot added the needs-author-fix WALL-D: fix needed on the head (P1/P2 finding, product fix-before-merge item, or failing CI) label Oct 30, 2026
@eunyoung14

Copy link
Copy Markdown
Member Author

@alpacax-wall-d review

@alpacax-wall-d alpacax-wall-d Bot removed the needs-author-fix WALL-D: fix needed on the head (P1/P2 finding, product fix-before-merge item, or failing CI) label Oct 31, 2026

@alpacax-wall-d alpacax-wall-d Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WALL-D review: Approval withheld · head 6e2843c

Code review result: 1 earlier finding resolved · head 6e2843c

Product review result: Check needed (1 item, 1 to fix before merge) · head 6e2843c
A maintainer will follow up on the product items.

Re-review requests and limits

WALL-D reviews a PR opened ready after this rule took effect once on its own. After that it reviews only on request: comment @alpacax-wall-d review or mark a draft ready for review, up to 5 times per PR including the first review. A re-review reads only the lines changed since the last review, and from the second re-review on only P1 findings block approval. Answering the product review's questions reviews only the product side again.

@alpacax-wall-d

Copy link
Copy Markdown

Reviewed 6e2843c: #675 (review)
#675 (comment)

@alpacax-wall-d alpacax-wall-d Bot added the needs-author-fix WALL-D: fix needed on the head (P1/P2 finding, product fix-before-merge item, or failing CI) label Oct 31, 2026
@eunyoung14 Eunyoung Jeong (eunyoung14) changed the title feat(websh): warn before an idle close and name it on close feat(websh): announce idle-warning and name an idle detach Oct 31, 2026
@eunyoung14

Copy link
Copy Markdown
Member Author

@alpacax-wall-d review

@alpacax-wall-d alpacax-wall-d Bot removed the needs-author-fix WALL-D: fix needed on the head (P1/P2 finding, product fix-before-merge item, or failing CI) label Oct 31, 2026
alpacax-wall-d[bot]
alpacax-wall-d Bot previously approved these changes Oct 31, 2026

@alpacax-wall-d alpacax-wall-d Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WALL-D review: Approved · head 072fb94 · no P1/P2, Product review result: Nothing flagged

@alpacax-wall-d

Copy link
Copy Markdown

Reviewed 072fb94: #675 (review)
#675 (comment)

@eunyoung14

Copy link
Copy Markdown
Member Author

@alpacax-wall-d review

@alpacax-wall-d alpacax-wall-d Bot added the needs-author-answer WALL-D: the product review left questions for the author label Oct 31, 2026
@eunyoung14

Copy link
Copy Markdown
Member Author

WALL-D product question (companion server behavior for sessions without a work session): confirmed. In the same release, the proxy and the server close an idle Websh session that has no work session, with code 4000 and reason idle_timeout; this is the session the CLI's session closed after inactivity line names. A session inside a work session is detached instead (idle_detached). The release ships this CLI and the web client before the proxy change, so no client sees either reason before it can name it.

Posted by the PR's coordinating agent to answer a WALL-D product question. To undo, delete this comment.

@alpacax-wall-d alpacax-wall-d Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WALL-D review: Approval withheld · head 07f14f1

Code review result: no P1/P2 · head 07f14f1

Product review result: Check needed (1 item) · head 07f14f1
A maintainer will follow up on the product items.

Re-review requests and limits

WALL-D reviews a PR opened ready after this rule took effect once on its own. After that it reviews only on request: comment @alpacax-wall-d review or mark a draft ready for review, up to 5 times per PR including the first review. A re-review reads only the lines changed since the last review, and from the second re-review on only P1 findings block approval. Answering the product review's questions reviews only the product side again.

@alpacax-wall-d

Copy link
Copy Markdown

Reviewed 07f14f1: #675 (review)
#675 (comment)

@eunyoung14

Copy link
Copy Markdown
Member Author

The companion server and proxy changes now close an idle Websh session that has no work session with code 4000 and reason idle_timeout (the server side is merged; the proxy side is in review), and detach sessions inside a work session with idle_detached. @alpacax-wall-d review

@eunyoung14

Copy link
Copy Markdown
Member Author

Answer to the open product question on api/websh/websh.go:64: no CLI change is needed. The paired server and proxy changes now close an idle Websh session that has no work session, and they send the close reason idle_timeout. They detach, with idle_detached, only sessions that run inside a work session, which is what bounds the surviving shell. So a token-opened session without a work session gets this CLI's idle_timeout message ("after inactivity"). The "keeps running on the server until the work session ends" text appears only for idle_detached, where it's true. This matches the design rule that a terminal opened without a work session closes when idle.

@eunyoung14

Copy link
Copy Markdown
Member Author

@alpacax-wall-d review

@alpacax-wall-d

Copy link
Copy Markdown

No changes in 07f14f1..07f14f1.
#675 (comment)

@alpacax-wall-d alpacax-wall-d Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WALL-D review: Approved · head 07f14f1 · Product review result: Nothing flagged

@alpacax-wall-d alpacax-wall-d Bot removed the needs-author-answer WALL-D: the product review left questions for the author label Oct 31, 2026
@eunyoung14
Eunyoung Jeong (eunyoung14) merged commit 65375be into main Oct 31, 2026
12 checks passed
@eunyoung14
Eunyoung Jeong (eunyoung14) deleted the feat/420-idle-warning branch October 31, 2026 08:24
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.

2 participants