[codex] kill timed-out Git status process groups#30235
Conversation
tamird
force-pushed
the
codex/kill-timed-out-git-process-tree
branch
2 times, most recently
from
0d178b5 to
adcd4dc
Compare
tamird
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adcd4dcbbd
ℹ️ 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".
| command | ||
| .kill_on_drop(true) | ||
| .stdout(Stdio::piped()) | ||
| .stderr(Stdio::piped()); |
There was a problem hiding this comment.
When run_git_status_with_timeout_from passes a fresh Command here, this setup pipes only stdout/stderr and leaves stdin at the spawn default, inherited from Codex, whereas the old command.output() path supplied EOF/null stdin. In an interactive session, any git wrapper that reads stdin can block until the five-second timeout or consume user input before being killed; set stdin to Stdio::null() or otherwise close it before spawn().
Useful? React with 👍 / 👎.
Codex limits `git status --porcelain` to five seconds. If `git` resolves to a wrapper, Tokio's drop cleanup kills only the wrapper. The wrapped Git process can keep scanning the worktree without an output reader. On Unix, run status in its own process group. When the timeout expires, kill the group and wait for the wrapper before returning.
tamird
force-pushed
the
codex/kill-timed-out-git-process-tree
branch
from
adcd4dc to
1e0deb5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e0deb5346
ℹ️ 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".
| "codex-protocol", | ||
| "codex-utils-absolute-path", | ||
| "codex-utils-path-uri", | ||
| "codex-utils-pty", |
There was a problem hiding this comment.
This lockfile update records codex-utils-pty under codex-file-system, but the changed manifest adds and the new code uses it from codex-git-utils; codex-rs/file-system/Cargo.toml has no such dependency and the codex-git-utils lock entry still lacks it. Locked Cargo/Bazel resolution is therefore stale, so regenerate Cargo.lock with the dependency under codex-git-utils and refresh the Bazel lockfile in the same change.
AGENTS.md reference: AGENTS.md:L37-L39
Useful? React with 👍 / 👎.
|
Good fix! Killing timed-out git status processes prevents zombie processes from accumulating. Adding the io-util tokio feature for PTY handling is the right approach. |
2 participants
Codex limits
git status --porcelainto five seconds. Ifgitresolves to a wrapper, Tokio's drop cleanup kills only the wrapper.
The wrapped Git process can keep scanning the worktree without an
output reader.
On Unix, run status in its own process group. When the timeout expires,
kill the group and wait for the wrapper before returning.