Skip to content

fix: make remote app-server proxy acquisition idempotent#23385

Open
efrazer-oai wants to merge 1 commit into
mainfrom
efrazer/codex/ssh-app-server-ownership
Open

fix: make remote app-server proxy acquisition idempotent#23385
efrazer-oai wants to merge 1 commit into
mainfrom
efrazer/codex/ssh-app-server-ownership

Conversation

@efrazer-oai
Copy link
Copy Markdown
Contributor

@efrazer-oai efrazer-oai commented May 19, 2026

Summary

Remote SSH clients can reconnect while an older detached app-server --listen unix:// process is still alive. Before this change, callers had to run their own "maybe start, then proxy" sequence, which could overlap and create duplicate startup work against one CODEX_HOME.

This PR adds a small app-server-owned acquisition primitive:

  • codex app-server proxy --ensure-listener
  • an advisory startup lock beside the control socket state
  • attach-first behavior that reuses an existing live listener
  • serialized listener creation with a readiness wait when no listener exists

Callers that do not pass --ensure-listener keep the existing attach-only proxy behavior.

What Changed

  • Added the startup lock path next to the existing control socket path.
  • Added a focused listener acquisition helper that:
    • checks for an existing control socket listener,
    • re-checks under the advisory lock,
    • spawns one detached listener only when needed,
    • keeps the lock until the socket is actually connectable,
    • surfaces a startup error if the detached child exits before binding the socket.
  • Extended codex app-server proxy with --ensure-listener.
  • Preserved the caller's explicit socket path and analytics default flag when a detached listener must be started.
  • Rejected root -c / --config overrides for proxy --ensure-listener, because a reused long-lived listener cannot safely apply process-scoped startup overrides after it already exists.
  • Kept detached listener logs under Codex-owned startup-control state instead of caller-provided socket directories.

Why

The app-server process is the only layer that can coordinate ownership of a remote control socket across multiple callers. Moving attach-or-start into the CLI gives desktop and other clients one idempotent contract instead of asking each caller to recreate a race-prone bootstrap protocol.

Design Decisions

  • Use advisory file locking through Rust's standard file-locking API.
  • Keep the change narrow and local to app-server startup ownership.
  • Preserve the existing attach-only proxy path for current callers.
  • Reject ambiguous process-scoped overrides on the ensure path rather than silently dropping them when a listener is reused.

Testing

Tests: targeted Rust transport coverage, targeted CLI proxy coverage, and recorded CLI smoke tests showing the old duplicate-bootstrap failure mode and the new concurrent --ensure-listener path collapsing to one listener without duplicate-start or SQLite startup errors.

Evidence

Before: blind duplicate bootstrap reaches conflicting remote listener startup.

ssh-ownership-before-window-recaptured.mov

After: concurrent --ensure-listener calls converge on one listener and a ready socket.

ssh-ownership-after-window-fresh.mov

@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

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: 838c631aab

ℹ️ 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/cli/src/main.rs Outdated
Comment thread codex-rs/cli/src/main.rs Outdated
Comment thread codex-rs/app-server-transport/src/transport/unix_socket.rs Outdated
@efrazer-oai efrazer-oai force-pushed the efrazer/codex/ssh-app-server-ownership branch from 838c631 to 2b24123 Compare May 19, 2026 00:53
@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

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: 2b241231a2

ℹ️ 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/cli/src/main.rs
Comment thread codex-rs/cli/src/main.rs
@efrazer-oai efrazer-oai force-pushed the efrazer/codex/ssh-app-server-ownership branch from 2b24123 to 3604750 Compare May 19, 2026 01:00
@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

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: 3604750780

ℹ️ 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-transport/src/transport/unix_socket.rs Outdated
Comment thread codex-rs/cli/src/main.rs
@efrazer-oai efrazer-oai force-pushed the efrazer/codex/ssh-app-server-ownership branch from 3604750 to 86c5f75 Compare May 19, 2026 01:07
@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

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: 86c5f75e20

ℹ️ 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-transport/src/transport/unix_socket.rs
@efrazer-oai efrazer-oai force-pushed the efrazer/codex/ssh-app-server-ownership branch from 86c5f75 to 345c054 Compare May 19, 2026 01:19
@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

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: 345c05405e

ℹ️ 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/cli/src/main.rs
Comment thread codex-rs/app-server-transport/src/transport/unix_socket_tests.rs
@efrazer-oai efrazer-oai force-pushed the efrazer/codex/ssh-app-server-ownership branch from 345c054 to 5daafb4 Compare May 19, 2026 01:55
@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ 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".

@efrazer-oai efrazer-oai force-pushed the efrazer/codex/ssh-app-server-ownership branch from 5daafb4 to 5b41d03 Compare May 19, 2026 02:11
@efrazer-oai
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ 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".

@efrazer-oai efrazer-oai marked this pull request as ready for review May 19, 2026 16:36
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