From 47f2b1a1289c165e56c8dddc6a61c2501f7b2d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=9F=E6=9C=88?= Date: Wed, 13 May 2026 09:43:57 +0800 Subject: [PATCH] fix(setup): address code review issues (#221) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix resolve variable shadowing in promptSecret (rename to fulfill) - Fix readline leak on invalid choice (close before returning err) - Remove Anthropic/Gemini from presets (not OpenAI-compatible) - Fix GLM URL: api.z.ai → open.bigmodel.cn - Restore terminal raw mode before process.exit on Ctrl+C - Add debug logging to fetchAvailableModels failures - Add comment explaining DashScope-specific model filter patterns - Move PresetProvider and CmdSetupSuccess types to types.ts per convention --- .../src/commands/setup/dispatch.ts | 22 ++++++++++++++----- .../cli-workflow/src/commands/setup/index.ts | 6 ++--- .../src/commands/setup/preset-providers.ts | 8 +++---- .../src/commands/setup/providers.yaml | 10 +-------- .../cli-workflow/src/commands/setup/setup.ts | 11 ++-------- .../cli-workflow/src/commands/setup/types.ts | 15 +++++++++++++ 6 files changed, 41 insertions(+), 31 deletions(-) diff --git a/packages/cli-workflow/src/commands/setup/dispatch.ts b/packages/cli-workflow/src/commands/setup/dispatch.ts index a1ba638..777ef41 100644 --- a/packages/cli-workflow/src/commands/setup/dispatch.ts +++ b/packages/cli-workflow/src/commands/setup/dispatch.ts @@ -1,11 +1,15 @@ import { existsSync } from "node:fs"; import { stdin as input, stdout as output } from "node:process"; import { createInterface } from "node:readline/promises"; -import { resolve } from "node:path"; +import { resolve as resolvePath } from "node:path"; import { err, ok, type Result } from "@uncaged/workflow-protocol"; +import { createLogger } from "@uncaged/workflow-util"; + import { printCliError, printCliLine, printCliWarn } from "../../cli-output.js"; + +const setupDispatchLog = createLogger({ sink: { kind: "stderr" } }); import { loadPresetProviders } from "./preset-providers.js"; import { cmdSetup, printSetupSummary } from "./setup.js"; import type { SetupCliArgs } from "./types.js"; @@ -153,7 +157,7 @@ async function promptLine( /** Read a line with terminal echo disabled (for secrets). */ async function promptSecret(label: string): Promise { process.stdout.write(label); - return new Promise((resolve) => { + return new Promise((fulfill) => { let buf = ""; const rawWasSet = process.stdin.isRaw; if (process.stdin.isTTY) { @@ -171,7 +175,7 @@ async function promptSecret(label: string): Promise { process.stdin.pause(); process.stdin.removeListener("data", onData); process.stdout.write("\n"); - resolve(buf.trim()); + fulfill(buf.trim()); return; } if (c === "\u007F" || c === "\b") { @@ -182,6 +186,9 @@ async function promptSecret(label: string): Promise { continue; } if (c === "\u0003") { + if (process.stdin.isTTY) { + process.stdin.setRawMode(rawWasSet); + } process.exit(130); } buf += c; @@ -205,19 +212,23 @@ async function fetchAvailableModels( signal: AbortSignal.timeout(10_000), }); if (!res.ok) { + setupDispatchLog("R5KH7WM3", `GET ${url} returned ${res.status}`); return []; } const body = (await res.json()) as OpenAiModelsResponse; if (!Array.isArray(body.data)) { return []; } + // Filter out non-chat models. Some patterns are DashScope-specific (sambert, cosyvoice, + // wordart, wanx, wan2, paraformer) but harmless for other providers. const NON_CHAT_RE = /speech|embed|image|video|audio|ocr|rerank|tts|asr|paraformer|sambert|cosyvoice|wordart|wanx|wan2|flux|stable-diffusion|z-image|s2s|livetranslate|realtime|gui-/i; return body.data .map((m) => m.id) .filter((id) => !NON_CHAT_RE.test(id)) .sort(); - } catch { + } catch (e) { + setupDispatchLog("V8NQ4JT6", `fetch models failed: ${e instanceof Error ? e.message : String(e)}`); return []; } } @@ -242,6 +253,7 @@ async function collectInteractiveSetup(): Promise> const choice = await promptLine(rl, `Choose [1-${presets.length + 1}]: `); const choiceNum = Number.parseInt(choice, 10); if (Number.isNaN(choiceNum) || choiceNum < 1 || choiceNum > presets.length + 1) { + rl.close(); return err(`invalid choice: ${choice}`); } @@ -331,7 +343,7 @@ async function collectInteractiveSetup(): Promise> } const candidate = wsPath === "" ? "./workflows" : wsPath; // Validate path before passing to cmdSetup. - const resolved = resolve(process.cwd(), candidate); + const resolved = resolvePath(process.cwd(), candidate); if (existsSync(resolved)) { printCliWarn(`directory already exists: ${resolved}`); printCliLine("Please enter a different path, or type 'skip' to skip."); diff --git a/packages/cli-workflow/src/commands/setup/index.ts b/packages/cli-workflow/src/commands/setup/index.ts index c058dcc..d1bea40 100644 --- a/packages/cli-workflow/src/commands/setup/index.ts +++ b/packages/cli-workflow/src/commands/setup/index.ts @@ -1,4 +1,4 @@ export { dispatchSetup } from "./dispatch.js"; -export { loadPresetProviders, type PresetProvider } from "./preset-providers.js"; -export { type CmdSetupSuccess, cmdSetup, printSetupSummary } from "./setup.js"; -export type { SetupCliArgs } from "./types.js"; +export { loadPresetProviders } from "./preset-providers.js"; +export { cmdSetup, printSetupSummary } from "./setup.js"; +export type { CmdSetupSuccess, PresetProvider, SetupCliArgs } from "./types.js"; diff --git a/packages/cli-workflow/src/commands/setup/preset-providers.ts b/packages/cli-workflow/src/commands/setup/preset-providers.ts index b56870a..34a9f45 100644 --- a/packages/cli-workflow/src/commands/setup/preset-providers.ts +++ b/packages/cli-workflow/src/commands/setup/preset-providers.ts @@ -3,11 +3,9 @@ import { join } from "node:path"; import { parse as parseYaml } from "yaml"; -export type PresetProvider = { - name: string; - label: string; - baseUrl: string; -}; +import type { PresetProvider } from "./types.js"; + + type RawPresetEntry = { name: unknown; diff --git a/packages/cli-workflow/src/commands/setup/providers.yaml b/packages/cli-workflow/src/commands/setup/providers.yaml index e237814..4ef3b7e 100644 --- a/packages/cli-workflow/src/commands/setup/providers.yaml +++ b/packages/cli-workflow/src/commands/setup/providers.yaml @@ -8,14 +8,6 @@ label: OpenAI baseUrl: https://api.openai.com/v1 -- name: anthropic - label: Anthropic - baseUrl: https://api.anthropic.com/v1 - -- name: gemini - label: Google Gemini - baseUrl: https://generativelanguage.googleapis.com/v1beta - - name: xai label: xAI baseUrl: https://api.x.ai/v1 @@ -52,7 +44,7 @@ - name: glm label: GLM (Zhipu AI) - baseUrl: https://api.z.ai/api/paas/v4 + baseUrl: https://open.bigmodel.cn/api/paas/v4 - name: stepfun label: StepFun diff --git a/packages/cli-workflow/src/commands/setup/setup.ts b/packages/cli-workflow/src/commands/setup/setup.ts index ddcebd0..c1ac0ea 100644 --- a/packages/cli-workflow/src/commands/setup/setup.ts +++ b/packages/cli-workflow/src/commands/setup/setup.ts @@ -9,18 +9,11 @@ import { createLogger } from "@uncaged/workflow-util"; import { printCliLine } from "../../cli-output.js"; import { cmdInitWorkspace } from "../init/index.js"; -import type { SetupCliArgs } from "./types.js"; +import type { CmdSetupSuccess, SetupCliArgs } from "./types.js"; const setupLog = createLogger({ sink: { kind: "stderr" } }); -export type CmdSetupSuccess = { - registryPath: string; - provider: string; - defaultModel: string; - maxDepth: number; - supervisorInterval: number; - initWorkspaceRootPath: string | null; -}; + function mergeWorkflowConfig( prev: WorkflowConfig | null, diff --git a/packages/cli-workflow/src/commands/setup/types.ts b/packages/cli-workflow/src/commands/setup/types.ts index e153ac6..9150dc8 100644 --- a/packages/cli-workflow/src/commands/setup/types.ts +++ b/packages/cli-workflow/src/commands/setup/types.ts @@ -6,3 +6,18 @@ export type SetupCliArgs = { defaultModel: string; initWorkspaceName: string | null; }; + +export type PresetProvider = { + name: string; + label: string; + baseUrl: string; +}; + +export type CmdSetupSuccess = { + registryPath: string; + provider: string; + defaultModel: string; + maxDepth: number; + supervisorInterval: number; + initWorkspaceRootPath: string | null; +};