fix: address PR #111 review feedback

- Extract validateWorkspaceSegment to commands/init/validate.ts
- Unify splitProviderModelRef in config/, used by both resolve-model and registry-normalize
- Warn on missing models.default during parse (tag Z2KP9NWQ)
This commit is contained in:
2026-05-08 02:14:20 +00:00
parent aa01283ce1
commit 1f1128ff4a
7 changed files with 48 additions and 61 deletions
@@ -13,19 +13,7 @@ import {
templateTsconfigJson, templateTsconfigJson,
} from "./templates.js"; } from "./templates.js";
import type { CmdInitTemplateSuccess } from "./types.js"; import type { CmdInitTemplateSuccess } from "./types.js";
import { validateWorkspaceSegment } from "./validate.js";
function validateWorkspaceSegment(name: string): Result<void, string> {
if (name.length === 0) {
return err("workspace name must not be empty");
}
if (name === "." || name === "..") {
return err("invalid workspace name");
}
if (name.includes("/") || name.includes("\\")) {
return err("workspace name must not contain path separators");
}
return ok(undefined);
}
function hasTemplatesWorkspaceGlob(workspaces: unknown): boolean { function hasTemplatesWorkspaceGlob(workspaces: unknown): boolean {
return Array.isArray(workspaces) && workspaces.includes("templates/*"); return Array.isArray(workspaces) && workspaces.includes("templates/*");
@@ -0,0 +1,15 @@
import { err, ok, type Result } from "@uncaged/workflow";
/** Validates a single path segment for workspace / template names (no separators, not `.` / `..`). */
export function validateWorkspaceSegment(name: string): Result<void, string> {
if (name.length === 0) {
return err("workspace name must not be empty");
}
if (name === "." || name === "..") {
return err("invalid workspace name");
}
if (name.includes("/") || name.includes("\\")) {
return err("workspace name must not contain path separators");
}
return ok(undefined);
}
@@ -5,19 +5,7 @@ import { err, ok, type Result } from "@uncaged/workflow";
import { pathExists } from "../../fs-utils.js"; import { pathExists } from "../../fs-utils.js";
import type { CmdInitWorkspaceSuccess } from "./types.js"; import type { CmdInitWorkspaceSuccess } from "./types.js";
import { validateWorkspaceSegment } from "./validate.js";
function validateWorkspaceSegment(name: string): Result<void, string> {
if (name.length === 0) {
return err("workspace name must not be empty");
}
if (name === "." || name === "..") {
return err("invalid workspace name");
}
if (name.includes("/") || name.includes("\\")) {
return err("workspace name must not contain path separators");
}
return ok(undefined);
}
function rootPackageJson(workspaceName: string): string { function rootPackageJson(workspaceName: string): string {
return `${JSON.stringify( return `${JSON.stringify(
+1
View File
@@ -1,2 +1,3 @@
export { resolveModel } from "./resolve-model.js"; export { resolveModel } from "./resolve-model.js";
export { splitProviderModelRef } from "./split-provider-model-ref.js";
export type { ProviderConfig, ResolvedModel } from "./types.js"; export type { ProviderConfig, ResolvedModel } from "./types.js";
+1 -15
View File
@@ -1,22 +1,8 @@
import type { WorkflowConfig } from "../registry/index.js"; import type { WorkflowConfig } from "../registry/index.js";
import { err, ok, type Result } from "../util/index.js"; import { err, ok, type Result } from "../util/index.js";
import { splitProviderModelRef } from "./split-provider-model-ref.js";
import type { ResolvedModel } from "./types.js"; import type { ResolvedModel } from "./types.js";
function splitProviderModelRef(
ref: string,
): Result<{ providerName: string; modelName: string }, string> {
const idx = ref.indexOf("/");
if (idx <= 0 || idx === ref.length - 1) {
return err(`invalid model reference "${ref}": expected providerName/modelName`);
}
const providerName = ref.slice(0, idx);
const modelName = ref.slice(idx + 1);
if (providerName === "" || modelName === "") {
return err(`invalid model reference "${ref}": expected providerName/modelName`);
}
return ok({ providerName, modelName });
}
/** Resolves scene → provider endpoint + model using {@link WorkflowConfig.providers} and {@link WorkflowConfig.models}. */ /** Resolves scene → provider endpoint + model using {@link WorkflowConfig.providers} and {@link WorkflowConfig.models}. */
export function resolveModel(config: WorkflowConfig, scene: string): Result<ResolvedModel, string> { export function resolveModel(config: WorkflowConfig, scene: string): Result<ResolvedModel, string> {
const models = config.models; const models = config.models;
@@ -0,0 +1,17 @@
import { err, ok, type Result } from "../util/index.js";
/** Parses `providerName/modelName` references used in {@link WorkflowConfig.models}. */
export function splitProviderModelRef(
ref: string,
): Result<{ providerName: string; modelName: string }, string> {
const idx = ref.indexOf("/");
if (idx <= 0 || idx === ref.length - 1) {
return err(`invalid model reference "${ref}": expected providerName/modelName`);
}
const providerName = ref.slice(0, idx);
const modelName = ref.slice(idx + 1);
if (providerName === "" || modelName === "") {
return err(`invalid model reference "${ref}": expected providerName/modelName`);
}
return ok({ providerName, modelName });
}
@@ -1,5 +1,5 @@
import type { ProviderConfig } from "../config/index.js"; import { type ProviderConfig, splitProviderModelRef } from "../config/index.js";
import { err, ok, type Result } from "../util/index.js"; import { createLogger, err, ok, type Result } from "../util/index.js";
import type { import type {
WorkflowConfig, WorkflowConfig,
WorkflowHistoryEntry, WorkflowHistoryEntry,
@@ -7,6 +7,8 @@ import type {
WorkflowRegistryFile, WorkflowRegistryFile,
} from "./types.js"; } from "./types.js";
const registryNormalizeLog = createLogger({ sink: { kind: "stderr" } });
function resolveRegistryApiKey(raw: string, ctx: string): Result<string, Error> { function resolveRegistryApiKey(raw: string, ctx: string): Result<string, Error> {
if (raw.startsWith("env:")) { if (raw.startsWith("env:")) {
const name = raw.slice("env:".length); const name = raw.slice("env:".length);
@@ -22,22 +24,6 @@ function resolveRegistryApiKey(raw: string, ctx: string): Result<string, Error>
return ok(raw); return ok(raw);
} }
function parseModelProviderRef(
ref: string,
ctx: string,
): Result<{ providerName: string; modelName: string }, Error> {
const idx = ref.indexOf("/");
if (idx <= 0 || idx === ref.length - 1) {
return err(new Error(`${ctx}: expected providerName/modelName, got "${ref}"`));
}
const providerName = ref.slice(0, idx);
const modelName = ref.slice(idx + 1);
if (providerName === "" || modelName === "") {
return err(new Error(`${ctx}: expected providerName/modelName, got "${ref}"`));
}
return ok({ providerName, modelName });
}
function normalizeProviderEntry(name: string, entryRaw: unknown): Result<ProviderConfig, Error> { function normalizeProviderEntry(name: string, entryRaw: unknown): Result<ProviderConfig, Error> {
if (name === "") { if (name === "") {
return err(new Error("config.providers must not contain an empty provider name")); return err(new Error("config.providers must not contain an empty provider name"));
@@ -96,9 +82,9 @@ function normalizeModels(
return err(new Error(`config.models.${scene} must be a non-empty string (provider/model)`)); return err(new Error(`config.models.${scene} must be a non-empty string (provider/model)`));
} }
const ctx = `config.models.${scene}`; const ctx = `config.models.${scene}`;
const parsed = parseModelProviderRef(refRaw, ctx); const parsed = splitProviderModelRef(refRaw);
if (!parsed.ok) { if (!parsed.ok) {
return parsed; return err(new Error(`${ctx}: ${parsed.error}`));
} }
if (!providerKeys.has(parsed.value.providerName)) { if (!providerKeys.has(parsed.value.providerName)) {
return err( return err(
@@ -109,6 +95,12 @@ function normalizeModels(
} }
models[scene] = refRaw; models[scene] = refRaw;
} }
if (!Object.hasOwn(models, "default")) {
registryNormalizeLog(
"Z2KP9NWQ",
'registry config: models mapping has no "default" key; scenes without explicit model mappings may fail at resolveModel',
);
}
return ok(models); return ok(models);
} }