Skip to content

feat(daemon): expose visionModelId in workspace provider status and web-shell model dialog #6262

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.

Sign up for GitHub

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

Merged
wenshao merged 4 commits into from
Jul 3, 2026
Merged

feat(daemon): expose visionModelId in workspace provider status and web-shell model dialog #6262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c4c8116
ci(autofix): restore sandbox image flow
yiliang114 Jul 3, 2026
e053581
feat(daemon): expose visionModelId in workspace provider status and w…
Jul 3, 2026
d0af634
merge: resolve conflict with origin/main in web-shell App.tsx
qwen-code-dev-bot Jul 3, 2026
71101ba
merge: resolve conflict with origin/main in autofix sandbox-image CI
qwen-code-dev-bot Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/acp-bridge/src/status.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ export interface ServeWorkspaceProviderCurrent {
modelId?: string;
baseUrl?: string;
fastModelId?: string;
visionModelId?: string;
}

export interface ServeWorkspaceProviderModel {
Expand Down
27 changes: 27 additions & 0 deletions packages/cli/src/serve/workspace-providers-status.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,33 @@ describe('createWorkspaceProvidersStatusProvider', () => {
expect(withEmptyFastModel.current).not.toHaveProperty('fastModelId');
});

it('includes only non-empty vision model settings in current selection', async () => {

@qwen-code-ci-bot qwen-code-ci-bot Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The new test always sets authType: 'openai' and model.name: 'main-model', so buildCurrent always has truthy authType and modelId. The guard clause change (!visionModelId added to the early-return check) is never exercised — no test verifies that visionModelId alone (all other fields falsy) produces a defined current object. Consider adding a test case where only visionModel is configured.

— qwen3.7-max via Qwen Code /review

const provider = createWorkspaceProvidersStatusProvider({ env: {} });
await writeUserSettings({
security: { auth: { selectedType: 'openai' } },
model: { name: 'main-model' },
visionModel: 'vision-model',

@qwen-code-ci-bot qwen-code-ci-bot Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Test uses a plain string 'vision-model' and doesn't exercise the \0-encoded format that encodeVisionModelSelector produces for custom-endpoint models (e.g., 'openai:gpt-4o\0https://custom-endpoint.example.com/v1'). If this test had covered the \0 path, the raw-value leak in workspace-providers-status.ts would have been caught.

— qwen3.7-max via Qwen Code /review

modelProviders: {
openai: [{ id: 'main-model', name: 'Main Model' }],
},
});

const withVisionModel = await provider(workspace, false);
expect(withVisionModel.current?.visionModelId).toBe('vision-model');

await writeUserSettings({
security: { auth: { selectedType: 'openai' } },
model: { name: 'main-model' },
visionModel: '',
modelProviders: {
openai: [{ id: 'main-model', name: 'Main Model' }],
},
});

const withEmptyVisionModel = await provider(workspace, false);
expect(withEmptyVisionModel.current).not.toHaveProperty('visionModelId');
});

it('does not include runtime models in the workspace provider catalog', async () => {
const provider = createWorkspaceProvidersStatusProvider({
argv: { model: 'runtime-only-model' },
Expand Down
11 changes: 10 additions & 1 deletion packages/cli/src/serve/workspace-providers-status.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ function buildWorkspaceProvidersStatus(
typeof settings.fastModel === 'string' && settings.fastModel.length > 0
? settings.fastModel
: undefined;
const visionModelId =

@qwen-code-ci-bot qwen-code-ci-bot Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Raw settings.visionModel is passed through as visionModelId without parsing. The visionModel setting uses a \0-delimited composite format (selector\0baseUrl) produced by encodeVisionModelSelector in ModelDialog.tsx. This raw string — including the null byte and embedded private base URL — is exposed verbatim in the HTTP status API.

Contrast with fastModel (encoded by encodeAuxModelSelector which strips \0baseUrl) and modelId (goes through formatAcpModelId()). The visionModelId field skips all formatting.

Impact: Internal endpoint URLs leak to status API consumers; \0 byte in JSON can cause silent truncation in downstream parsers; semantic mismatch — field named visionModelId carries a composite blob.

Suggested change
const visionModelId =
const parsedVision =
typeof settings.visionModel === 'string' &&
settings.visionModel.length > 0
? parseVisionModelSetting(settings.visionModel)
: undefined;
const visionModelId = parsedVision?.selector ?? undefined;

— qwen3.7-max via Qwen Code /review

typeof settings.visionModel === 'string' &&
settings.visionModel.length > 0
? settings.visionModel
: undefined;
const approvalMode = resolveApprovalMode(settings);
const providers = new Map<string, ServeWorkspaceProviderStatus>();
const explicitModelBaseUrls = buildExplicitModelBaseUrls(
Expand Down Expand Up @@ -167,6 +172,7 @@ function buildWorkspaceProvidersStatus(
currentAcpModelId,
currentBaseUrl,
fastModelId,
visionModelId,
);

return {
Expand Down Expand Up @@ -391,12 +397,15 @@ function buildCurrent(
modelId: string | undefined,
baseUrl: string | undefined,
fastModelId: string | undefined,
visionModelId: string | undefined,
): ServeWorkspaceProviderCurrent | undefined {
if (!authType && !modelId && !baseUrl && !fastModelId) return undefined;
if (!authType && !modelId && !baseUrl && !fastModelId && !visionModelId)
return undefined;
return {
...(authType ? { authType: String(authType) } : {}),
...(modelId ? { modelId } : {}),
...(baseUrl ? { baseUrl: sanitizeProviderBaseUrl(baseUrl) } : {}),
...(fastModelId ? { fastModelId } : {}),
...(visionModelId ? { visionModelId } : {}),
};
}
Loading