🤖 fix: Docker sub-agent container double-creation and sidebar latency - #1506
Conversation
Two related fixes for Docker runtime sub-agents: 1. Container double-creation: When creating a sub-agent workspace in Docker runtime, both forkWorkspace() and initWorkspace() were trying to create the container, causing 'Container already exists' errors. Now initWorkspace() detects when the container is already running (from a successful fork) and skips directly to running the init hook. 2. Sidebar latency: Sub-agent workspaces weren't appearing in the sidebar until container creation completed, even though metadata was emitted earlier. The race condition was in buildSortedWorkspacesByProject() which only included workspaces from the (stale) projects config, ignoring workspaces in workspaceMetadata that hadn't been picked up by refreshProjects() yet. Changes: - DockerRuntime.ts: Add skip detection for already-running containers, extract setupContainerAndSyncProject() helper, preserve forked containers on init hook failure - workspaceFiltering.ts: Include all metadata workspaces not yet in config (not just status='creating' which was never actually set) - runtime.test.ts: Add tests for skip-path and container preservation
![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: 6484debb12
ℹ️ 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".
- initHook.ts: Throw error when init hook exits with non-zero code,
so initWorkspace() properly returns { success: false } instead of
silently succeeding
- DockerRuntime.ts: Before skipping container setup for a running
container, validate that it has an initialized git repo. If not
(e.g., crash after docker run but before sync), remove and recreate
instead of operating on an uninitialized environment
|
@codex review Addressed the container validation concern: before skipping setup for a running container, we now verify that Also fixed init hook failure propagation— |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13c218a34e
ℹ️ 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 both review comments: P2 - Branch validation: Refactored container validation into P3 - Double logComplete: Added |
- Add ContainerCheckResult discriminated union for validation outcomes - Extract nested validation logic into flat early-return helper method - Branch validation (P2): check current branch matches branchName before skipping - Double logComplete (P3): use InitHookError class with instanceof check Addresses Codex review comments on branch validation and init hook logging.
4728991 to
66c3a62
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4728991c95
ℹ️ 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".
Tests now initialize containers with branchName instead of 'main' to match the branch validation in checkExistingContainer().
|
@codex review Fixed: Both tests now initialize the container git repo on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efff060e38
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Init hook failures should be logged but not prevent workspace usage, per docs/hooks/init.mdx. Reverting InitHookError approach - the original design was correct (log exit code, don't throw). - Remove InitHookError class from initHook.ts - Remove instanceof checks from Docker/SSH runtime catch blocks - Update test to expect success=true when hook fails (non-fatal)
|
@codex review Fixed P1: Restored non-fatal init hook semantics per
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c54ad3bad7
ℹ️ 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".
Extract setupCredentials() to copy gitconfig and run gh auth setup-git. Called in skip path so forked containers get credentials before init hook. - Add setupCredentials() method for gitconfig + gh auth - Call in skip path for reused forked containers - Refactor setupContainerAndSyncProject to use the new method
|
@codex review Fixed P2: Extracted
|
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ 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
Two related fixes for Docker runtime sub-agents, plus refactoring and Codex review fixes.
1. Container double-creation fix
When creating a sub-agent workspace in Docker runtime, both
forkWorkspace()andinitWorkspace()were trying to create the container, causing "Container already exists and is running" errors.Root cause:
TaskService.create()callsforkWorkspace()(which creates the container), then callsinitWorkspace()(which also tried to create a container).Fix:
initWorkspace()now detects when the container is already running (from a successful fork) and skips directly to running the init hook. AddedcheckExistingContainer()method that validates:.gitdirectory exists)2. Sidebar latency fix
Sub-agent workspaces weren't appearing in the sidebar until container creation completed (~5-15s delay).
Root cause:
buildSortedWorkspacesByProject()had a race condition where metadata events arrived before config refresh completed, and the second pass only caught workspaces withstatus === "creating"(never set for sub-agents).Fix: Remove the dead
status === "creating"check - include any workspace in metadata not yet in config.3. Credential setup for forked containers (Codex P2)
When reusing forked containers via the skip path,
setupContainerAndSyncProject()was bypassed, which meant~/.gitconfigandgh auth setup-gitwere never run.Fix: Extract
setupCredentials()method and call it in the skip path before running the init hook.4. Non-fatal init hook semantics (Codex P1)
Init hook failures should be logged but not prevent workspace usage per
docs/hooks/init.mdx.Fix:
runInitHookOnRuntime()logs the exit code but doesn't throw, so hook failures don't trigger container cleanup.Changes
src/node/runtime/DockerRuntime.tscheckExistingContainer(),setupCredentials(),setupContainerAndSyncProject()helpers; preserve forked containers on failuresrc/node/runtime/initHook.tssrc/browser/utils/ui/workspaceFiltering.tstests/runtime/runtime.test.tsTesting
make devGenerated with
mux• Model:anthropic:claude-opus-4-5• Thinking:high