Add app-server background terminal process APIs#26041
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a5856f7ed
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d37f064d8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b90d036c9c
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0f9c328df
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cce9ca7334
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e78df1ea9d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b59232841e
ℹ️ 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".
|
|
||
| let entry = { | ||
| let mut store = self.process_store.lock().await; | ||
| let Some(entry) = store.processes.get(&process_id) else { |
There was a problem hiding this comment.
Can we verify this is still the same process before removing it?
There was a problem hiding this comment.
Yep, good call. Fixed.
| let Some(entry) = store.processes.get(&process_id) else { | ||
| return true; | ||
| }; | ||
| if !entry.initial_exec_command_returned { |
There was a problem hiding this comment.
(found by Codex)
This flag can remain false after the initial exec future has been canceled, not just while it is still in flight. A later terminate then succeeds forever while leaving the exited entry, reserved ID, and any approval registration behind. Can we track whether the initial call is actually live, or clean this up from its cancellation/drop path?
| limit: Option<u32>, | ||
| ) -> Result<(Vec<ThreadBackgroundTerminal>, Option<String>), JSONRPCErrorError> { | ||
| let start = match cursor { | ||
| Some(cursor) => terminals |
There was a problem hiding this comment.
Can we make this cursor survive the anchor process exiting?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28b3f1ea58
ℹ️ 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".
Summary
Codex Apps needs app-server as the source of truth for chat-started background terminals instead of guessing from local process trees.
This PR adds experimental v2 APIs to list and terminate background terminals for a loaded thread using app-server process ids, so clients can manage background terminals without local PID discovery.
Changes
thread/backgroundTerminals/listreturns paginated background terminal records withitemId, app-serverprocessId,command,cwd, nullableosPid, nullablecpuPercent, and nullablerssKb.thread/backgroundTerminals/terminateterminates one running background terminal by app-serverprocessIdand returns whether a process was terminated.