🛡️ fix: Let Background Subagents Run With HITL Enabled - #571
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: c557539d30
ℹ️ 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".
| if (this.humanInTheLoop?.backgroundPausePolicy === 'deny') { | ||
| childGraph.humanInTheLoop = this.humanInTheLoop; | ||
| childGraph.eagerEventToolExecution = undefined; |
There was a problem hiding this comment.
When a real Run has HITL enabled, snapshotChildGraphFactory has already attached the parent's checkpointer (src/graphs/Graph.ts:5311-5313) before this override runs. Because the detached executor is configured with enabled: false, its invocation path then reuses the inherited parent thread_id instead of childThreadId (SubagentExecutor.ts:2816-2819). A default-policy background child therefore runs an independent graph against the same root checkpoint namespace as its parent, potentially loading/merging parent state and concurrently overwriting checkpoints used for parent resume or sibling tasks. Clear the inherited checkpointer for deny-policy children or assign each child a dedicated checkpoint thread/namespace.
Useful? React with 👍 / 👎.
|
Self-review follow-up pushed at exact head Codex P1 fixed: a task-private saver and distinct child thread prevent reading the parent checkpoint. A real Run test reproduced that leak before the fix and passed afterward. My review also fixed fail-open background hook errors (direct/event tools, programmatic inner tools, and SubagentStart), unexpected interrupts being reported as successful tasks, and explicit background deny changing foreground HITL resume. Local verification: 420 focused tests passed; TypeScript, touched-file ESLint/import order, SDK build, and whitespace checks passed. A maintainer must request a new Codex review for this exact head. |
|
@codex review the latest head |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Summary
denyfor tools requesting approval, whilebackgroundPausePolicy: 'reject'preserves the former admission check.ask_user_question, report denied tools to the parent, and fail unexpected interrupts clearly.denydoes not break foreground HITL or cross-run resume.Verification
npx tsc --noEmit, ESLint/import order on nine touched files,npm run build, andgit diff --check: passed.Scope
SDK Phase 0 only. Durable deferred approvals and LibreChat configuration/UI remain separate work.