Skip to content

Commit 026189c

Browse files
s-JoLclaude
s-JoL
and
claude
committed
fix: address code review feedback and add release workflow
- P1: Treat models without input metadata as potentially multimodal instead of hard-failing grounding - P2: Remove API-key gate from runtime grounding resolution; only require model presence, not a getApiKey() token - Add workflow_dispatch release.yml for one-command version bumps Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3711c7b commit 026189c

3 files changed

Lines changed: 125 additions & 43 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
name: Release
2+
3+
on:
4+
workflow_dispatch:
5+
inputs:
6+
version:
7+
description: "Release version (e.g., 0.1.6 or 0.1.6-pre.1)"
8+
required: true
9+
type: string
10+
11+
env:
12+
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
13+
14+
jobs:
15+
release:
16+
runs-on: ubuntu-latest
17+
timeout-minutes: 10
18+
19+
permissions:
20+
contents: write
21+
22+
steps:
23+
- name: Checkout
24+
uses: actions/checkout@v4
25+
with:
26+
token: ${{ secrets.GITHUB_TOKEN }}
27+
persist-credentials: true
28+
29+
- name: Configure git user
30+
run: |
31+
git config user.name "github-actions[bot]"
32+
git config user.email "github-actions[bot]@users.noreply.github.com"
33+
34+
- name: Validate branch is main
35+
run: |
36+
CURRENT_BRANCH="${GITHUB_REF_NAME}"
37+
if [ "${CURRENT_BRANCH}" != "main" ]; then
38+
echo "Release must be run from main branch, got: ${CURRENT_BRANCH}"
39+
exit 1
40+
fi
41+
42+
- name: Validate version format
43+
run: |
44+
VERSION="${{ inputs.version }}"
45+
if ! echo "${VERSION}" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9.]+)?$'; then
46+
echo "Invalid version format: ${VERSION}"
47+
echo "Expected: digits.digits.digits or digits.digits.digits-pre-suffix"
48+
exit 1
49+
fi
50+
51+
- name: Update version in package.json
52+
run: |
53+
node -e "
54+
const fs = require('fs');
55+
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
56+
pkg.version = '${{ inputs.version }}';
57+
fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n');
58+
"
59+
60+
- name: Commit version bump
61+
run: git commit -am "release: v${{ inputs.version }}"
62+
63+
- name: Create tag
64+
run: git tag "v${{ inputs.version }}"
65+
66+
- name: Push commit to main
67+
run: git push origin main
68+
69+
- name: Push tag
70+
run: git push origin "v${{ inputs.version }}"

‎apps/cli/src/commands/gui-grounding.test.ts‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ describe("primeGuiGroundingForConfig", () => {
144144
expect(resolved.groundingProvider).toBeUndefined();
145145
});
146146

147-
it("remains unavailable when the model lacks image input", async () => {
147+
it("remains unavailable when the model explicitly lacks image input", async () => {
148148
const authManager = {
149149
findModel: vi.fn().mockReturnValue({ provider: "openai-codex", id: "gpt-5.4", input: ["text"] }),
150150
getAvailableModels: vi.fn().mockReturnValue([{ provider: "openai-codex", id: "gpt-5.4", input: ["text"] }]),
@@ -166,6 +166,52 @@ describe("primeGuiGroundingForConfig", () => {
166166
expect(resolved.unavailableReason).toContain("image input");
167167
});
168168

169+
it("treats models without an input field as potentially multimodal", async () => {
170+
const authManager = {
171+
findModel: vi.fn().mockReturnValue({ provider: "openai-codex", id: "gpt-5.4" }),
172+
getAvailableModels: vi.fn().mockReturnValue([{ provider: "openai-codex", id: "gpt-5.4" }]),
173+
getApiKey: vi.fn().mockResolvedValue("main-model-key"),
174+
authStorage: {},
175+
modelRegistry: {},
176+
};
177+
178+
const resolved = await primeGuiGroundingForConfig(
179+
createConfig({
180+
defaultProvider: "openai-codex",
181+
defaultModel: "gpt-5.4",
182+
}),
183+
authManager as any,
184+
(() => ({ available: true, provider: "openai-codex", credentialType: "api_key", source: "env" })) as any,
185+
);
186+
187+
expect(resolved.available).toBe(true);
188+
expect(resolved.label).toBe("main:openai-codex/gpt-5.4");
189+
expect(typeof resolved.groundingProvider?.ground).toBe("function");
190+
});
191+
192+
it("resolves grounding without requiring an API key when runtime auth is available", async () => {
193+
const authManager = {
194+
findModel: vi.fn().mockReturnValue({ provider: "openai-codex", id: "gpt-5.4", input: ["text", "image"] }),
195+
getAvailableModels: vi.fn().mockReturnValue([{ provider: "openai-codex", id: "gpt-5.4", input: ["text", "image"] }]),
196+
getApiKey: vi.fn().mockRejectedValue(new Error("no API key")),
197+
authStorage: {},
198+
modelRegistry: {},
199+
};
200+
201+
const resolved = await primeGuiGroundingForConfig(
202+
createConfig({
203+
defaultProvider: "openai-codex",
204+
defaultModel: "gpt-5.4",
205+
}),
206+
authManager as any,
207+
(() => ({ available: true, provider: "openai-codex", credentialType: "oauth", source: "env" })) as any,
208+
);
209+
210+
expect(resolved.available).toBe(true);
211+
expect(resolved.label).toBe("main:openai-codex/gpt-5.4");
212+
expect(typeof resolved.groundingProvider?.ground).toBe("function");
213+
});
214+
169215
it("prefers the explicit runtime model when selecting the grounding provider", async () => {
170216
const explicitModel = { provider: "openai-codex", id: "gpt-5.4", input: ["text", "image"] };
171217
const authManager = {
@@ -195,7 +241,6 @@ describe("primeGuiGroundingForConfig", () => {
195241

196242
expect(resolved.available).toBe(true);
197243
expect(resolved.label).toBe("main:openai-codex/gpt-5.4");
198-
expect(authManager.getApiKey).toHaveBeenCalledWith(explicitModel);
199244
});
200245

201246
it("can reuse a fallback candidate from the resolved runtime model chain", async () => {

‎apps/cli/src/commands/gui-grounding.ts‎

Lines changed: 8 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,9 @@ const RUNTIME_GROUNDING_SYSTEM_PROMPT = [
3535
"Return strict JSON only and never include markdown fences, commentary, or tool calls.",
3636
].join("\n");
3737

38-
type ResolvedProviderCredential = {
38+
type ResolvedProviderModel = {
3939
provider: string;
4040
modelId: string;
41-
apiKey: string;
4241
model: Model<any>;
4342
};
4443

@@ -70,53 +69,28 @@ function resolveGuiGroundingThinkingLevel(config: UnderstudyConfig): RuntimeThin
7069
return config.agent.guiGroundingThinkingLevel ?? "medium";
7170
}
7271

73-
async function resolveProviderApiKey(params: {
72+
function resolveProviderModel(params: {
7473
authManager: AuthManager;
7574
provider: string;
7675
modelId: string;
77-
}): Promise<ResolvedProviderCredential | undefined> {
76+
}): ResolvedProviderModel | undefined {
7877
const models = params.authManager.getAvailableModels();
7978
for (const alias of providerAliases(params.provider)) {
8079
const exact =
8180
params.authManager.findModel(alias, params.modelId) ??
8281
models.find((model) => model.provider === alias && model.id === params.modelId) ??
8382
models.find((model) => model.provider === alias);
84-
if (!exact) {
85-
continue;
86-
}
87-
const apiKey = await params.authManager.getApiKey(exact).catch(() => undefined);
88-
if (typeof apiKey === "string" && apiKey.trim().length > 0) {
83+
if (exact) {
8984
return {
9085
provider: exact.provider,
9186
modelId: exact.id,
92-
apiKey: apiKey.trim(),
9387
model: exact,
9488
};
9589
}
9690
}
9791
return undefined;
9892
}
9993

100-
async function resolvePreferredProviderCredential(params: {
101-
authManager: AuthManager;
102-
candidate: RuntimeResolvedModelCandidate;
103-
}): Promise<ResolvedProviderCredential | undefined> {
104-
const directApiKey = await params.authManager.getApiKey(params.candidate.model).catch(() => undefined);
105-
if (typeof directApiKey === "string" && directApiKey.trim().length > 0) {
106-
return {
107-
provider: params.candidate.provider,
108-
modelId: params.candidate.modelId,
109-
apiKey: directApiKey.trim(),
110-
model: params.candidate.model,
111-
};
112-
}
113-
return await resolveProviderApiKey({
114-
authManager: params.authManager,
115-
provider: params.candidate.provider,
116-
modelId: params.candidate.modelId,
117-
});
118-
}
119-
12094
function buildGuiGroundingCandidates(
12195
options?: GuiGroundingModelSelectionOptions,
12296
): RuntimeResolvedModelCandidate[] {
@@ -325,7 +299,7 @@ export function createRuntimeGroundingProvider(options: {
325299
guideImageImpl?: RuntimeGuideImageImpl;
326300
simulationImageImpl?: RuntimeSimulationImageImpl;
327301
}): GuiGroundingProvider | undefined {
328-
if (!Array.isArray(options.model.input) || !options.model.input.includes("image")) {
302+
if (Array.isArray(options.model.input) && !options.model.input.includes("image")) {
329303
return undefined;
330304
}
331305
const timeoutMs = Math.max(5_000, Math.floor(options.timeoutMs ?? DEFAULT_RUNTIME_GROUNDING_TIMEOUT_MS));
@@ -443,7 +417,7 @@ export async function resolveMainModelGuiGroundingProvider(
443417
if (!providerStatus.available) {
444418
return undefined;
445419
}
446-
const resolved = await resolveProviderApiKey({
420+
const resolved = resolveProviderModel({
447421
authManager,
448422
provider: config.defaultProvider,
449423
modelId: config.defaultModel,
@@ -476,17 +450,10 @@ export async function resolveMainModelGuiGroundingProvider(
476450
if (!providerStatus.available) {
477451
continue;
478452
}
479-
const resolved = await resolvePreferredProviderCredential({
480-
authManager,
481-
candidate,
482-
});
483-
if (!resolved) {
484-
continue;
485-
}
486-
const label = `main:${resolved.provider}/${resolved.modelId}`;
453+
const label = `main:${candidate.provider}/${candidate.modelId}`;
487454
const runtimeProvider = createRuntimeGroundingProvider({
488455
authManager,
489-
model: resolved.model,
456+
model: candidate.model,
490457
providerName: label,
491458
thinkingLevel,
492459
});

0 commit comments

Comments
 (0)