Skip to content

fix: make app-server connection loss terminal - #685

Open
seungpyoson wants to merge 3 commits into
openai:mainfrom
seungpyoson:fix/app-server-terminal-lifecycle
Open

fix: make app-server connection loss terminal#685
seungpyoson wants to merge 3 commits into
openai:mainfrom
seungpyoson:fix/app-server-terminal-lifecycle

Conversation

@seungpyoson

Copy link
Copy Markdown

Summary

  • make app-server connection loss a single terminal transition that rejects pending RPCs, future RPCs, and turn-completion waiters
  • bound initialization and owned-child teardown without adding a production turn-duration timeout
  • make broker shutdown idempotent and able to close half-open clients and reap failed-startup processes
  • add behavioral coverage for parse failures, non-object messages, EOF, write failures, early child exit, process-tree cleanup, and shutdown races

Motivation

The app-server client previously tracked logical protocol failure separately from process closure. Several connection-loss paths could therefore leave later requests or turn-completion waits unsettled, and initialization failure could return without reaping the owned launcher tree.

This change gives the connection one write-once terminal cause and routes every relevant protocol or process failure through it. Initialization and teardown have bounded lifecycle deadlines; normal Codex turns remain unbounded.

Testing

  • npm test (106 tests)
  • npm run build

@seungpyoson
seungpyoson requested a review from a team August 25, 2026 13:35

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a01a9f8bcf

ℹ️ 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 tests/runtime.test.mjs
Comment thread plugins/codex/scripts/lib/app-server.mjs Outdated
Comment thread plugins/codex/scripts/lib/app-server.mjs Outdated

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 92ed77c7f4

ℹ️ 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 tests/app-server-lifecycle.test.mjs
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