fix(setup): address code review issues (#221)
- 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
This commit is contained in:
@@ -1,11 +1,15 @@
|
|||||||
import { existsSync } from "node:fs";
|
import { existsSync } from "node:fs";
|
||||||
import { stdin as input, stdout as output } from "node:process";
|
import { stdin as input, stdout as output } from "node:process";
|
||||||
import { createInterface } from "node:readline/promises";
|
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 { err, ok, type Result } from "@uncaged/workflow-protocol";
|
||||||
|
|
||||||
|
import { createLogger } from "@uncaged/workflow-util";
|
||||||
|
|
||||||
import { printCliError, printCliLine, printCliWarn } from "../../cli-output.js";
|
import { printCliError, printCliLine, printCliWarn } from "../../cli-output.js";
|
||||||
|
|
||||||
|
const setupDispatchLog = createLogger({ sink: { kind: "stderr" } });
|
||||||
import { loadPresetProviders } from "./preset-providers.js";
|
import { loadPresetProviders } from "./preset-providers.js";
|
||||||
import { cmdSetup, printSetupSummary } from "./setup.js";
|
import { cmdSetup, printSetupSummary } from "./setup.js";
|
||||||
import type { SetupCliArgs } from "./types.js";
|
import type { SetupCliArgs } from "./types.js";
|
||||||
@@ -153,7 +157,7 @@ async function promptLine(
|
|||||||
/** Read a line with terminal echo disabled (for secrets). */
|
/** Read a line with terminal echo disabled (for secrets). */
|
||||||
async function promptSecret(label: string): Promise<string> {
|
async function promptSecret(label: string): Promise<string> {
|
||||||
process.stdout.write(label);
|
process.stdout.write(label);
|
||||||
return new Promise((resolve) => {
|
return new Promise((fulfill) => {
|
||||||
let buf = "";
|
let buf = "";
|
||||||
const rawWasSet = process.stdin.isRaw;
|
const rawWasSet = process.stdin.isRaw;
|
||||||
if (process.stdin.isTTY) {
|
if (process.stdin.isTTY) {
|
||||||
@@ -171,7 +175,7 @@ async function promptSecret(label: string): Promise<string> {
|
|||||||
process.stdin.pause();
|
process.stdin.pause();
|
||||||
process.stdin.removeListener("data", onData);
|
process.stdin.removeListener("data", onData);
|
||||||
process.stdout.write("\n");
|
process.stdout.write("\n");
|
||||||
resolve(buf.trim());
|
fulfill(buf.trim());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (c === "\u007F" || c === "\b") {
|
if (c === "\u007F" || c === "\b") {
|
||||||
@@ -182,6 +186,9 @@ async function promptSecret(label: string): Promise<string> {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if (c === "\u0003") {
|
if (c === "\u0003") {
|
||||||
|
if (process.stdin.isTTY) {
|
||||||
|
process.stdin.setRawMode(rawWasSet);
|
||||||
|
}
|
||||||
process.exit(130);
|
process.exit(130);
|
||||||
}
|
}
|
||||||
buf += c;
|
buf += c;
|
||||||
@@ -205,19 +212,23 @@ async function fetchAvailableModels(
|
|||||||
signal: AbortSignal.timeout(10_000),
|
signal: AbortSignal.timeout(10_000),
|
||||||
});
|
});
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
|
setupDispatchLog("R5KH7WM3", `GET ${url} returned ${res.status}`);
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
const body = (await res.json()) as OpenAiModelsResponse;
|
const body = (await res.json()) as OpenAiModelsResponse;
|
||||||
if (!Array.isArray(body.data)) {
|
if (!Array.isArray(body.data)) {
|
||||||
return [];
|
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 =
|
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;
|
/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
|
return body.data
|
||||||
.map((m) => m.id)
|
.map((m) => m.id)
|
||||||
.filter((id) => !NON_CHAT_RE.test(id))
|
.filter((id) => !NON_CHAT_RE.test(id))
|
||||||
.sort();
|
.sort();
|
||||||
} catch {
|
} catch (e) {
|
||||||
|
setupDispatchLog("V8NQ4JT6", `fetch models failed: ${e instanceof Error ? e.message : String(e)}`);
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -242,6 +253,7 @@ async function collectInteractiveSetup(): Promise<Result<SetupCliArgs, string>>
|
|||||||
const choice = await promptLine(rl, `Choose [1-${presets.length + 1}]: `);
|
const choice = await promptLine(rl, `Choose [1-${presets.length + 1}]: `);
|
||||||
const choiceNum = Number.parseInt(choice, 10);
|
const choiceNum = Number.parseInt(choice, 10);
|
||||||
if (Number.isNaN(choiceNum) || choiceNum < 1 || choiceNum > presets.length + 1) {
|
if (Number.isNaN(choiceNum) || choiceNum < 1 || choiceNum > presets.length + 1) {
|
||||||
|
rl.close();
|
||||||
return err(`invalid choice: ${choice}`);
|
return err(`invalid choice: ${choice}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -331,7 +343,7 @@ async function collectInteractiveSetup(): Promise<Result<SetupCliArgs, string>>
|
|||||||
}
|
}
|
||||||
const candidate = wsPath === "" ? "./workflows" : wsPath;
|
const candidate = wsPath === "" ? "./workflows" : wsPath;
|
||||||
// Validate path before passing to cmdSetup.
|
// Validate path before passing to cmdSetup.
|
||||||
const resolved = resolve(process.cwd(), candidate);
|
const resolved = resolvePath(process.cwd(), candidate);
|
||||||
if (existsSync(resolved)) {
|
if (existsSync(resolved)) {
|
||||||
printCliWarn(`directory already exists: ${resolved}`);
|
printCliWarn(`directory already exists: ${resolved}`);
|
||||||
printCliLine("Please enter a different path, or type 'skip' to skip.");
|
printCliLine("Please enter a different path, or type 'skip' to skip.");
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
export { dispatchSetup } from "./dispatch.js";
|
export { dispatchSetup } from "./dispatch.js";
|
||||||
export { loadPresetProviders, type PresetProvider } from "./preset-providers.js";
|
export { loadPresetProviders } from "./preset-providers.js";
|
||||||
export { type CmdSetupSuccess, cmdSetup, printSetupSummary } from "./setup.js";
|
export { cmdSetup, printSetupSummary } from "./setup.js";
|
||||||
export type { SetupCliArgs } from "./types.js";
|
export type { CmdSetupSuccess, PresetProvider, SetupCliArgs } from "./types.js";
|
||||||
|
|||||||
@@ -3,11 +3,9 @@ import { join } from "node:path";
|
|||||||
|
|
||||||
import { parse as parseYaml } from "yaml";
|
import { parse as parseYaml } from "yaml";
|
||||||
|
|
||||||
export type PresetProvider = {
|
import type { PresetProvider } from "./types.js";
|
||||||
name: string;
|
|
||||||
label: string;
|
|
||||||
baseUrl: string;
|
|
||||||
};
|
|
||||||
|
|
||||||
type RawPresetEntry = {
|
type RawPresetEntry = {
|
||||||
name: unknown;
|
name: unknown;
|
||||||
|
|||||||
@@ -8,14 +8,6 @@
|
|||||||
label: OpenAI
|
label: OpenAI
|
||||||
baseUrl: https://api.openai.com/v1
|
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
|
- name: xai
|
||||||
label: xAI
|
label: xAI
|
||||||
baseUrl: https://api.x.ai/v1
|
baseUrl: https://api.x.ai/v1
|
||||||
@@ -52,7 +44,7 @@
|
|||||||
|
|
||||||
- name: glm
|
- name: glm
|
||||||
label: GLM (Zhipu AI)
|
label: GLM (Zhipu AI)
|
||||||
baseUrl: https://api.z.ai/api/paas/v4
|
baseUrl: https://open.bigmodel.cn/api/paas/v4
|
||||||
|
|
||||||
- name: stepfun
|
- name: stepfun
|
||||||
label: StepFun
|
label: StepFun
|
||||||
|
|||||||
@@ -9,18 +9,11 @@ import { createLogger } from "@uncaged/workflow-util";
|
|||||||
|
|
||||||
import { printCliLine } from "../../cli-output.js";
|
import { printCliLine } from "../../cli-output.js";
|
||||||
import { cmdInitWorkspace } from "../init/index.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" } });
|
const setupLog = createLogger({ sink: { kind: "stderr" } });
|
||||||
|
|
||||||
export type CmdSetupSuccess = {
|
|
||||||
registryPath: string;
|
|
||||||
provider: string;
|
|
||||||
defaultModel: string;
|
|
||||||
maxDepth: number;
|
|
||||||
supervisorInterval: number;
|
|
||||||
initWorkspaceRootPath: string | null;
|
|
||||||
};
|
|
||||||
|
|
||||||
function mergeWorkflowConfig(
|
function mergeWorkflowConfig(
|
||||||
prev: WorkflowConfig | null,
|
prev: WorkflowConfig | null,
|
||||||
|
|||||||
@@ -6,3 +6,18 @@ export type SetupCliArgs = {
|
|||||||
defaultModel: string;
|
defaultModel: string;
|
||||||
initWorkspaceName: string | null;
|
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;
|
||||||
|
};
|
||||||
|
|||||||
Reference in New Issue
Block a user