-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(web-shell): defer session creation until first prompt #6066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
49eca5b
08b4278
49e833f
1288afe
0014e47
76b5fd3
5ddb217
af17c1a
6f513ae
cd3b62a
41252b5
1e7271a
88e7522
c3cf484
104a60e
9e9d8fa
df0df95
c1591c5
0c60ad0
48ad570
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,9 @@ | |
| */ | ||
|
|
||
| import { | ||
| ApprovalMode, | ||
| APPROVAL_MODES, | ||
| createDebugLogger, | ||
| ModelsConfig, | ||
| resolveProviderProtocol, | ||
| tokenLimit, | ||
|
|
@@ -30,6 +33,8 @@ import { | |
| sanitizeProviderBaseUrl, | ||
| } from '../utils/acpModelUtils.js'; | ||
|
|
||
| const debugLogger = createDebugLogger('WORKSPACE_PROVIDERS_STATUS'); | ||
|
|
||
| export type WorkspaceProvidersStatusProvider = ( | ||
| workspaceCwd: string, | ||
| acpChannelLive: boolean, | ||
|
|
@@ -97,6 +102,7 @@ function buildWorkspaceProvidersStatus( | |
| typeof settings.fastModel === 'string' && settings.fastModel.length > 0 | ||
| ? settings.fastModel | ||
| : undefined; | ||
| const approvalMode = resolveApprovalMode(settings); | ||
| const providers = new Map<string, ServeWorkspaceProviderStatus>(); | ||
| const explicitModelBaseUrls = buildExplicitModelBaseUrls( | ||
| settings.modelProviders, | ||
|
|
@@ -169,6 +175,7 @@ function buildWorkspaceProvidersStatus( | |
| initialized: true, | ||
| acpChannelLive, | ||
| ...(current ? { current } : {}), | ||
| approvalMode, | ||
|
ytahdn marked this conversation as resolved.
|
||
| providers: [...providers.values()], | ||
| ...(resolvedCliConfig.warnings.length > 0 | ||
| ? { | ||
|
|
@@ -200,6 +207,24 @@ function buildWorkspaceProvidersStatus( | |
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Consider splitting this into a separate small PR. Benefits:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the scope callout. I am leaving approvalMode in this PR for now because the current session-preparation flow reads the daemon-provided approval mode as part of first-prompt setup, and splitting it out now would add another branch dependency/rebase step to an already active lifecycle PR. I am not resolving this thread since it is a scope suggestion rather than a code fix in this follow-up. |
||
| } | ||
|
|
||
| function resolveApprovalMode(settings: Settings): ApprovalMode { | ||
| const value = settings.tools?.approvalMode; | ||
| if (typeof value !== 'string') return ApprovalMode.DEFAULT; | ||
|
|
||
| const normalized = value.trim().toLowerCase().replaceAll('_', '-'); | ||
| const mode = normalized === 'autoedit' ? ApprovalMode.AUTO_EDIT : normalized; | ||
| if ((APPROVAL_MODES as readonly string[]).includes(mode)) { | ||
| return mode as ApprovalMode; | ||
| } | ||
|
|
||
| if (value.trim().length > 0) { | ||
| debugLogger.warn( | ||
| `[workspace-providers-status] unrecognized approvalMode "${value}", falling back to default`, | ||
| ); | ||
| } | ||
| return ApprovalMode.DEFAULT; | ||
|
ytahdn marked this conversation as resolved.
|
||
| } | ||
|
|
||
| function isMainSelectableModel(model: { | ||
| fastOnly?: boolean; | ||
| voiceOnly?: boolean; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] This uses the raw merged
tools.approvalModefrom settings, but session creation normally ignores that setting in--safe/--baremodes and downgrades unsafe modes for untrusted folders. The deferred web-shell path copies this value intoconnection.currentModeand then applies it withsetApprovalMode()before the first prompt, so a workspace setting likeyolocan re-enable a mode that the normal session config would have forced back todefault. Please return the daemon's effective approval mode with the same safety/trust precedence as session config, or omit it until a real session context is available.— gpt-5 via Qwen Code /review