Repository navigation
🏝️ fix: Apply the No-Workspace Decision Wherever an Agent's Code Route Resolves - #16798
Conversation
…e Resolves A conversation set to run without attached workspaces failed mid-turn with "Choose an attached workspace before using this agent." when its agent's default machine was attached and the turn loaded a skill or another tool at execution time. Request-start initialization honoured the decision, but the execution-time tool loader re-resolved the agent's attached default and then demanded a workspace selection the conversation intentionally never made. The per-agent code rule now lives in one typed function, resolveAgentCodeExecution (packages/api/src/code/agent.ts): it applies the conversation's no-workspace opt-out, the run-code grant, the stateful capability and the stateful environment policy, then resolves the base route. Request-start initialization, lazy subagent metadata and graph members, the execution-time and definition tool loaders, and the legacy execute_code loader all go through it, so an opted-out agent lands on the managed default route with no code tools at every site. Per-call subagent machine choices offer and accept no attached machine in such a conversation, and a source invariant test keeps new resolver call sites on the shared rule.
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: 15a6cd7383
ℹ️ 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".
| isImplicitStatefulCodeRouteAvailable( | ||
| process.env.CODE_ENVIRONMENT_DECISION_VERSION, | ||
| process.env.LIBRECHAT_CODE_BASEURL_STATEFUL, | ||
| ), |
There was a problem hiding this comment.
When this rule is exercised with different deployment/test configurations in the same process, reading these environment variables directly makes the opt-out decision depend on hidden global state rather than the caller's resolved configuration; callers can no longer supply or isolate the implicit-route decision without mutating process.env. Add the availability value to AgentCodeFlagsParams and have each request boundary pass it in, as the repository requires backend modules to receive configuration instead of reading global state.
AGENTS.md reference: AGENTS.md:L90-L99
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2b0dae9. The rule now takes implicitStatefulRouteAvailable as a parameter and stays a pure function of its inputs. The request boundaries (initializeAgent and withRequestCodeInputs, which every /api call site uses) resolve the rollout from the deployment configuration, and tests cover both values.
The per-agent code rule read the implicit stateful route rollout from the environment itself. It now takes it as a parameter, and the request boundaries (initializeAgent and withRequestCodeInputs) resolve it from the deployment configuration, so the rule stays a pure function of its inputs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b0dae99e2
ℹ️ 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".
| AgentCapabilities.stateful_code_sessions, | ||
| ), | ||
| conversationId: runtimeRequestBody?.conversationId, | ||
| resolvedContext: codeExecutionContext, |
There was a problem hiding this comment.
When an attached-default agent selects without_attached and has programmatic MCP tools, initializeAgent passes a stateless codeExecutionContext after setting its effective code availability to false, but the CJS tool-loader wrapper supplies only a stripped agent containing id, tools, provider, and model. This call therefore recomputes codeExecutionEnabled as true because it cannot see the attached/stateful fields, and buildToolClassification advertises the programmatic bash tool; execution later recomputes with the full agent and skips that tool, leaving the model with a tool it cannot invoke. Carry the already-resolved availability or the route fields alongside the reused context instead of independently recomputing the flag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8f062c3. initializeAgent now passes its attachedEnvironmentOptOut to the tool loader next to the reused codeExecutionContext. All three createToolLoader wrappers (chat, OpenAI-compatible and Responses) forward it to loadAgentTools, and both definition paths hand it to the shared rule. The rule can add an opt-out but never clears one a caller passes in. A partial agent record therefore keeps codeExecutionEnabled false, so classification no longer advertises programmatic bash for an opted-out agent. Tests cover the rule, the initializer argument, the CJS forwarding, and the definitions payload.
initializeAgent hands the tool loaders a partial agent record (id, tools, provider, model) together with the code route it already resolved. Without the agent's route fields the definition loaders could not see a no-workspace opt-out and recomputed code execution as enabled, so tool classification could advertise the programmatic bash tool that execution then refuses. initializeAgent now passes attachedEnvironmentOptOut through every tool loader, and the shared rule keeps an opt-out its caller already decided.
|
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". |
…e Rule After the dev merge, three lane-recording tests in ToolService.spec.js still stubbed resolveCodeExecutionContext, which LibreChat-AI#16798 removed from the execution-time loader in favour of resolveAgentCodeExecution. The stub variable no longer exists, so the tests failed with a ReferenceError and ESLint reported no-undef on a file this branch touches. The tests now configure the attached environment and the agent's code_environment_id, as the neighbouring attached-transport test does, and let the shared rule resolve the route. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Stops before LibreChat-AI#16798, whose ToolService spec uses an undeclared mock (fixed on dev by LibreChat-AI#16840).
Upstream's lane tests (LibreChat-AI#16793) call mockResolveCodeExecutionContext, which does not exist since LibreChat-AI#16798 renamed the code-route mock, so they throw a ReferenceError. Resolve the attached environment the way their passing neighbours do: configure statefulCodeSessions.environments and give the agent code_environment_id. Test-only; upstream main (e1dfc10) has the same failure. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HUZnrJ6DoLGzKRxd5uZGFu
1 participant
Pull Request
Summary
A conversation set to "No workspace" (
codeEnvironmentMode: "without_attached") could fail mid-turn with "Choose an attached workspace before using this agent." when the agent's default code machine was an attached machine. The turn started normally, but as soon as the model loaded a skill (or any tool that the execution-time loader resolves, such asread_fileorbash_tool), the request died with a 409code_workspace_unavailable(CodeWorkspaceSelectionError, reasonrequired) thrown fromresolveCodeExecutionWorkspaceContextinsideloadToolsForExecution(api/server/services/ToolService.js), reached through theON_TOOL_EXECUTEloadToolscallback inapi/server/services/Endpoints/agents/initialize.js.The cause was that only some resolvers applied the conversation's decision. Request-start initialization (
initializeAgent) and lazy subagent metadata calledoptsOutOfAttachedCodeEnvironmentbefore resolving the code route, so the agent ran with no attached machine and no code tools. The execution-time loader did not. It resolved the agent's saved attached default from scratch and then asked for the workspace selection that a "No workspace" conversation never makes on purpose. The definition loaders and the legacyexecute_codeloader also skipped the opt-out, but their callers happened not to reach it there.This PR moves the per-agent code rule into one typed function,
resolveAgentCodeExecutioninpackages/api/src/code/agent.ts, and every server path that resolves an agent's code route now goes through it. An agent whose default is an attached machine runs on the managed default route with no code tools in a "No workspace" conversation, the same way request start already handled it, whichever path asks. Skill loads on that route skip sandbox file priming cleanly because the agent'scodeEnvAvailableis false. Attached-mode conversations still require a workspace selection as before.How it works
The shared rule decides the flags and the base route together:
Call sites now on the rule:
initializeAgent(packages/api/src/agents/initialize.ts)getSubagentCodeFlags/toLazySubagentMetadata(initialize.js)bash_tool,read_file, PTC)loadToolsForExecution(ToolService.js), used by chat, OpenAI-compatible and Responses controllersloadToolDefinitionsWrapperandloadAgentToolsfallbacks (ToolService.js)execute_codetoolhandleTools.jsfallbackwithRequestCodeInputsfills the request-scoped inputs (admitted conversation, deployment machines, inheritance, principal) so the/apicall sites stay thin wiring.Subagents follow the same rule. A child whose default is attached is opted out by its own flags, so it advertises no per-call
machine/workspacechoice (#16757) and inherits no parent machine (#16756), since a "No workspace" decision admits no selections. Per-call routing now also checks the mode explicitly:resolveSubagentCodeTargetsreturns no targets in a "No workspace" conversation, so a model-suppliedmachineorworkspaceargument is refused asnot_allowedwithout probing any worker. The existing design has no exception that lets a per-call choice add an attached machine the conversation did not admit, and this keeps it that way.Approval mode (#16763) reads its targets from the initialized agent configs, whose routes come from the same rule. A "No workspace" turn therefore still has no attached approval target, and the execution-time context now matches the initialized one.
A source invariant test (
agent.spec.ts) scanspackages/api/srcandapi/and fails when a new file callsresolveCodeExecutionContextoroptsOutOfAttachedCodeEnvironmentoutside the shared rule, or when an/apifile binds more workspaces than it resolved throughresolveAgentCodeExecution. The remaining direct callers are listed with reasons: the per-call target probe intargets.ts, gated on the rule's flags and the mode, and the prewarm and file-session fallbacks, which consume flagsinitializeAgentalready resolved and never name a machine.Type of change
Testing
Reproduced the incident shape in tests that run the real routing logic: an agent with stateful sessions and an attached default machine (
code-yuwoQAAPhY1WMaDD6oIk), a conversation whose decision iswithout_attachedwith no selections, and an execution-time load of the skill tool. Before this change the ToolService test threwCodeWorkspaceSelectionError('required'). After it, the load resolves the managed default route and offers none of the attached machine tools. The same agent in an attached-mode conversation still throwsrequiredwithout a selection.Tested environments/configuration:
Automated tests:
packages/api/src/code/agent.spec.tscovers the incident, attached mode still requiring a selection, a managed stateful agent keeping its machine, the grant, tool and policy gates, reuse of an initialized context,withRequestCodeInputs, and the call-site invariant.packages/api/src/code/targets.spec.tschecks that per-call routing in a "No workspace" conversation offers and accepts no machine or workspace and probes no worker.api/server/services/__tests__/ToolService.spec.jsadds the incident repro (skill load, attached tools not offered, attached mode stillrequired). Existing routing tests now run the real route resolver against configured environments instead of a mocked one.api/server/services/Endpoints/agents/initialize.spec.jschecks that a lazy subagent with an attached default and an allowlist stays off attached machines in a "No workspace" conversation: no host arguments, a refused per-callmachine, no worker probes, and the decision is carried into the child's initialization.npx tsc --noEmitinpackages/api,npm run build:api, jest forpackages/apisrc/codeandsrc/agents, and jest forapiToolService,Endpoints/agents,app/clients/toolsandcontrollers/agents.Screenshots / recordings
No user-facing change.
Risk / compatibility
optsOutOfAttachedCodeEnvironmentmoved fromagents/initialize.tstocode/agent.ts. It is still exported from@librechat/apiunder the same name. The subagent stateful environment is now normalized the same way request start normalizes it, so an unrecognized stored value resolves touserinstead of being checked raw against the deployment allowlist. The definition loaders only receive stripped agent records with no stateful fields, so the opt-out cannot change what they register.Checklist