fix(sandbox): read QM_CORE_CONTAINER where the local sandbox can use it - #934
Open
importantitemzl-tech wants to merge 1 commit into
Open
Conversation
The docker target sets QM_CORE_CONTAINER on core (cli/src/backends/docker.ts) for one purpose: telling the local sandbox backend that core runs in a container. createLocalSandbox uses it to connect core to the sandbox's network and address the exec daemon as http://<container>:8080; without it the sandbox publishes a port and core addresses http://127.0.0.1:<port>, which inside a containerized core is core itself. config.ts read the variable in awsSandboxEnv() instead of localSandboxEnv(). AwsSandboxEnv does not declare coreContainer and aws-sandbox.ts never reads one, so the assignment was dead where it was and missing where it was needed — the shape of a block that moved and left a line behind. The effect on a docker deployment using the local sandbox is that every tool call fails: skills | tree_materialize_failed | local sandbox qm-sbx-...: exec daemon never became reachable: fetch failed Observed on a docker deployment: the same published port answers 404 from the host and connection-refused from inside core, core is on none of the sandbox networks, and it cannot resolve the sandbox container name. The agent retries, gives up, and replies that its execution environment is unreachable. It survived because coverage sits on both sides of the gap and not across it: test/local-sandbox.test.ts passes coreContainer straight into the constructor, cli/test/auth-broker.test.ts asserts the variable is in the docker service env, and the docker e2e test asserts it is present on the container — none assert that anything consumes it, and none run an agent turn that uses a tool. The same code path is also correct under `qm dev`, where core is a host process and 127.0.0.1:<published port> really is the sandbox. The added config test crosses that gap: it fails with the line in its old place and passes with it moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What breaks
The
dockertarget setsQM_CORE_CONTAINERon core (cli/src/backends/docker.ts:358) for exactly one purpose: telling the local sandbox backend that core runs in a container.createLocalSandboxuses it to connect core to the sandbox's network and address the exec daemon ashttp://<container>:8080. Without it, the sandbox is started with a published port and core addresseshttp://127.0.0.1:<port>— which, inside a containerized core, is core itself.config.tsread the variable inawsSandboxEnv()instead oflocalSandboxEnv().AwsSandboxEnvdoes not declarecoreContainerandaws-sandbox.tsnever reads one, so the assignment was dead where it sat and missing where it was needed.What it looks like
Every tool call on a docker deployment using
sandbox.backend: "local"fails:The agent retries, gives up, and tells the user its execution environment is unreachable. Chat works; anything needing a tool does not.
Observed on such a deployment:
GET http://127.0.0.1:<published port>/from the host404— the daemon is alivegetent hosts <sandbox container>inside coreWhy it survived
Coverage sits on both sides of the gap and not across it:
test/local-sandbox.test.tspassescoreContainerstraight into the constructor, so container mode itself is well covered;cli/test/auth-broker.test.tsasserts the variable is in the docker service env;cli/test/e2e/docker-lifecycle.e2e.test.tsasserts it is present on the container, and stops there.None assert that anything consumes it, and none run an agent turn that uses a tool. The same code path is also correct under
qm dev, where core is a host process and127.0.0.1:<published port>really is the sandbox — so the mode developers use all day is unaffected.The change
Move the line to
localSandboxEnv(), and add a config test that crosses the gap: it fails with the line in its old place and passes with it moved.npm test4286 pass / 0 fail, typecheck and lint clean.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.