🤖 fix: let built-in forked explore tasks run in parallel#3576
Conversation
|
@codex review |
![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: 79919ddffb
ℹ️ 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".
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
79919dd to
5db9e90
Compare
|
Addressed in 5db9e90. The exemption is now per-call and runtime-aware instead of blanket. A new pure helper
Behavioral tests cover the matrix: worktree default/ |
|
@codex review Please take another look. The blanket task exemption is now per-call and runtime-aware (only provably-forked task calls skip the serialization mutex; shared-checkout task calls and all other tools stay serialized). |
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
5db9e90 to
c4ade72
Compare
|
@codex review Pushed a formatting-only fix (Prettier) on top of the runtime-aware change. No behavioral difference. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4ade72cc8
ℹ️ 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".
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
c4ade72 to
1505b69
Compare
|
Addressed in 1505b69. You're right that a forked task still touches the parent checkout during Replaced the blunt per-stream mutex with a per-stream read/write lock (
So within a stream: forked task ‖ forked task overlap (both readers), but forked task ‖ bash / file-write are mutually exclusive (read‖write), in either start order — no more concurrent access to the parent checkout. The classifier ( The RW lock is FIFO-fair (a queued writer blocks later readers from jumping ahead, so writers can't starve). Tests cover reader overlap, both-direction read/write exclusion, writer serialization, queued-writer fairness, and double-dispose safety, plus the task/bash interleaving matrix in One intentional conservatism (documented inline): a foreground forked task holds the read lock across its whole execute chain incl. |
|
@codex review Replaced the per-stream mutex with a read/write lock so forked task calls overlap each other (readers) but stay mutually exclusive with mutating tools (writers). Please take another look. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1505b695aa
ℹ️ 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".
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
1505b69 to
7c44e03
Compare
ethanndickson
changed the title
|
Re: Codex P2 on |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c44e031bf
ℹ️ 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 review Addressed the create-on-missing race (P2) in 546db28: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 546db280a3
ℹ️ 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 review Addressed the mux_config_write read-modify-write race (P2) in a5f6c39: the read → applyMutations → write cycle now runs under a per-config-document lock ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5f6c39811
ℹ️ 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 review Addressed the background process-ID allocation race (P2) in 591c134: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 591c134400
ℹ️ 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 review Addressed the cross-layer config race (P2) in edd89a3: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edd89a3aa4
ℹ️ 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 review Addressed the providers/secrets locking P2: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61ed07a05e
ℹ️ 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 review Addressed the routePriority TOCTOU (P2) in 037f3ae: |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01772febe4
ℹ️ 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 review The remaining P2 (local-runtime explore tasks sharing the checkout) is an intentional maintainer decision: explore is read-only by prompt contract, and parallel explore tasks sharing a checkout — including best-effort concurrent bash writes — is explicitly accepted, not a race we guard against. Reader parallelism is gated on the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dce6e4fbad
ℹ️ 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 review Addressed the P2 about hook-wrapped task tools: parent-side |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e02165f40
ℹ️ 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 Review: Didn't find any major issues. Already looking forward to the next diff. 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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 372487ec24
ℹ️ 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 Review: Didn't find any major issues. 🎉 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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". |
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
ec9ed56 to
95d09a6
Compare
|
@codex review Rebased onto latest |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
7f48e07 to
0d8bceb
Compare
|
@codex review This push slims the PR to the two load-bearing production files ( re: "Preserve the task marker when cache control recreates tools" (P3, previously fixed in Dropped, because the path is unreachable. re: "Keep task hooks serialized for explore fan-out" (P2, previously fixed by stripping the marker in Dropped, because that logic was itself the regression it was meant to prevent. It gated on "the project has any task hook configured" rather than "the hook actually affects |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d8bceb7c6
ℹ️ 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".
Replace the per-stream AsyncMutex in withSequentialExecution with a fair reader-writer lock (SharedExecutionLock). Built-in `task` calls targeting the `explore` agent (without `isolation: "none"`) share the read side and overlap with each other; every other tool keeps the exclusive write lock and stays serialized. The built-in task tool is identified by a non-forgeable Symbol marker set in createTaskTool. Preserve that Symbol marker through Anthropic cache-control recreation (applyCacheControlToTools), which rebuilds the last function tool via createTool() and would otherwise drop the marker — silently serializing explore fan-out whenever tool-policy filtering leaves `task` as the last tool.
ethanndickson
force-pushed
the
parallel-tools-k5nv
branch
from
0d8bceb to
2d45a77
Compare
|
@codex review re: "Preserve the task marker after Anthropic cache wrapping" (P3) Fixed in 2d45a77. You were correct that tool-policy filtering can leave |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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". |
1 participant
Summary
Sub-agent exploration regained its parallelism: built-in
taskcalls to theexploreagent now run concurrently with each other again, while every mutating tool stays strictly serialized as before. The agent could emit several sibling tool calls intending parallel execution, but only the first actually ran — the rest serialized despite the provider advertisingparallelToolCalls: true.Background
PR #2906 (commit
386346261) wrapped every tool'sexecute()in a single shared per-streamAsyncMutex(withSequentialExecution). That made all sibling tool calls run one-at-a-time, which is the safe default for mutating tools (file edits, bash, config writes) but also defeated parallel sub-agent fan-out — the most visible casualty being thetasktool spawningexploresub-agents.A full revert was rejected because broad parallelism re-introduces real races that #2906 quietly fixed: shared-checkout task collisions, background-process ID collisions, and concurrent config read-modify-write. This change keeps all of those protections and only carves out the one case that is provably safe to overlap.
Implementation
The per-stream mutex is replaced with a fair reader-writer lock (
SharedExecutionLock):taskcalls that targetagentId: "explore"and do not requestisolation: "none". These overlap with each other but never with a writer.The exception is gated on three independent conditions so it can't widen accidentally:
tasktool, identified by a non-forgeableSymbolmarker (markBuiltInTaskTool/isBuiltInTaskTool) rather than by tool name. A user/MCP tool that happens to be namedtaskdoes not qualify.explore(read-only by contract).isolation: "none"), so forked-checkout explore tasks can't race on a shared working tree.The lock uses a turnstile pattern (turnstile + room-empty + reader-count mutexes) so writers can't be starved by a steady stream of readers and ordering stays fair. Abort handling reuses the existing
acquireLockOrAbortpath.Risks
Scoped to tool-call scheduling within a single stream. The only behavioral change is that two-or-more concurrent built-in
exploretask calls may now overlap; all mutating tools retain identical serialized behavior. Because the exception excludes shared-checkout tasks and is keyed off a symbol (not a name), the shared-checkout and background-process-ID races that motivated #2906 remain closed.Generated with
mux• Model:anthropic:claude-opus-4-8• Thinking:xhigh• Cost:$29.64