Skip to content

app-server: notify clients of remote-control status changes#19919

Merged
euroelessar merged 7 commits intomainfrom
ruslan/app-server-remote-control-environment-notification
Apr 28, 2026
Merged

app-server: notify clients of remote-control status changes#19919
euroelessar merged 7 commits intomainfrom
ruslan/app-server-remote-control-environment-notification

Conversation

@euroelessar
Copy link
Copy Markdown
Collaborator

@euroelessar euroelessar commented Apr 28, 2026

Why

Remote-control app-server enrollments have both an internal server id and the environment id exposed to remote-control clients. App-server clients need one current status snapshot that says whether remote control is usable and which environment id, if any, is exposed.

A temporary websocket disconnect is not itself an identity change. Account changes, stale enrollment invalidation, successful re-enrollment, and missing ChatGPT auth are meaningful status changes. Disabled remote control remains disabled regardless of auth or SQLite state. SQLite startup failure disablement and enrollment persistence failures are handled in #20068; this PR reports the resulting effective status to clients.

What changed

  • Adds v2 remoteControl/status/changed carrying state and environmentId.
  • Adds RemoteControlConnectionState values: disabled, connecting, connected, and errored.
  • Exposes remote-control status updates through RemoteControlHandle using a Tokio watch channel.
  • Always sends the current remote-control status snapshot to newly initialized app-server clients.
  • Broadcasts status changes to initialized app-server clients when state or environment id changes.
  • Treats missing ChatGPT auth as an errored status while leaving it retryable because auth can change at runtime.
  • Clears environmentId when enrollment is cleared for account changes, auth loss, stale backend invalidation, or disabled remote control.
  • Updates app-server protocol schema fixtures, generated TypeScript, app-server README, remote-control tests, and TUI exhaustive notification matches.

Stack

Verification

  • just write-app-server-schema
  • cargo test -p codex-app-server-protocol
  • cargo test -p codex-app-server transport::remote_control --lib
  • cargo check -p codex-tui
  • just fix -p codex-app-server-protocol
  • just fix -p codex-app-server
  • just fix -p codex-tui

euroelessar added a commit that referenced this pull request Apr 28, 2026
Copy link
Copy Markdown

@SagarJadhav30 SagarJadhav30 left a comment

Choose a reason for hiding this comment

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

best intreseting

@euroelessar euroelessar marked this pull request as draft April 28, 2026 17:49
@euroelessar euroelessar changed the title app-server: notify clients of remote-control environment changes app-server: notify clients of remote-control status changes Apr 28, 2026
euroelessar added a commit that referenced this pull request Apr 28, 2026
@euroelessar euroelessar force-pushed the ruslan/app-server-remote-control-environment-notification branch from c91d450 to 57e804b Compare April 28, 2026 18:21
euroelessar added a commit that referenced this pull request Apr 28, 2026
@euroelessar euroelessar force-pushed the ruslan/app-server-remote-control-environment-notification branch from 453c5eb to a6576d5 Compare April 28, 2026 19:31
@euroelessar euroelessar changed the base branch from main to ruslan/app-server-disable-remote-control-without-sqlite April 28, 2026 19:31
@euroelessar euroelessar force-pushed the ruslan/app-server-disable-remote-control-without-sqlite branch from 38507f2 to 8ff72d0 Compare April 28, 2026 19:42
euroelessar added a commit that referenced this pull request Apr 28, 2026
@euroelessar euroelessar force-pushed the ruslan/app-server-remote-control-environment-notification branch from a6576d5 to 97b944a Compare April 28, 2026 19:51
@euroelessar euroelessar marked this pull request as ready for review April 28, 2026 19:54
Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97b944a76b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/app-server/src/transport/remote_control/websocket.rs
Comment thread codex-rs/app-server/src/transport/remote_control/websocket.rs Outdated
Comment thread codex-rs/app-server/src/transport/remote_control/websocket.rs
Copy link
Copy Markdown
Collaborator

@owenlin0 owenlin0 left a comment

Choose a reason for hiding this comment

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

focused mostly on the API and skimmed the impl given that it is isolated to remote_control. codex flagged some comments though, worth addressing?

Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs Outdated
Comment thread codex-rs/app-server-protocol/src/protocol/v2.rs Outdated
Base automatically changed from ruslan/app-server-disable-remote-control-without-sqlite to main April 28, 2026 20:49
@euroelessar euroelessar force-pushed the ruslan/app-server-remote-control-environment-notification branch from 97b944a to 6414787 Compare April 28, 2026 21:27
@euroelessar euroelessar enabled auto-merge (squash) April 28, 2026 23:51
@euroelessar euroelessar merged commit c6465c1 into main Apr 28, 2026
25 checks passed
@euroelessar euroelessar deleted the ruslan/app-server-remote-control-environment-notification branch April 28, 2026 23:52
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants