Repository navigation
🏝️ fix: Let a No-Workspace Turn Carry a Remembered Approval Mode - #16763
Conversation
A conversation set to run without attached workspaces failed every turn with "The selected code approval mode is not permitted by the current policy" when the reader had picked Accept edits or Full access for a workspace earlier. With no attached target, the server validated the requested mode against an empty policy that only allows ask. A turn with no attached target now accepts any known mode, resolves it to ask for the run, and keeps the conversation's stored mode instead of overwriting or unsetting it. Unknown values are still rejected, and any attached target, including a subagent's, keeps the strict check. The composer treated an explicit no-workspace chat as still running on the agent's default machine, so it kept offering and submitting the remembered mode. It now offers no attached mode there, and the submit path sends ask whenever the turn resolves to run without a workspace.
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: 9024359290
ℹ️ 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".
| codeEnvironmentMode?: CodeEnvironmentMode; | ||
| }): CodeApprovalMode | undefined { | ||
| if (fallback == null) return undefined; | ||
| if (codeEnvironmentMode === 'without_attached') return 'ask'; |
There was a problem hiding this comment.
When a new conversation has no persisted row yet, selecting or inheriting fullAccess/acceptEdits and then sending its first turn with No workspace replaces that choice with ask. The backend can preserve a mode only from the existing row or the request, so this forced ask becomes the stored value; after a reload or later workspace attachment, the user's explicit or remembered selection is lost. Carry the preference separately or send it now that the server safely resolves no-target execution to ask.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in 05ceaa9. Rather than sending the raw pick (the submit path should keep sending the policy-gated mode), the server no longer seeds a mode from the request on a turn without an attached target. Such a turn now only carries forward a mode the conversation already stores; when none is stored it records nothing, so the first no-workspace turn of a new chat leaves the row without a mode (omitting the key from save options is a no-op $unset on a row that has none) and the composer keeps offering the browser's remembered pick, which CodeApprovalMenu writes on every selection. A stored mode now only ever comes from a turn that validated it against attached targets. Covered by the Mongo round trip in save.spec.ts (ask and fullAccess on a new chat without a workspace leave it unset; a later attached turn records acceptEdits, which a following no-workspace ask keeps) and by client.test.js.
| withoutAttached | ||
| ? [] |
There was a problem hiding this comment.
When the primary conversation uses without_attached but a reachable lazy or graph subagent has its own attached environment, the server still collects that subagent as an attached target and validates its policy. This branch nevertheless discards every discovered attached environment solely because of the primary mode, making available false; the submission path then sends ask, so users cannot choose policy-permitted acceptEdits or fullAccess for the subagent and must approve its writes or commands individually.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not applicable as written: the server does not keep a subagent's own attached machine in a no-workspace turn. optsOutOfAttachedCodeEnvironment runs per agent against the same request body, and every initialization path applies it: the primary and eager/graph configs through initializeAgent (requestBody: runtimeRequestBody), and lazy subagents through toLazySubagentMetadata in api/server/services/Endpoints/agents/initialize.js, which calls it with runtimeRequestBody before resolving the context. Any agent whose configured machine is attached is opted out when codeEnvironmentMode === 'without_attached', and inheritance (#16756) has no parent machine to pass down, since the parent is opted out too. So no-workspace turns produce an empty settings map, which is the case this PR fixes. The server-side subagent tests in byom.spec.ts/client.test.js are defensive: if a target ever does survive, validation stays strict. On the client, hiding the menu and sending ask in that state is the fail-closed choice, and ask is accepted by every target.
codegraph-librechat
Bot
added
the
🗺️ Agent Lifecycle
|
Codex Review: Didn't find any major issues. Delightful! 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". |
A turn without an attached target seeded the conversation's approval mode from the request when the row stored none. A current client sends the gated ask there, so the first no-workspace turn of a new chat wrote ask and replaced the remembered pick the composer would otherwise keep offering. Such a turn now only carries forward a mode the conversation already stores; a stored mode still comes only from a turn that validated it against its attached targets.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ceaa9436
ℹ️ 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".
| conversationId: this.options.req.body.conversationId, | ||
| conversation: this.options.req.resolvedConversation, |
There was a problem hiding this comment.
When overrideConvoId targets a different conversation and the turn has no attached workspace, these arguments still identify the source conversation: the request controller keeps the override only as effectiveConversationId, and BaseClient.setMessageOptions later saves under the override. Consequently, a source row storing fullAccess can overwrite an override target that stored acceptEdits or no mode, rather than preserving the target's state. Resolve the persisted mode against the effective overridden conversation, or avoid carrying source state into that save.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in 4299af4. resolvePersistedCodeApprovalMode now takes the request's overrideConvoId and reads the stored mode only when the loaded row is the conversation the turn is saved under (the override with its __index suffix stripped, as BaseClient.processOverideIds does). Under an override to another conversation, a no-workspace turn records no mode, so the source row's mode is no longer copied onto the target. Covered in byom.spec.ts (override to another id, override to the same id, empty override) and client.test.js.
| ); | ||
| /** A chat that runs without attached workspaces opts every agent out of its attached machine | ||
| * on the server, so no attached mode applies to it, whatever the agents' defaults are. */ | ||
| const withoutAttached = conversation?.codeEnvironmentMode === 'without_attached'; |
There was a problem hiding this comment.
This only recognizes a persisted without_attached value, but useCodeWorkspace deliberately infers that same mode for saved chats with no recorded code-environment decision and while workspace metadata is unavailable. In those cases this hook still reports attached modes as available, so the composer can show and let the reader select Full access even though resolveSubmission sends the turn as without_attached and resolveSubmittedCodeApprovalMode silently replaces the selection with ask. Base availability on the resolved/inferred workspace mode as well as the stored field so the visible control reflects what the turn will use.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in 4299af4. useCodeApprovalMode takes the workspace mode the composer resolved (useCodeWorkspace().mode, which includes the inferred without_attached for a saved chat with no recorded decision and for a draft still choosing) and prefers it over the stored field. ChatForm passes it to CodeApprovalMenu and useSteering (so a queued turn's snapshot matches too), and useChatFunctions passes codeWorkspaceState.mode; the submit path still gates on the mode actually resolved at send time. Tests: useCodeApprovalMode.test.ts (resolved without_attached on a chat with no recorded decision hides the modes, attached restores them) and useSteering.spec.tsx (the resolved mode reaches the hook).
…ved Conversation The composer hid attached approval modes only for a chat that recorded a no-workspace decision, while useCodeWorkspace also resolves one for a saved chat with no recorded decision and for a draft still choosing. The approval menu, the queued-turn snapshot, and the submit path now read the mode the composer resolved, so the control matches what the turn sends. A no-workspace turn kept the mode of the row admission loaded, but a turn sent with overrideConvoId is saved under another conversation; the stored mode is now read only when that row is the conversation being saved.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4299af4f19
ℹ️ 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".
| codeEnvironmentMode?: CodeEnvironmentMode; | ||
| }): CodeApprovalMode | undefined { | ||
| if (fallback == null) return undefined; | ||
| if (codeEnvironmentMode === 'without_attached') return 'ask'; |
There was a problem hiding this comment.
When a browser running this head submits a without_attached turn to a pre-change backend replica during a rolling deployment or from an already-open tab, this unconditional ask is processed by the old empty-target fallback and persisted, replacing a stored acceptEdits or fullAccess preference. The new backend preservation logic cannot protect requests that land on the older replica, so gate this wire-format change on an advertised server capability or continue sending the policy-validated preference until mixed versions are safe.
AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not adopting this one. On a pre-change replica a no-workspace turn validates the requested mode against the empty-target fallback, which allows only ask, so ask is the only value an old replica accepts there. Sending the remembered acceptEdits/fullAccess reproduces the exact turn failure this PR fixes, and omitting the mode makes the old replica $unset the stored one, so a capability gate has no fallback value that preserves the mode on an old replica. The cost of ask is limited to the rollout window and to a preference: the turn succeeds, nothing is granted, and the reader can re-pick when a workspace is attached. Once every replica runs this head, the server preserves the stored mode whatever the client sends.
| const stored = | ||
| conversation != null && conversation.conversationId === savedConversationId | ||
| ? conversation.codeApprovalMode | ||
| : undefined; | ||
| return isCodeApprovalMode(stored) ? stored : undefined; |
There was a problem hiding this comment.
Fresh evidence after 4299af4: on a no-workspace overrideConvoId__0 request where the loaded source row has codeApprovalMode, this mismatch returns undefined, but BaseClient still passes that source row as the existing conversation; getUnsetFields therefore sees the source mode omitted from the save options and adds codeApprovalMode to the $unset applied to the override target. The target's own acceptEdits or fullAccess value is consequently deleted rather than preserved, so this path needs to load/carry the target row's value or prevent source-derived unsets.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in 8bc2170, by removing the dependence on the loaded row rather than reading the target. A turn with no attached target now records no mode and declares codeApprovalMode preserved: AgentClient.getTurnConversationFields passes preservedFields from getCodeApprovalPreservedFields, and getUnsetFields in packages/api/src/conversations/save.ts skips preserved keys. The write therefore neither sets nor unsets the mode, so whichever conversation the turn is saved under (an overrideConvoId target included) keeps its own mode, whatever row admission loaded. The override id parsing from the previous head is gone. Covered by a Mongo round trip in save.spec.ts (target keeps acceptEdits when a no-workspace turn is saved under it with the source row, which stores fullAccess, loaded; the source is unchanged), plus byom.spec.ts and client.test.js. Attached turns record exactly the validated mode and preserve nothing, as before.
…kspace Turn A no-workspace turn chose what to record from the row admission loaded, but a turn sent with overrideConvoId is saved under another conversation while its unsets are still computed from that loaded row, so either the source row's mode was copied onto the target or the target's own mode was unset. Such a turn now records no mode and declares codeApprovalMode preserved: the conversation write skips it when computing unsets, so the stored mode of whichever conversation the turn is saved under stays as it is, and a chat that stores none keeps offering the remembered pick. Turns with attached targets record exactly the mode they validated, as before.
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
Dev's per-call subagent machine selection (#16757) changed the approval-mode resolver to validate against every attached policy a run may execute under, including machines a parent may route a subagent to per call. The no-target branch, persisted mode, and preserved-field rule now read that policy list (collectAttachedCodeApprovalPolicies), so a turn is treated as having no attached target only when no agent runs, or may be routed, on one.
1 participant
Summary
Choosing No workspace for an agent conversation fails the whole turn with "The model provider could not complete this request. The selected code approval mode is not permitted by the current policy." whenever the reader had picked Accept edits or Full access for a workspace before (on that chat, or as the browser's remembered default from #15973). The trace from the demo deployment:
Two things combined. On the server, a no-workspace turn opts every agent out of its attached machine, so no agent in the turn has an attached execution context and the per-target policy map is empty.
resolveAttachedCodeApprovalModethen fell back to validating the request against a policy with no permissions schema, which allows onlyask, so any stronger mode threw. The same resolver runs ingetSaveOptions,sendCompletionandcreateRun, so every entry point failed. On the client,useCodeApprovalModeignoredcodeEnvironmentMode: 'without_attached'and kept resolving the agent's default attached machine, so the composer kept offering the remembered mode anduseChatFunctionsput it in the request body. The hook's comment said the server ignores the mode when no attached tool is active, which was only true forask.This PR makes the server authoritative, so older clients are fixed too. When the turn has no attached target at all, across the root agents and every eager, lazy and graph subagent the turn can reach, including machines a parent may route a subagent to per call (#16757), a requested mode governs nothing: any known mode is accepted and resolves to
askfor the run, so a target discovered later still starts gated. Values that are not approval modes are still rejected. Once any attached target exists, including a subagent's own machine while the parent runs without a workspace, validation is exactly as strict as before, including "Full access requires every target to permit it". The approvals-disabled branch is unchanged. On the client, the hook follows the workspace mode the composer resolved (useCodeWorkspace().mode, which also covers a saved chat with no recorded decision and a draft still choosing) and offers no attached mode when that iswithout_attached, so the approval menu and the queued-turn snapshot no longer show or carry a mode that governs nothing, and the submit path sendsaskwhenever the turn resolves to run without a workspace, including when the composer only infers that at send time.Persistence.
getSaveOptionscannot simply drop the key on these turns:BaseClientunsets stored conversation fields the save options omit, computed from the row admission loaded, so that would erase the reader's pick, and writingaskwould overwrite it. A turn without an attached target makes no approval decision, so it records no mode and declarescodeApprovalModepreserved: the conversation write neither sets nor unsets it. The stored mode of whichever conversation the turn is saved under (anoverrideConvoIdtarget included) stays as it is, and a new chat's first no-workspace turn leaves the row without a mode, so the composer keeps offering the browser's remembered pick instead of a storedask. A stored mode therefore only ever comes from a turn that validated it against attached targets. Leaving a workspace for a turn and coming back keeps the pick, and that pick grants nothing on its own: it is validated again against the targets of whichever later turn uses it, including background and subagent wake-ups that replay the stored mode. A turn with attached targets records exactly the mode it validated and preserves nothing, as before.How it works
resolvePersistedCodeApprovalModewraps that forgetSaveOptions: with attached targets it returns the validated mode; without, it still rejects garbage but returns nothing.getCodeApprovalPreservedFieldsreturns['codeApprovalMode']for the same no-target condition;AgentClient.getTurnConversationFieldspasses it aspreservedFields, andgetUnsetFieldsinpackages/api/src/conversations/save.tsskips preserved keys. No database read is added.Type of change
Testing
Tested environments/configuration: Windows 11, Node 24,
mongodb-memory-server.Automated tests:
packages/api/src/agents/hitl/byom.spec.ts: every known mode accepted and resolved toaskwith no attached target; garbage values still rejected; strict rejection with an attached target present; a parent without a workspace validated against a lazy subagent's or graph member's restrictive machine; approvals-disabled contract unchanged; persisted-mode and preserved-field rules.packages/api/src/conversations/save.spec.ts: real MongoDB round trip throughsaveTurnConversation: Full access on a workspace turn, then no-workspace turns sendingask(current client),fullAccess(older client) and garbage, then back on a workspace where the kept mode is revalidated (permissive passes, restrictive throws, an omitted mode still unsets); a new chat without a workspace stays unset until a workspace turn validates a mode; a no-workspace turn saved under another conversation keeps that conversation's mode and does not copy the loaded row's.api/server/controllers/agents/client.test.js: the exact repro throughAgentClient.getSaveOptionsandgetTurnConversationFields(no workspace plus any requested mode proceeds, records no mode and preserves the stored one), nothing preserved once a turn runs on an attached machine, garbage rejected, and subagent-machine validation.client:useCodeApprovalModeoffers no attached mode for a no-workspace chat or a composer-resolvedwithout_attachedand recovers onattached;useSteeringforwards the resolved mode;resolveSubmittedCodeApprovalModeunit tests;useChatFunctionssendsaskin the payload for a turn resolved without a workspace while the chat keeps its pick.npx tsc --noEmitinpackages/api,npm run typecheckinclient, ESLint and import order on touched files.Screenshots / recordings
The approval-mode menu no longer renders in a chat set to No workspace; before, it showed the remembered mode even though no attached machine was in use. Not captured here because it needs a live attached machine; the hook test covers the before/after states.
Risk / compatibility
A no-workspace turn no longer overwrites the stored approval mode. An older client that explicitly picked a mode while in a no-workspace chat (where the menu governed nothing) will see the stored mode, or its browser's remembered pick when the chat stores none, again after reload; either is still validated against policy before it applies to any attached target.
Checklist