feat(web): Docker container workspaces#97
Conversation
Bind a task to a Docker container from the remote-connect wizard, alongside SSH. Implemented with the Docker Go SDK. Backend - DockerExecutor (internal/tools/docker_exec.go): runs all file/command ops inside the container via `docker exec`, mirroring SSHExecutor (cat / base64 / test / mkdir). Shared daemon client (client.FromEnv → honors DOCKER_HOST). - Lifecycle (A1 + ref-count): a stopped container is started on connect and stopped again once the last task using it is torn down. Containers the user already had running are never stopped. A one-shot container that exits right after start is rejected with its logs instead of failing silently. - Generalized the engine/remote plumbing from *tools.SSHExecutor to a new tools.RemoteExecutor interface (SetRemote, IsRemote, ProjectLabel as a method; CloseRemote released on engine teardown). - Terminal into the container: ptyManager grew a backend interface; a TTY `docker exec` backend (bash→sh) powers the embedded terminal for container-bound engines (SSH/local stay on a local shell). - Wizard endpoints: GET /api/docker/containers, docker branch in /api/remote/connect, docker save-alias; docker_aliases config + switch_env support so the agent can switch into a container too. Frontend - RemoteConnectWizard: enabled the Docker method with a container picker; bind/reconnect handle docker:// workspaces. project store parses docker:// labels; types/api + composable updated. Tests - Daemon-gated integration tests (skip without Docker) create and clean up their own throwaway container and verify A1 start, exec, file roundtrip, stat, and ref-count auto-stop, plus one-shot rejection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Docker container workspaces to the remote-connect flow. A new ChangesDocker Container Workspace Support
Sequence Diagram(s)sequenceDiagram
participant User
participant RemoteConnectWizard
participant api
participant handleRemoteConnect
participant AcquireDockerContainer
participant handleRemoteBind
participant buildRemoteEngine
participant handleCreatePTY
participant dockerPTYBackend
User->>RemoteConnectWizard: select Docker method
RemoteConnectWizard->>api: dockerContainers()
api-->>RemoteConnectWizard: []DockerContainer
User->>RemoteConnectWizard: select container
RemoteConnectWizard->>handleRemoteConnect: POST /api/remote/connect {type:"docker", container}
handleRemoteConnect->>AcquireDockerContainer: containerRef
AcquireDockerContainer-->>handleRemoteConnect: *DockerExecutor (started+ref-counted)
handleRemoteConnect-->>RemoteConnectWizard: {id, container, pwd}
User->>RemoteConnectWizard: bindHere
RemoteConnectWizard->>handleRemoteBind: POST /api/remote/bind {id, pwd}
handleRemoteBind->>buildRemoteEngine: RemoteExecutor, pwd
handleRemoteBind-->>RemoteConnectWizard: {label:"docker://...", kind:"docker"}
RemoteConnectWizard->>api: remoteSaveDockerAlias(name, container, path)
User->>handleCreatePTY: open terminal
handleCreatePTY->>dockerPTYBackend: createDocker(containerID)
dockerPTYBackend-->>User: TTY stream via WebSocket
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… (CI lint) golangci-lint (unconvert) flagged string(s.State) — s.State is already a string. Use it directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/web/engine.go (1)
346-348: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClose the remote executor when engine creation fails.
If
s.newRemoteEngine(...)returns an error after the Docker/SSH executor has been acquired, this path returns without releasing it. For Docker, that can leave the container ref held and skip the expected auto-stop behavior.🧹 Proposed cleanup on factory failure
ec, err := s.newRemoteEngine(taskID, exec, remotePwd, modeStr) if err != nil { + if closeErr := exec.Close(); closeErr != nil { + config.Logger().Printf("[web] failed to close remote executor after engine creation error: %v", closeErr) + } return nil, err }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/web/engine.go` around lines 346 - 348, When s.newRemoteEngine() returns an error in the error handling block after the executor has been acquired, the remote executor (the exec parameter) needs to be properly closed to release the Docker/SSH resource. Add a cleanup call to close the executor before returning the error, ensuring the container reference is released and expected auto-stop behavior is not skipped.internal/web/pty.go (1)
246-264: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAvoid leaving a stale backend reader after the WebSocket disconnects.
When
conn.ReadMessage()fails, the handler breaks and waits on<-done>, but nothing closes or signalssess.backend. An idle shell/container exec can keep the backend read goroutine blocked indefinitely; on reconnect, the stale reader can also consume terminal output.🔌 Minimal fix if PTY sessions are WebSocket-scoped
msgType, msg, err := conn.ReadMessage() if err != nil { + _ = sess.backend.Close() break }If sessions are meant to survive browser refreshes, add explicit attach/detach ownership so the old reader stops before a new WebSocket can attach.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/web/pty.go` around lines 246 - 264, When conn.ReadMessage() fails and the loop breaks, the backend reader goroutine is left stale and blocked, consuming resources and potentially receiving output meant for new connections. After the break statement in the WebSocket read loop (where err != nil), add cleanup logic to close or signal sess.backend before waiting on the done channel. This ensures the backend reader goroutine unblocks and releases resources, preventing stale readers from interfering with new WebSocket connections. If PTY sessions are meant to survive reconnects, implement explicit attach/detach ownership tracking so that old readers are properly detached before new WebSocket clients can attach.
🧹 Nitpick comments (2)
internal/tools/docker_exec.go (1)
49-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
sync.RWMutexfor docker ref shared state.
dockerRefshas read-heavy access (e.g., Line 113), but the lock issync.Mutex. Switching tosync.RWMutexaligns this file with the tool-state concurrency guideline.As per coding guidelines:
internal/tools/**/*.gorequires “Usesync.RWMutexfor shared state in tools (see TodoStore, BackgroundManager).”♻️ Suggested adjustment
var ( - dockerRefMu sync.Mutex + dockerRefMu sync.RWMutex dockerRefs = map[string]int{} ) @@ - dockerRefMu.Lock() + dockerRefMu.RLock() alreadyOurs := dockerRefs[info.ID] > 0 - dockerRefMu.Unlock() + dockerRefMu.RUnlock()Also applies to: 112-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/tools/docker_exec.go` around lines 49 - 52, The dockerRefMu variable is declared as sync.Mutex but should be sync.RWMutex to support read-heavy access patterns (such as the reads at line 113). Change the type declaration of dockerRefMu from sync.Mutex to sync.RWMutex, then update all lock usage throughout the file to use RLock/RUnlock for read-only operations on dockerRefs and Lock/Unlock for write operations to maintain proper concurrency control.Source: Coding guidelines
internal/remote/docker.go (1)
27-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrap returned errors with context.
Both daemon errors are returned bare, so callers can't tell whether client init or
ContainerListfailed. Per the Go guideline, wrap with a context prefix.♻️ Proposed wrapping
cli, err := tools.DockerClient() if err != nil { - return nil, err + return nil, fmt.Errorf("docker client: %w", err) } summaries, err := cli.ContainerList(ctx, container.ListOptions{All: true}) if err != nil { - return nil, err + return nil, fmt.Errorf("list containers: %w", err) }(add
"fmt"to imports)As per coding guidelines: "Use
fmt.Errorf("tool_name: %w", err)for wrapped errors in non-tool code."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/remote/docker.go` around lines 27 - 34, Wrap both error returns with context using fmt.Errorf to help callers identify which operation failed. When the tools.DockerClient() call returns an error, wrap it with a descriptive context prefix using fmt.Errorf with the "%w" verb. Similarly, when the cli.ContainerList() call returns an error, wrap it with a context prefix using the same pattern. Make sure to add "fmt" to the imports if not already present, then replace both bare error returns with wrapped versions that include meaningful context about which operation failed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Line 16: The github.com/docker/docker dependency is deprecated and will not
receive security patches. Remove or replace the github.com/docker/docker
v28.5.2+incompatible line in go.mod and add the maintained replacements
github.com/moby/moby/client and github.com/moby/moby/api modules with
appropriate versions. Additionally, update all import statements throughout the
codebase that currently reference github.com/docker/docker packages to use the
corresponding github.com/moby/moby/client and github.com/moby/moby/api packages
instead.
In `@internal/tools/docker_exec.go`:
- Around line 299-314: The code discards errors from two critical operations:
the StdCopy stream copy and the ContainerExecInspect call. When the `copyDone`
channel receives a signal in the first select case, capture any error from the
underlying stdcopy.StdCopy operation and return it if it exists. Additionally,
after the ContainerExecInspect call, check if `ierr != nil` (not just the
condition on line 311) and return the error instead of silently succeeding. This
ensures that I/O failures and inspect operation errors are properly propagated
rather than marked as successful executions.
In `@internal/tools/switch_env.go`:
- Around line 74-98: The PTY backend created by ptyManager.createDocker() does
not hold its own reference to the Docker container, so when switch_env calls
closeIfRemote(prev), the container can be stopped even while a PTY session is
actively reading/writing to it. Fix this by having ptyManager.createDocker()
call AcquireDockerContainer() to acquire and hold a reference to the container
at session creation time, then store this reference in the dockerPTYBackend
structure. Update the PTY backend's Close() method to release this container
reference when the session terminates, ensuring the container remains alive for
the duration of the active terminal connection.
In `@internal/web/pty.go`:
- Around line 104-118: The createDocker function and related Docker operations
use context.Background() which doesn't support request cancellation, causing
goroutines to block indefinitely if the Docker daemon stalls. Add a
context.Context parameter as the first parameter to the createDocker function
signature, then replace all context.Background() calls within createDocker and
the related functions at lines 131-135 and 313-317 with the passed context or
bounded child contexts with appropriate timeouts. Finally, update all callers of
createDocker in internal/web/server.go to pass the request context instead of
creating fresh background contexts.
- Around line 252-256: The code currently treats any websocket TextMessage
starting with `{` as a control message and consumes it via the
handleControlMessage call with a continue statement. This causes invalid JSON or
non-resize JSON to be dropped instead of being sent to the shell. Fix this by
only consuming messages that are actually recognized control messages. Modify
the logic around the check for msg[0] == '{' to validate that the message is a
legitimate control message (such as a resize command with the proper JSON
structure) before calling handleControlMessage and using continue. If the
message is not a recognized control message, allow it to fall through to the
normal shell input handling instead of discarding it. Apply this fix to both
occurrences of this pattern mentioned in the comment.
In `@internal/web/remote.go`:
- Around line 385-404: The config read-modify-write section in this handler
incorrectly uses s.mu instead of the dedicated s.cfgMu mutex. Replace all
s.mu.Lock() and s.mu.Unlock() calls in this handler block (including the early
unlock after the s.cfg == nil check and the final unlock after SaveConfig) with
s.cfgMu.Lock() and s.cfgMu.Unlock() respectively. This ensures proper
serialization of concurrent config modifications across all handlers and
prevents races with handlers that correctly use s.cfgMu.
- Around line 167-192: The remoteConnRegistry only performs lazy cleanup via
sweepLocked() when add() is called in connectDocker, causing abandoned
connections and expired entries to linger indefinitely. More critically, on
server shutdown (around lines 338-345), remoteConns is never cleaned up, leaving
Docker container reference counts unrelemented. Add a closeAll() method to the
remoteConnRegistry type that iterates through and closes all pending connections
and their executors, then invoke this method during server shutdown before
calling srv.Shutdown(). Additionally, consider implementing a background
goroutine using a ticker that periodically invokes the sweep mechanism to
proactively enforce TTL cleanup rather than relying exclusively on lazy
evaluation triggered by add().
In `@web/src/components/RemoteConnectWizard.vue`:
- Around line 577-618: The Docker wizard template in RemoteConnectWizard.vue is
using four new i18n keys (wizard.dockerConnection, wizard.dockerDesc,
wizard.selectContainer, wizard.noContainers) that are missing translations in
the non-English locale files. Add these four keys with appropriate translations
to each of the locale files ja.ts, ko.ts, zh-Hans.ts, and zh-Hant.ts, following
the same structure and naming convention as other wizard-related translation
keys already present in those files.
In `@web/src/i18n/locales/en.ts`:
- Around line 441-449: The Docker wizard keys (container, dockerConnection,
dockerDesc, selectContainer, noContainers, refresh, running, stopped) have been
added to the English locale file but are missing from the non-English locale
files. Add these same keys with appropriate translations to the zh-Hans.ts,
zh-Hant.ts, ja.ts, and ko.ts locale files, maintaining the same object structure
and key names as in the English version. Ensure all keys are present and
properly translated in each non-English locale to avoid runtime translation
mismatches.
---
Outside diff comments:
In `@internal/web/engine.go`:
- Around line 346-348: When s.newRemoteEngine() returns an error in the error
handling block after the executor has been acquired, the remote executor (the
exec parameter) needs to be properly closed to release the Docker/SSH resource.
Add a cleanup call to close the executor before returning the error, ensuring
the container reference is released and expected auto-stop behavior is not
skipped.
In `@internal/web/pty.go`:
- Around line 246-264: When conn.ReadMessage() fails and the loop breaks, the
backend reader goroutine is left stale and blocked, consuming resources and
potentially receiving output meant for new connections. After the break
statement in the WebSocket read loop (where err != nil), add cleanup logic to
close or signal sess.backend before waiting on the done channel. This ensures
the backend reader goroutine unblocks and releases resources, preventing stale
readers from interfering with new WebSocket connections. If PTY sessions are
meant to survive reconnects, implement explicit attach/detach ownership tracking
so that old readers are properly detached before new WebSocket clients can
attach.
---
Nitpick comments:
In `@internal/remote/docker.go`:
- Around line 27-34: Wrap both error returns with context using fmt.Errorf to
help callers identify which operation failed. When the tools.DockerClient() call
returns an error, wrap it with a descriptive context prefix using fmt.Errorf
with the "%w" verb. Similarly, when the cli.ContainerList() call returns an
error, wrap it with a context prefix using the same pattern. Make sure to add
"fmt" to the imports if not already present, then replace both bare error
returns with wrapped versions that include meaningful context about which
operation failed.
In `@internal/tools/docker_exec.go`:
- Around line 49-52: The dockerRefMu variable is declared as sync.Mutex but
should be sync.RWMutex to support read-heavy access patterns (such as the reads
at line 113). Change the type declaration of dockerRefMu from sync.Mutex to
sync.RWMutex, then update all lock usage throughout the file to use
RLock/RUnlock for read-only operations on dockerRefs and Lock/Unlock for write
operations to maintain proper concurrency control.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97eadeba-7307-4aff-8ede-2bcd26a07367
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
CHANGELOG.mdgo.modinternal/command/web.gointernal/config/config.gointernal/remote/docker.gointernal/tools/docker_exec.gointernal/tools/docker_exec_test.gointernal/tools/env.gointernal/tools/switch_env.gointernal/web/engine.gointernal/web/pty.gointernal/web/remote.gointernal/web/server.goweb/src/components/RemoteConnectWizard.vueweb/src/composables/api.tsweb/src/i18n/locales/en.tsweb/src/stores/project.tsweb/src/types/api.ts
| github.com/cloudwego/eino-ext/libs/acl/langfuse v0.1.1 | ||
| github.com/coder/acp-go-sdk v0.13.5 | ||
| github.com/creack/pty v1.1.24 | ||
| github.com/docker/docker v28.5.2+incompatible |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major
🧩 Analysis chain
🌐 Web query:
For github.com/docker/docker, do GO-2026-4883, GO-2026-4887, GHSA-rg2x-37c3-w2rh, GHSA-vp62-88p7-qqf5, and GHSA-x86f-5xw2-fm2r impact the client package (github.com/docker/docker/client) or only daemon/server components? Also list the first patched module version for each advisory.
💡 Result:
The Go module github.com/docker/docker was deprecated as of Docker Engine v29 (November 2025) and will not receive further updates or patches [1][2][3]. Vulnerabilities affecting this module, including those listed, generally impact the daemon/server-side logic rather than the client package [4][5][6][7][8]. Users are advised to migrate to the maintained github.com/moby/moby/client and github.com/moby/moby/api modules [1][9][2]. Below is the impact analysis and status for each advisory: 1. GO-2026-4883 (CVE-2026-33997): Impacts daemon components (plugin privilege validation) [4][10]. No patched version exists for the deprecated github.com/docker/docker module [10]. 2. GO-2026-4887 (CVE-2026-34040): Impacts daemon components (AuthZ plugin bypass) [5]. No patched version exists for the deprecated github.com/docker/docker module [5]. 3. GHSA-rg2x-37c3-w2rh (CVE-2026-42306): Impacts daemon components (bind mount redirection during docker cp) [6][3][11]. No patched version exists for the deprecated github.com/docker/docker module. 4. GHSA-vp62-88p7-qqf5 (CVE-2026-41568): Impacts daemon components (symlink race during docker cp) [7]. No patched version exists for the deprecated github.com/docker/docker module. 5. GHSA-x86f-5xw2-fm2r (CVE-2026-41567): Impacts daemon components (arbitrary code execution via compressed archives) [8]. No patched version exists for the deprecated github.com/docker/docker module. Since github.com/docker/docker is deprecated and frozen, there are no first patched module versions for these advisories within that specific module path [1][9]. Mitigation requires migrating to the supported moby/moby modules [1][12].
Citations:
- 1: https://pkg.go.dev/github.com/docker/docker@v28.3.3+incompatible
- 2: https://pkg.go.dev/github.com/moby/moby/client@v0.4.0
- 3: https://docs.docker.com/engine/release-notes/29/
- 4: https://osv.dev/vulnerability/GO-2026-4883
- 5: https://osv.dev/vulnerability/GO-2026-4887
- 6: https://osv.dev/vulnerability/GHSA-rg2x-37c3-w2rh
- 7: GHSA-vp62-88p7-qqf5
- 8: https://osv.dev/vulnerability/GHSA-x86f-5xw2-fm2r
- 9: Migrate container client to moby/moby modules stacklok/toolhive#5420
- 10: https://pkg.go.dev/vuln/GO-2026-4883
- 11: https://advisories.gitlab.com/golang/github.com/moby/moby/CVE-2026-42306/
- 12: Replace deprecated github.com/docker/docker dependency flagged by govulncheck (GO-2026-4883) and (GO-2026-4887) elastic/beats#50227
github.com/docker/docker is deprecated with no available patches; migrate to github.com/moby/moby modules.
The listed advisories (GO-2026-4883, GO-2026-4887, GHSA-rg2x-37c3-w2rh, GHSA-vp62-88p7-qqf5, GHSA-x86f-5xw2-fm2r) impact only daemon/server-side components, not the client package. However, github.com/docker/docker was deprecated as of Docker Engine v29 (November 2025) and is frozen—no patched versions will be released. Replace with the maintained github.com/moby/moby/client and github.com/moby/moby/api modules.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Moby has an Off-by-one error in its plugin privilege validation in github.com/docker/docker
(GO-2026-4883)
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Moby has AuthZ plugin bypass when provided oversized request bodies in github.com/docker/docker
(GO-2026-4887)
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Moby has an Off-by-one error in its plugin privilege validation
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Docker: Race condition in docker cp allows bind mount redirection to host path
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Docker: Race condition in docker cp allows creation of arbitrary empty files on the host via symlink swap
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Moby has AuthZ plugin bypass when provided oversized request bodies
[HIGH] 16-16: github.com/docker/docker 28.5.2+incompatible: Docker: PUT /containers/{id}/archive executes container binary on the host
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 16, The github.com/docker/docker dependency is deprecated and
will not receive security patches. Remove or replace the
github.com/docker/docker v28.5.2+incompatible line in go.mod and add the
maintained replacements github.com/moby/moby/client and github.com/moby/moby/api
modules with appropriate versions. Additionally, update all import statements
throughout the codebase that currently reference github.com/docker/docker
packages to use the corresponding github.com/moby/moby/client and
github.com/moby/moby/api packages instead.
Source: Linters/SAST tools
| select { | ||
| case <-copyDone: | ||
| // stream drained: command finished | ||
| case <-ctx.Done(): | ||
| att.Close() // unblock the StdCopy goroutine | ||
| <-copyDone | ||
| return stdout.String(), stderr.String(), fmt.Errorf("command timed out or cancelled: %w", ctx.Err()) | ||
| } | ||
|
|
||
| // Use a fresh context for the inspect: the exec ctx may already be at its | ||
| // deadline, but the exec itself completed and its exit code is available. | ||
| inspect, ierr := d.cli.ContainerExecInspect(context.Background(), resp.ID) | ||
| if ierr == nil && inspect.ExitCode != 0 { | ||
| return stdout.String(), stderr.String(), fmt.Errorf("command exited with code %d", inspect.ExitCode) | ||
| } | ||
| return stdout.String(), stderr.String(), nil |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n internal/tools/docker_exec.go | head -350Repository: cnjack/jcode
Length of output: 14519
Capture and return errors from stream copy and inspect operations.
Line 300 discards the error from copyDone, and lines 310–314 silently succeed when ContainerExecInspect fails. This can mark failed or partial execs as successful, corrupting file and command operations.
Specifically:
- If
stdcopy.StdCopyreturns an error (I/O failure, stream truncation, etc.), it is discarded at line 300. - If
ContainerExecInspectfails (network error, server error), line 311's conditionierr == nil && ...evaluates false, skipping the error return and reaching line 314 which returns success.
Proposed fix
select {
- case <-copyDone:
+ case copyErr := <-copyDone:
+ if copyErr != nil {
+ return stdout.String(), stderr.String(), fmt.Errorf("docker exec stream copy: %w", copyErr)
+ }
// stream drained: command finished
case <-ctx.Done():
att.Close() // unblock the StdCopy goroutine
<-copyDone
return stdout.String(), stderr.String(), fmt.Errorf("command timed out or cancelled: %w", ctx.Err())
}
- inspect, ierr := d.cli.ContainerExecInspect(context.Background(), resp.ID)
- if ierr == nil && inspect.ExitCode != 0 {
+ inspect, ierr := d.cli.ContainerExecInspect(context.Background(), resp.ID)
+ if ierr != nil {
+ return stdout.String(), stderr.String(), fmt.Errorf("docker exec inspect: %w", ierr)
+ }
+ if inspect.ExitCode != 0 {
return stdout.String(), stderr.String(), fmt.Errorf("command exited with code %d", inspect.ExitCode)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| select { | |
| case <-copyDone: | |
| // stream drained: command finished | |
| case <-ctx.Done(): | |
| att.Close() // unblock the StdCopy goroutine | |
| <-copyDone | |
| return stdout.String(), stderr.String(), fmt.Errorf("command timed out or cancelled: %w", ctx.Err()) | |
| } | |
| // Use a fresh context for the inspect: the exec ctx may already be at its | |
| // deadline, but the exec itself completed and its exit code is available. | |
| inspect, ierr := d.cli.ContainerExecInspect(context.Background(), resp.ID) | |
| if ierr == nil && inspect.ExitCode != 0 { | |
| return stdout.String(), stderr.String(), fmt.Errorf("command exited with code %d", inspect.ExitCode) | |
| } | |
| return stdout.String(), stderr.String(), nil | |
| select { | |
| case copyErr := <-copyDone: | |
| if copyErr != nil { | |
| return stdout.String(), stderr.String(), fmt.Errorf("docker exec stream copy: %w", copyErr) | |
| } | |
| // stream drained: command finished | |
| case <-ctx.Done(): | |
| att.Close() // unblock the StdCopy goroutine | |
| <-copyDone | |
| return stdout.String(), stderr.String(), fmt.Errorf("command timed out or cancelled: %w", ctx.Err()) | |
| } | |
| // Use a fresh context for the inspect: the exec ctx may already be at its | |
| // deadline, but the exec itself completed and its exit code is available. | |
| inspect, ierr := d.cli.ContainerExecInspect(context.Background(), resp.ID) | |
| if ierr != nil { | |
| return stdout.String(), stderr.String(), fmt.Errorf("docker exec inspect: %w", ierr) | |
| } | |
| if inspect.ExitCode != 0 { | |
| return stdout.String(), stderr.String(), fmt.Errorf("command exited with code %d", inspect.ExitCode) | |
| } | |
| return stdout.String(), stderr.String(), nil |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tools/docker_exec.go` around lines 299 - 314, The code discards
errors from two critical operations: the StdCopy stream copy and the
ContainerExecInspect call. When the `copyDone` channel receives a signal in the
first select case, capture any error from the underlying stdcopy.StdCopy
operation and return it if it exists. Additionally, after the
ContainerExecInspect call, check if `ierr != nil` (not just the condition on
line 311) and return the error instead of silently succeeding. This ensures that
I/O failures and inspect operation errors are properly propagated rather than
marked as successful executions.
| // Docker alias? | ||
| for i := range cfg.DockerAliases { | ||
| if cfg.DockerAliases[i].Name != input.Target { | ||
| continue | ||
| } | ||
| da := cfg.DockerAliases[i] | ||
| dexec, derr := AcquireDockerContainer(ctx, da.Container) | ||
| if derr != nil { | ||
| if s.env.OnEnvChange != nil { | ||
| s.env.OnEnvChange("", false, fmt.Errorf("failed to connect to docker '%s': %v", input.Target, derr)) | ||
| } | ||
| return "", fmt.Errorf("failed to connect to docker '%s': %v", input.Target, derr) | ||
| } | ||
| path := da.Path | ||
| if path == "" { | ||
| path = "/" | ||
| } | ||
| s.env.SetRemote(dexec, path) | ||
| closeIfRemote(prev) | ||
| label := dexec.Label() | ||
| if s.env.OnEnvChange != nil { | ||
| s.env.OnEnvChange(label, false, nil) | ||
| } | ||
| return fmt.Sprintf("Switched to '%s' (%s: %s).", input.Target, label, path), nil | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Does any PTY/terminal path acquire its own docker ref, or does it rely on the engine's executor?
rg -nP -C3 '(dockerAcquireRef|AcquireDockerContainer|dockerReleaseRef|startedByUs)'
# Inspect dockerPTYBackend lifecycle and how the container ref is kept alive during a terminal session.
fd -e go pty.go --exec rg -nP -C4 '(dockerPTYBackend|AcquireDockerContainer|Close)'Repository: cnjack/jcode
Length of output: 342
🏁 Script executed:
cat /tmp/coderabbit-shell-logs/shell-output-5QNPrRRepository: cnjack/jcode
Length of output: 342
🏁 Script executed:
# Find and examine the PTY-related files
find . -name "*pty*" -type f -name "*.go"Repository: cnjack/jcode
Length of output: 172
🏁 Script executed:
# Search for dockerReleaseRef, dockerAcquireRef, and startedByUs usage
rg "dockerReleaseRef|dockerAcquireRef|startedByUs" --type go -B2 -A2Repository: cnjack/jcode
Length of output: 150
🏁 Script executed:
# Examine DockerExecutor.Close() implementation
rg "func.*DockerExecutor.*Close|type DockerExecutor" --type go -A 20Repository: cnjack/jcode
Length of output: 150
🏁 Script executed:
# Check the log files
tail -100 /tmp/coderabbit-shell-logs/shell-output-UWeX4BRepository: cnjack/jcode
Length of output: 150
🏁 Script executed:
# List Go files in docker-related areas
find . -name "*.go" -path "*/docker*" -o -name "*.go" -path "*/internal/*" | grep -i docker | head -20Repository: cnjack/jcode
Length of output: 247
🏁 Script executed:
# Read the PTY file to understand lifecycle
wc -l ./internal/web/pty.goRepository: cnjack/jcode
Length of output: 176
🏁 Script executed:
# Read switch_env.go to see full context
wc -l ./internal/tools/switch_env.goRepository: cnjack/jcode
Length of output: 185
🏁 Script executed:
# Read the switch_env.go file
cat -n ./internal/tools/switch_env.goRepository: cnjack/jcode
Length of output: 4985
🏁 Script executed:
# Read the pty.go file to see PTY backend implementation
cat -n ./internal/web/pty.goRepository: cnjack/jcode
Length of output: 10946
🏁 Script executed:
# Read docker.go to understand ref counting
cat -n ./internal/remote/docker.go | head -100Repository: cnjack/jcode
Length of output: 2183
🏁 Script executed:
# Find Docker executor implementation
fd -e go docker.go docker_exec.go --exec grep -l "DockerExecutor" {} \;Repository: cnjack/jcode
Length of output: 252
🏁 Script executed:
# Read the DockerExecutor type and Close method
rg "type DockerExecutor" -A 30 internal/tools/Repository: cnjack/jcode
Length of output: 2219
🏁 Script executed:
# Search for AcquireDockerContainer and dockerReleaseRef
rg "func AcquireDockerContainer|func.*dockerReleaseRef|startedByUs" -B 2 -A 10 internal/tools/Repository: cnjack/jcode
Length of output: 7433
🏁 Script executed:
# Check docker_exec.go for the full Docker executor implementation
cat -n ./internal/tools/docker_exec.go | head -150Repository: cnjack/jcode
Length of output: 6358
Docker ref-count drop can terminate active PTY sessions: PTY backend should acquire its own container reference.
The acquire → SetRemote → closeIfRemote(prev) ordering is correct for typical switching. However, there's a critical gap: when switch_env closes prev after a successful acquire, DockerExecutor.Close() calls dockerReleaseRef(containerID, true) whenever startedByUs is set. If this is the last reference, the container stops immediately—even if a web PTY/terminal session created via ptyManager.createDocker() is actively reading/writing to the container via docker exec.
The PTY backend (dockerPTYBackend in internal/web/pty.go lines 304–322) does not acquire its own Docker container reference. Its Close() method only calls att.Close() on the hijacked connection and never touches container lifetime. The PTY's cleanup goroutine (lines 128–138) similarly expects the container to remain running and only polls the exec process state.
When an agent switches away from a container that started a PTY session, the last ref release stops the container, terminating the active docker exec session out from under the connected client.
The fix: ptyManager.createDocker() should call AcquireDockerContainer() to hold a reference on containers it creates PTY backends for, and release it when the session closes. This prevents the container from being stopped while any terminal remains attached.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/tools/switch_env.go` around lines 74 - 98, The PTY backend created
by ptyManager.createDocker() does not hold its own reference to the Docker
container, so when switch_env calls closeIfRemote(prev), the container can be
stopped even while a PTY session is actively reading/writing to it. Fix this by
having ptyManager.createDocker() call AcquireDockerContainer() to acquire and
hold a reference to the container at session creation time, then store this
reference in the dockerPTYBackend structure. Update the PTY backend's Close()
method to release this container reference when the session terminates, ensuring
the container remains alive for the duration of the active terminal connection.
| func (m *ptyManager) createDocker(cli *client.Client, containerID, workDir, ownerID string) (string, error) { | ||
| ctx := context.Background() | ||
| resp, err := cli.ContainerExecCreate(ctx, containerID, container.ExecOptions{ | ||
| Cmd: []string{"sh", "-c", "exec bash 2>/dev/null || exec sh"}, | ||
| WorkingDir: workDir, | ||
| Env: []string{"TERM=xterm-256color"}, | ||
| Tty: true, | ||
| AttachStdin: true, | ||
| AttachStdout: true, | ||
| AttachStderr: true, | ||
| }) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| att, err := cli.ContainerExecAttach(ctx, resp.ID, container.ExecAttachOptions{Tty: true}) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Thread request cancellation into Docker PTY operations.
createDocker, the exec-inspect poll, and resize calls all use context.Background(), so Docker daemon stalls can outlive the HTTP/WebSocket request and leave goroutines blocked. Pass a caller context into createDocker/backend operations and use bounded child contexts for long-running Docker calls. As per coding guidelines, Use context.Context as the first parameter and thread cancellation properly.
⏱️ Sketch of the direction
-func (m *ptyManager) createDocker(cli *client.Client, containerID, workDir, ownerID string) (string, error) {
- ctx := context.Background()
+func (m *ptyManager) createDocker(ctx context.Context, cli *client.Client, containerID, workDir, ownerID string) (string, error) {
+ ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
+ defer cancel()
resp, err := cli.ContainerExecCreate(ctx, containerID, container.ExecOptions{And update the caller in internal/web/server.go:
- id, err = s.ptyMgr.createDocker(cli, dockerExec.ContainerID(), pwd, owner)
+ id, err = s.ptyMgr.createDocker(r.Context(), cli, dockerExec.ContainerID(), pwd, owner)Also applies to: 131-135, 313-317
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/pty.go` around lines 104 - 118, The createDocker function and
related Docker operations use context.Background() which doesn't support request
cancellation, causing goroutines to block indefinitely if the Docker daemon
stalls. Add a context.Context parameter as the first parameter to the
createDocker function signature, then replace all context.Background() calls
within createDocker and the related functions at lines 131-135 and 313-317 with
the passed context or bounded child contexts with appropriate timeouts. Finally,
update all callers of createDocker in internal/web/server.go to pass the request
context instead of creating fresh background contexts.
Source: Coding guidelines
| if msgType == websocket.TextMessage { | ||
| // Check for resize command | ||
| // Check for resize command: {"type":"resize","cols":80,"rows":24} | ||
| if len(msg) > 0 && msg[0] == '{' { | ||
| m.handleControlMessage(sess, msg) | ||
| continue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t swallow terminal input that starts with {.
Any text frame beginning with { is treated as a control message; invalid JSON or non-resize JSON is then dropped instead of being written to the shell. This breaks normal shell input such as grouped commands pasted into the terminal.
⌨️ Proposed fix: only consume recognized control messages
- m.handleControlMessage(sess, msg)
- continue
+ if m.handleControlMessage(sess, msg) {
+ continue
+ }-func (m *ptyManager) handleControlMessage(sess *ptySession, msg []byte) {
+func (m *ptyManager) handleControlMessage(sess *ptySession, msg []byte) bool {
// Parse JSON control message: {"type":"resize","cols":80,"rows":24}
var ctrl struct {
Type string `json:"type"`
Cols uint16 `json:"cols"`
Rows uint16 `json:"rows"`
}
if err := json.Unmarshal(msg, &ctrl); err != nil {
- return
+ return false
}
if ctrl.Type == "resize" && ctrl.Cols > 0 && ctrl.Rows > 0 {
_ = sess.backend.Resize(ctrl.Cols, ctrl.Rows)
+ return true
}
+ return false
}Also applies to: 267-279
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/pty.go` around lines 252 - 256, The code currently treats any
websocket TextMessage starting with `{` as a control message and consumes it via
the handleControlMessage call with a continue statement. This causes invalid
JSON or non-resize JSON to be dropped instead of being sent to the shell. Fix
this by only consuming messages that are actually recognized control messages.
Modify the logic around the check for msg[0] == '{' to validate that the message
is a legitimate control message (such as a resize command with the proper JSON
structure) before calling handleControlMessage and using continue. If the
message is not a recognized control message, allow it to fall through to the
normal shell input handling instead of discarding it. Apply this fix to both
occurrences of this pattern mentioned in the comment.
| // connectDocker binds (starting if stopped) the named container and parks it in | ||
| // the pending registry, mirroring the SSH connect flow. | ||
| func (s *Server) connectDocker(w http.ResponseWriter, r *http.Request, containerRef string) { | ||
| if containerRef == "" { | ||
| writeJSON(w, http.StatusBadRequest, map[string]string{"error": "container is required"}) | ||
| return | ||
| } | ||
| exec, err := remote.ConnectDocker(r.Context(), containerRef) | ||
| if err != nil { | ||
| writeJSON(w, http.StatusBadGateway, map[string]string{"error": err.Error()}) | ||
| return | ||
| } | ||
| remotePwd := remote.DiscoverPwd(r.Context(), exec, "/") | ||
| id := s.remoteConns.add(&pendingConn{ | ||
| exec: exec, | ||
| kind: "docker", | ||
| container: exec.Name(), | ||
| createdAt: time.Now(), | ||
| }) | ||
| writeJSON(w, http.StatusOK, map[string]any{ | ||
| "connection_id": id, | ||
| "remote_pwd": remotePwd, | ||
| "platform": exec.Platform(), | ||
| "container": exec.Name(), | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the pending registry's expiry/cleanup path to confirm it closes executors.
rg -nP -C8 'remoteConnRegistry|func .*remoteConns|createdAt|TTL|expire|sweep|reap' internal/web
ast-grep --pattern '$_.exec.Close()'Repository: cnjack/jcode
Length of output: 9792
🏁 Script executed:
# Check for background cleanup, goroutines, or periodic sweeps related to remoteConns
rg -n "go\s+.*sweep|ticker|time\.NewTicker|time\.After" internal/web --type go
rg -n "newServer|func.*Server\)" internal/web --type go -A10 | head -60Repository: cnjack/jcode
Length of output: 4205
🏁 Script executed:
# Look for Server initialization and any background cleanup goroutines
rg -n "func newServer|func NewServer" internal/web --type go -A30
rg -n "func.*Server.*Close|Shutdown" internal/web --type go -B2 -A10Repository: cnjack/jcode
Length of output: 3455
🏁 Script executed:
# Get the complete NewServer function to check for background cleanup goroutines
sed -n '154,250p' internal/web/server.goRepository: cnjack/jcode
Length of output: 3638
🏁 Script executed:
# Continue reading the Start function to check for background cleanup
sed -n '250,350p' internal/web/server.goRepository: cnjack/jcode
Length of output: 5095
🏁 Script executed:
# Check what happens to remoteConns during server shutdown or context cancellation
rg -n "remoteConns" internal/web/server.go -B2 -A2
rg -n "CloseAllEngines\|teardown" internal/web/server.goRepository: cnjack/jcode
Length of output: 717
🏁 Script executed:
# Check if remoteConns is ever closed or cleaned up during shutdown
rg -n "remoteConns\|Close\|Shutdown" internal/web/server.go | grep -A5 -B5 "remoteConns"
# Also check if there's a method to close all pending connections
rg -n "func.*remoteConnRegistry.*Close\|func.*pendingConn.*Close" internal/webRepository: cnjack/jcode
Length of output: 150
🏁 Script executed:
# Check the complete remote.go file to see all methods on remoteConnRegistry
cat -n internal/web/remote.go | sed -n '38,100p'
# Also check handleRemoteCancel to see if it properly closes connections
rg -n "handleRemoteCancel" internal/web --type go -A15Repository: cnjack/jcode
Length of output: 5054
🏁 Script executed:
# Check the shutdown path more carefully - look at the graceful shutdown handler
sed -n '330,360p' internal/web/server.goRepository: cnjack/jcode
Length of output: 1041
Add server-shutdown cleanup and optional background sweep for pending connections.
The registry's sweepLocked() method does close executors on TTL expiry (lines 85–95), as expected. However, the mechanism is lazy: it only runs when add() is called. If the user abandons a Docker connection and creates no new connections afterward, the expired pending entry lingers indefinitely. More critically, on server shutdown (lines 338–345), remoteConns is never swept, so any unbound pending connections remain open. For Docker containers started via AcquireDockerContainer, this means the ref-count is never decremented and auto-started containers continue running indefinitely.
Either:
- Add a
closeAll()method toremoteConnRegistryand call it during server shutdown beforesrv.Shutdown(), or - Start a background sweep goroutine (e.g., with a ticker) to enforce TTL proactively, or
- Both for redundancy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/remote.go` around lines 167 - 192, The remoteConnRegistry only
performs lazy cleanup via sweepLocked() when add() is called in connectDocker,
causing abandoned connections and expired entries to linger indefinitely. More
critically, on server shutdown (around lines 338-345), remoteConns is never
cleaned up, leaving Docker container reference counts unrelemented. Add a
closeAll() method to the remoteConnRegistry type that iterates through and
closes all pending connections and their executors, then invoke this method
during server shutdown before calling srv.Shutdown(). Additionally, consider
implementing a background goroutine using a ticker that periodically invokes the
sweep mechanism to proactively enforce TTL cleanup rather than relying
exclusively on lazy evaluation triggered by add().
| s.mu.Lock() | ||
| if s.cfg == nil { | ||
| s.mu.Unlock() | ||
| writeJSON(w, http.StatusInternalServerError, map[string]string{"error": "config unavailable"}) | ||
| return | ||
| } | ||
| updated := false | ||
| for i := range s.cfg.DockerAliases { | ||
| if s.cfg.DockerAliases[i].Name == req.Name { | ||
| s.cfg.DockerAliases[i].Container = req.Container | ||
| s.cfg.DockerAliases[i].Path = req.Path | ||
| updated = true | ||
| break | ||
| } | ||
| } | ||
| if !updated { | ||
| s.cfg.DockerAliases = append(s.cfg.DockerAliases, config.DockerAlias{Name: req.Name, Container: req.Container, Path: req.Path}) | ||
| } | ||
| err := config.SaveConfig(s.cfg) | ||
| s.mu.Unlock() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all SaveConfig call sites in web handlers and the lock guarding them.
rg -nP -C6 '\bSaveConfig\s*\(' internal/web
echo '--- cfgMu usages ---'
rg -nP '\bcfgMu\b' internal/web
echo '--- s.mu usages around cfg mutation ---'
rg -nP -B2 -A2 's\.cfg\b' internal/webRepository: cnjack/jcode
Length of output: 21495
Lock inconsistency — config read-modify-write uses s.mu, but other handlers use s.cfgMu
The handler guards its config read-modify-write and SaveConfig under s.mu, but the dedicated cfgMu mutex is explicitly documented for exactly this purpose ("serializes read-modify-write SaveConfig from concurrent handlers"). The inconsistency is widespread: most config-mutating handlers (handleCreateMCP, handleUpdateMCP, handleDeleteMCP, handleToggleMCP, handleMCPLogin, handleRemoteSaveSSHAlias, handleChannelBLEStatus) use s.mu, but others (handleApprovalModeSwitch at line 2422, handleToggleSkill at line 2656) correctly use s.cfgMu. This allows races: a config writer under s.mu does not serialize against one under s.cfgMu, creating lost-update and data-race hazards during concurrent config persists.
Align all config-mutating handlers to use s.cfgMu (not s.mu) for read-modify-write + SaveConfig, starting with this handler.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/remote.go` around lines 385 - 404, The config read-modify-write
section in this handler incorrectly uses s.mu instead of the dedicated s.cfgMu
mutex. Replace all s.mu.Lock() and s.mu.Unlock() calls in this handler block
(including the early unlock after the s.cfg == nil check and the final unlock
after SaveConfig) with s.cfgMu.Lock() and s.cfgMu.Unlock() respectively. This
ensures proper serialization of concurrent config modifications across all
handlers and prevents races with handlers that correctly use s.cfgMu.
| <!-- Step 2b: docker container picker --> | ||
| <template v-else-if="step === 'docker' || (step === 'connecting' && method === 'docker')"> | ||
| <h3 class="rcw-h">{{ t('wizard.dockerConnection') }}</h3> | ||
| <p class="rcw-sub">{{ t('wizard.dockerDesc') }}</p> | ||
|
|
||
| <div v-if="error" class="rcw-error">{{ error }}</div> | ||
|
|
||
| <div v-if="step === 'connecting'" class="rcw-hint"> | ||
| <ArrowPathIcon class="w-3.5 h-3.5 spin" /> {{ t('wizard.connecting') }} | ||
| </div> | ||
| <template v-else> | ||
| <div class="rcw-dirbar"> | ||
| <span class="rcw-dir-path">{{ t('wizard.selectContainer') }}</span> | ||
| <button class="rcw-back" style="margin-left:auto" :title="t('wizard.refresh')" @click="loadContainers"> | ||
| <ArrowPathIcon class="w-3.5 h-3.5" :class="{ spin: containersLoading }" /> | ||
| </button> | ||
| </div> | ||
|
|
||
| <div class="rcw-dirlist"> | ||
| <div v-if="containersLoading" class="rcw-hint"><ArrowPathIcon class="w-3.5 h-3.5 spin" /> {{ t('wizard.loading') }}</div> | ||
| <template v-else> | ||
| <button | ||
| v-for="c in containers" | ||
| :key="c.id" | ||
| class="rcw-dir-row" | ||
| @click="connectDocker(c.name || c.id)" | ||
| > | ||
| <CubeIcon class="w-3.5 h-3.5" /> | ||
| <span class="rcw-ctr-name">{{ c.name || c.id.slice(0, 12) }}</span> | ||
| <span class="rcw-ctr-img">{{ c.image }}</span> | ||
| <span class="rcw-ctr-state" :class="{ on: c.running }">{{ c.running ? t('wizard.running') : t('wizard.stopped') }}</span> | ||
| </button> | ||
| <div v-if="containers.length === 0" class="rcw-hint">{{ t('wizard.noContainers') }}</div> | ||
| </template> | ||
| </div> | ||
|
|
||
| <div class="rcw-foot"> | ||
| <button class="rcw-ghost" @click="step = 'method'">{{ t('common.back') }}</button> | ||
| <span /> | ||
| </div> | ||
| </template> | ||
| </template> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm each new wizard key resolves in every locale file.
keys=(docker dockerConnection dockerDesc selectContainer running stopped noContainers container refresh loading connecting next)
fd -e ts . web/src/i18n/locales | while read -r f; do
echo "== $f =="
for k in "${keys[@]}"; do
rg -nP "(^|\s)$k\s*:" "$f" >/dev/null || echo "MISSING wizard.$k"
done
doneRepository: cnjack/jcode
Length of output: 893
Add missing i18n keys to all non-English locale files.
The new Docker wizard keys are missing translations in Japanese, Korean, Simplified Chinese, and Traditional Chinese. Japanese, Korean, and Simplified/Traditional Chinese users will see untranslated key strings instead of localized text.
Missing keys in ja.ts, ko.ts, zh-Hans.ts, and zh-Hant.ts:
wizard.dockerConnectionwizard.dockerDescwizard.selectContainerwizard.noContainers
Add these keys with appropriate translations to all non-English locale files before merge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/RemoteConnectWizard.vue` around lines 577 - 618, The
Docker wizard template in RemoteConnectWizard.vue is using four new i18n keys
(wizard.dockerConnection, wizard.dockerDesc, wizard.selectContainer,
wizard.noContainers) that are missing translations in the non-English locale
files. Add these four keys with appropriate translations to each of the locale
files ja.ts, ko.ts, zh-Hans.ts, and zh-Hant.ts, following the same structure and
naming convention as other wizard-related translation keys already present in
those files.
| container: 'Container', | ||
| comingSoon: 'Coming soon', | ||
| dockerConnection: 'Docker container', | ||
| dockerDesc: 'Pick a container to use as the workspace. A stopped container is started for you, and stopped again when no task is using it.', | ||
| selectContainer: 'Select a container', | ||
| noContainers: 'No containers found on this Docker host.', | ||
| refresh: 'Refresh', | ||
| running: 'running', | ||
| stopped: 'stopped', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare locale key paths across web/src/i18n/locales/*.ts without modifying files.
# Expect: No missing keys reported for the newly added wizard Docker strings.
fd -e ts . web/src/i18n/locales -x printf '%s\n' {}
python - <<'PY'
from pathlib import Path
import re
locale_dir = Path("web/src/i18n/locales")
files = sorted(locale_dir.glob("*.ts"))
if not files:
raise SystemExit("No locale files found")
target_keys = {
"container",
"dockerConnection",
"dockerDesc",
"selectContainer",
"noContainers",
"refresh",
"running",
"stopped",
}
for path in files:
text = path.read_text(encoding="utf-8")
missing = []
wizard = re.search(r"\bwizard\s*:\s*\{(?P<body>.*?)\n\s*\},\n\n\s*sidebar\s*:", text, re.S)
body = wizard.group("body") if wizard else ""
for key in sorted(target_keys):
if not re.search(rf"\b{re.escape(key)}\s*:", body):
missing.append(key)
if missing:
print(f"{path}: missing {', '.join(missing)}")
PYRepository: cnjack/jcode
Length of output: 886
Add missing Docker wizard keys to non-English locales.
The new Docker wizard keys (container, dockerConnection, dockerDesc, selectContainer, noContainers, refresh, running, stopped) are missing from all non-English locale files. Add these keys to zh-Hans.ts, zh-Hant.ts, ja.ts, and ko.ts to match the English source.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/i18n/locales/en.ts` around lines 441 - 449, The Docker wizard keys
(container, dockerConnection, dockerDesc, selectContainer, noContainers,
refresh, running, stopped) have been added to the English locale file but are
missing from the non-English locale files. Add these same keys with appropriate
translations to the zh-Hans.ts, zh-Hant.ts, ja.ts, and ko.ts locale files,
maintaining the same object structure and key names as in the English version.
Ensure all keys are present and properly translated in each non-English locale
to avoid runtime translation mismatches.
1 participant
What
Adds Docker container as a remote workspace target in the web UI, alongside SSH. A task can be bound to a container; the agent's file/command tools and the embedded terminal then operate inside that container. Built with the Docker Go SDK.
This is the implementation of the design we converged on (research → design → decisions A1 / B / C / D / E).
Design decisions implemented
github.com/docker/docker/client,FromEnv→ honorsDOCKER_HOST, with API-version negotiation).docker exec(cat/base64/test/mkdir), identical in spirit to the SSH executor —catfollows symlinks naturally.switch_env(E): the agent can switch into a saved Docker alias (docker_aliasesin config); switching away releases the previous target's hold.docker exec, bash→sh). SSH/local engines keep a local shell (unchanged).Key changes
Backend
internal/tools/docker_exec.go—DockerExecutor+ shared client + ref-count lifecycle + A1AcquireDockerContainer.internal/tools/env.go— newRemoteExecutorinterface;SetRemote/CloseRemote;IsRemotenow means "not local";ProjectLabelis a method.*tools.SSHExecutor→tools.RemoteExecutoracrossserver.go,engine.go,command/web.go,web/remote.go. Engine teardown releases the remote hold.internal/web/pty.go—ptyBackendinterface with local-PTY and docker-exec-TTY backends;handleCreatePTYroutes container-bound engines to the docker terminal.internal/remote/docker.go—ListContainers/ConnectDocker.GET /api/docker/containers, docker branch inPOST /api/remote/connect,POST /api/remote/save-docker-alias.config.DockerAlias.Frontend
RemoteConnectWizard.vue— Docker method enabled with a container picker; bind/reconnect handledocker://workspaces.docker://<container>/<path>labels; types + api composable updated.Testing
go build ./...(default and-tags jcode_headless),go vet ./...,gofmt, fullgo test ./...— all clean.vue-tsctype-check,vite build,oxlint,eslint— all clean.internal/tools/docker_exec_test.go) skip cleanly without Docker; against a real daemon they create and clean up their own throwaway container and verify A1 start, exec, write/read roundtrip, stat, ProjectLabel, and ref-count auto-stop — plus the one-shot-rejection path. Verified passing locally against the daemon.Known limitations (consistent with SSH today)
client.FromEnvdoes not readdocker context; users with a non-default socket (e.g. OrbStack) needDOCKER_HOSTset.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes