Compare commits

...

6 Commits

Author SHA1 Message Date
xiaoju 76fab22827 fix: explicitly forbid extra frontmatter fields in output format instruction
buildOutputFormatInstruction now includes explicit language telling agents to
output ONLY schema-defined fields and to focus on their role's deliverable.

Fixes #394
2026-05-22 10:49:04 +00:00
xiaomo 669875fb46 Merge pull request 'feat: validate model connectivity during uwf setup' (#392) from feat/335-setup-validate-model into main 2026-05-22 10:32:01 +00:00
xiaoju 6d94be34a9 feat: validate model connectivity during uwf setup
Send a test completion request after configuration to verify the model
is reachable. If validation fails, warn the user and suggest trying a
different model or checking their settings.

Fixes #335
2026-05-22 10:30:39 +00:00
xiaomo d95fe45a3d Merge pull request 'feat: add --count/-c flag to uwf thread step' (#390) from feat/373-thread-step-count into main 2026-05-22 10:11:13 +00:00
xiaoju b9252b5ce2 fix: dynamic frontmatter instruction from role schema (closes #389) 2026-05-22 10:03:56 +00:00
xiaoju 45dacf540b feat: thread step --count/-c <number> to run multiple steps
Add --count/-c flag to 'uwf thread step' for running N steps in one
invocation, stopping early if $END is reached.

- cmdThreadStep now loops up to count times, delegates to cmdThreadStepOnce
- CLI parses -c/--count, defaults to 1 (backward compatible single output)
- Validation rejects 0, negative, and non-integer counts
- 7 new tests covering CLI parsing and count validation

Fixes #373

Co-authored-by: uwf-hermes (solve-issue workflow)
2026-05-22 08:06:26 +00:00
8 changed files with 414 additions and 6 deletions
+83
View File
@@ -0,0 +1,83 @@
# Test Spec: uwf setup model connectivity validation (#335)
## Context
File: `packages/cli-workflow/src/commands/setup.ts`
Test file: `packages/cli-workflow/src/__tests__/setup-validate.test.ts`
After `cmdSetup` writes config, it should send a test chat completion request to verify the configured model is reachable. If validation fails, warn the user (don't abort — config is already saved).
## Implementation Notes
- Add a `validateModel(baseUrl, apiKey, model)` function that sends a minimal chat completion request (`POST /chat/completions` with `messages: [{role:"user",content:"hi"}]`, `max_tokens: 1`)
- Returns `Result<void, string>` — ok if 2xx response, error with reason string otherwise
- Use `AbortSignal.timeout(15_000)` for the request
- Both `cmdSetup` and `cmdSetupInteractive` should call it after saving config
- `cmdSetup` returns validation result in its return object: `{ ...existing, validation: { ok: true } | { ok: false, error: string } }`
- `cmdSetupInteractive` prints a warning to console if validation fails, success message if it passes
- Use the project logger (`createLogger`) — no raw `console.log` except in interactive CLI output (per CLAUDE.md)
## Test Cases (vitest)
### 1. `validateModel` — success path
- Mock `fetch` to return `{ status: 200, ok: true, json: () => ({}) }`
- Call `validateModel(baseUrl, apiKey, model)`
- Assert returns `{ ok: true, value: undefined }`
- Assert fetch was called with correct URL (`${baseUrl}/chat/completions`), correct headers (`Authorization: Bearer ${apiKey}`), correct body (model, messages, max_tokens: 1)
### 2. `validateModel` — HTTP error (401 unauthorized)
- Mock `fetch` to return `{ status: 401, ok: false, statusText: "Unauthorized" }`
- Call `validateModel(baseUrl, apiKey, model)`
- Assert returns `{ ok: false, error: <string containing "401"> }`
### 3. `validateModel` — HTTP error (404 model not found)
- Mock `fetch` to return `{ status: 404, ok: false, statusText: "Not Found" }`
- Assert returns `{ ok: false, error: <string containing "404"> }`
### 4. `validateModel` — network timeout
- Mock `fetch` to throw `DOMException` with name `AbortError`
- Assert returns `{ ok: false, error: <string containing "timeout" or "unreachable"> }`
### 5. `validateModel` — network error (DNS failure, connection refused)
- Mock `fetch` to throw `TypeError("fetch failed")`
- Assert returns `{ ok: false, error: <string mentioning connectivity> }`
### 6. `cmdSetup` — includes validation result on success
- Mock global `fetch` for `/chat/completions` to succeed
- Call `cmdSetup({ provider, baseUrl, apiKey, model, storageRoot })`
- Assert returned object has `validation: { ok: true, value: undefined }`
- Assert config files are still written (existing behavior preserved)
### 7. `cmdSetup` — includes validation result on failure (config still saved)
- Mock global `fetch` for `/chat/completions` to return 401
- Call `cmdSetup({ ... })`
- Assert returned object has `validation: { ok: false, error: ... }`
- Assert `config.yaml` and `.env` are still written (validation failure doesn't prevent saving)
### 8. `cmdSetupInteractive` — prints success message on validation pass
- Mock `fetch` for both `/models` and `/chat/completions` to succeed
- Mock stdin to provide valid selections
- Capture console output
- Assert output contains a success message like "Model verified" or "✓"
### 9. `cmdSetupInteractive` — prints warning on validation failure
- Mock `fetch`: `/models` succeeds, `/chat/completions` returns 401
- Mock stdin for valid selections
- Capture console output
- Assert output contains a warning about model not being reachable and suggests trying a different model
### 10. `validateModel` — request body correctness
- Mock `fetch` to capture the request body
- Call `validateModel(baseUrl, apiKey, "test-model")`
- Assert body is `{ model: "test-model", messages: [{role: "user", content: "hi"}], max_tokens: 1 }`
## Export Requirements
- `validateModel` must be exported (for direct unit testing)
- Signature: `async function validateModel(baseUrl: string, apiKey: string, model: string): Promise<Result<void, string>>`
- `Result` type: `{ ok: true; value: T } | { ok: false; error: E }` (project convention)
## Files to Create/Modify
- **New**: `packages/cli-workflow/src/__tests__/setup-validate.test.ts` — all test cases above
- **Modify**: `packages/cli-workflow/src/commands/setup.ts` — add `validateModel`, integrate into `cmdSetup` and `cmdSetupInteractive`
@@ -0,0 +1,150 @@
import { mkdtemp, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import { cmdSetup, validateModel } from "../commands/setup.js";
describe("validateModel", () => {
const BASE_URL = "https://api.example.com/v1";
const API_KEY = "sk-test-key";
const MODEL = "test-model";
afterEach(() => {
vi.restoreAllMocks();
});
test("success path — returns ok on 200", async () => {
const mockFetch = vi
.spyOn(globalThis, "fetch")
.mockResolvedValue(new Response(JSON.stringify({}), { status: 200 }));
const result = await validateModel(BASE_URL, API_KEY, MODEL);
expect(result).toEqual({ ok: true, value: undefined });
expect(mockFetch).toHaveBeenCalledOnce();
const [url, opts] = mockFetch.mock.calls[0]!;
expect(url).toBe(`${BASE_URL}/chat/completions`);
expect((opts as RequestInit).headers).toEqual(
expect.objectContaining({ Authorization: `Bearer ${API_KEY}` }),
);
const body = JSON.parse((opts as RequestInit).body as string);
expect(body).toEqual({
model: MODEL,
messages: [{ role: "user", content: "hi" }],
max_tokens: 1,
});
});
test("HTTP 401 — returns error containing 401", async () => {
vi.spyOn(globalThis, "fetch").mockResolvedValue(
new Response("Unauthorized", { status: 401, statusText: "Unauthorized" }),
);
const result = await validateModel(BASE_URL, API_KEY, MODEL);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toContain("401");
}
});
test("HTTP 404 — returns error containing 404", async () => {
vi.spyOn(globalThis, "fetch").mockResolvedValue(
new Response("Not Found", { status: 404, statusText: "Not Found" }),
);
const result = await validateModel(BASE_URL, API_KEY, MODEL);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toContain("404");
}
});
test("network timeout — returns error mentioning timeout", async () => {
const err = new DOMException("signal timed out", "AbortError");
vi.spyOn(globalThis, "fetch").mockRejectedValue(err);
const result = await validateModel(BASE_URL, API_KEY, MODEL);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error.toLowerCase()).toMatch(/timeout|timed out/);
}
});
test("network error (DNS/connection) — returns error mentioning connectivity", async () => {
vi.spyOn(globalThis, "fetch").mockRejectedValue(new TypeError("fetch failed"));
const result = await validateModel(BASE_URL, API_KEY, MODEL);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error.toLowerCase()).toMatch(/connect|reach|network/);
}
});
test("request body correctness", async () => {
const mockFetch = vi
.spyOn(globalThis, "fetch")
.mockResolvedValue(new Response(JSON.stringify({}), { status: 200 }));
await validateModel(BASE_URL, API_KEY, "my-special-model");
const body = JSON.parse((mockFetch.mock.calls[0]![1] as RequestInit).body as string);
expect(body).toEqual({
model: "my-special-model",
messages: [{ role: "user", content: "hi" }],
max_tokens: 1,
});
});
});
describe("cmdSetup with validation", () => {
let storageRoot: string;
beforeEach(async () => {
storageRoot = await mkdtemp(join(tmpdir(), "uwf-setup-validate-"));
});
afterEach(async () => {
vi.restoreAllMocks();
await rm(storageRoot, { recursive: true, force: true });
});
const setupArgs = () => ({
provider: "testprovider",
baseUrl: "https://api.test.com/v1",
apiKey: "sk-test",
model: "test-model",
storageRoot,
});
test("includes validation result on success", async () => {
vi.spyOn(globalThis, "fetch").mockResolvedValue(
new Response(JSON.stringify({}), { status: 200 }),
);
const result = await cmdSetup(setupArgs());
expect(result.validation).toEqual({ ok: true, value: undefined });
// Config files should still be written
expect(result.configPath).toBeTruthy();
expect(result.envPath).toBeTruthy();
});
test("includes validation failure — config still saved", async () => {
vi.spyOn(globalThis, "fetch").mockResolvedValue(
new Response("Unauthorized", { status: 401, statusText: "Unauthorized" }),
);
const result = await cmdSetup(setupArgs());
expect(result.validation).toBeDefined();
expect((result.validation as { ok: boolean }).ok).toBe(false);
// Config files should still be written despite validation failure
expect(result.configPath).toBeTruthy();
expect(result.envPath).toBeTruthy();
});
});
@@ -0,0 +1,71 @@
import { execFileSync } from "node:child_process";
import { join } from "node:path";
import { describe, expect, test } from "vitest";
const CLI_PATH = join(import.meta.dirname, "..", "cli.js");
function runCli(args: string[]): { stdout: string; stderr: string; exitCode: number } {
try {
const stdout = execFileSync("bun", ["run", CLI_PATH, ...args], {
encoding: "utf8",
env: { ...process.env, WORKFLOW_STORAGE_ROOT: "/tmp/uwf-test-nonexistent" },
stdio: ["ignore", "pipe", "pipe"],
});
return { stdout, stderr: "", exitCode: 0 };
} catch (e: unknown) {
const err = e as NodeJS.ErrnoException & { stdout?: string; stderr?: string; status?: number };
return {
stdout: err.stdout ?? "",
stderr: err.stderr ?? "",
exitCode: err.status ?? 1,
};
}
}
describe("thread step --count CLI parsing", () => {
test("--help shows -c/--count option", () => {
const result = runCli(["thread", "step", "--help"]);
expect(result.stdout).toContain("--count");
expect(result.stdout).toContain("-c");
});
test("description says 'one or more steps'", () => {
const result = runCli(["thread", "step", "--help"]);
expect(result.stdout).toContain("one or more steps");
});
});
describe("cmdThreadStep count logic", () => {
test("count=0 fails with validation error", () => {
const result = runCli(["thread", "step", "FAKE_THREAD_ID", "-c", "0"]);
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("positive integer");
});
test("negative count fails with validation error", () => {
const result = runCli(["thread", "step", "FAKE_THREAD_ID", "-c", "-1"]);
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("positive integer");
});
test("non-integer count fails with validation error", () => {
const result = runCli(["thread", "step", "FAKE_THREAD_ID", "-c", "1.5"]);
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("positive integer");
});
test("count=1 is the default (no -c flag)", () => {
// Without -c, it should attempt to run 1 step (failing on missing thread, not on count validation)
const result = runCli(["thread", "step", "FAKE_THREAD_ID"]);
expect(result.exitCode).not.toBe(0);
// Should NOT contain "positive integer" error — should fail on thread lookup instead
expect(result.stderr).not.toContain("positive integer");
});
test("count=3 passes validation (fails on thread lookup)", () => {
const result = runCli(["thread", "step", "FAKE_THREAD_ID", "-c", "3"]);
expect(result.exitCode).not.toBe(0);
// Should NOT contain "positive integer" error — should fail on thread/storage lookup
expect(result.stderr).not.toContain("positive integer");
});
});
+10 -4
View File
@@ -109,15 +109,21 @@ thread
thread
.command("step")
.description("Execute one step")
.description("Execute one or more steps")
.argument("<thread-id>", "Thread ULID")
.option("--agent <cmd>", "Override agent command")
.action((threadId: string, opts: { agent: string | undefined }) => {
.option("-c, --count <number>", "Number of steps to run (default: 1)")
.action((threadId: string, opts: { agent: string | undefined; count: string | undefined }) => {
const storageRoot = resolveStorageRoot();
runAction(async () => {
const agentOverride = opts.agent ?? null;
const result = await cmdThreadStep(storageRoot, threadId, agentOverride);
writeOutput(result);
const count = opts.count !== undefined ? Number(opts.count) : 1;
const results = await cmdThreadStep(storageRoot, threadId, agentOverride, count);
if (results.length === 1) {
writeOutput(results[0]);
} else {
writeOutput(results);
}
});
});
+55 -2
View File
@@ -2,9 +2,45 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { stdin as input, stdout as output } from "node:process";
import { createInterface } from "node:readline/promises";
import type { Result } from "@uncaged/workflow-util";
import { parse, stringify } from "yaml";
/**
* Send a minimal chat completion request to verify the model is reachable.
* Returns ok on 2xx, error with reason string otherwise.
*/
export async function validateModel(
baseUrl: string,
apiKey: string,
model: string,
): Promise<Result<void, string>> {
try {
const url = `${baseUrl.replace(/\/+$/, "")}/chat/completions`;
const res = await fetch(url, {
method: "POST",
headers: {
Authorization: `Bearer ${apiKey}`,
"Content-Type": "application/json",
},
body: JSON.stringify({
model,
messages: [{ role: "user", content: "hi" }],
max_tokens: 1,
}),
signal: AbortSignal.timeout(15_000),
});
if (!res.ok) {
return { ok: false, error: `HTTP ${res.status} ${res.statusText}` };
}
return { ok: true, value: undefined };
} catch (err: unknown) {
if (err instanceof DOMException && err.name === "AbortError") {
return { ok: false, error: "Request timed out — model endpoint unreachable" };
}
return { ok: false, error: `Network error — could not reach endpoint (${String(err)})` };
}
}
/**
* Preset provider list — embedded to avoid runtime YAML loading dependency.
* Keep in sync with providers.yaml in cli-workflow.
@@ -163,12 +199,16 @@ export async function cmdSetup(args: SetupArgs): Promise<Record<string, unknown>
envData[envName] = args.apiKey;
saveEnvFile(envPath, envData);
// Validate model connectivity
const validation = await validateModel(args.baseUrl, args.apiKey, args.model);
return {
configPath,
envPath,
provider: args.provider,
model: args.model,
defaultAgent: merged.defaultAgent,
validation,
};
}
@@ -328,7 +368,7 @@ export async function cmdSetupInteractive(storageRoot: string): Promise<Record<s
console.log(`${providerName}/${model}\n`);
await cmdSetup({
const setupResult = await cmdSetup({
provider: providerName,
baseUrl,
apiKey,
@@ -336,6 +376,19 @@ export async function cmdSetupInteractive(storageRoot: string): Promise<Record<s
storageRoot,
});
// Show validation result
if (setupResult.validation && typeof setupResult.validation === "object") {
const v = setupResult.validation as { ok: boolean; error?: string };
if (v.ok) {
console.log("✓ Model verified — connection successful.\n");
} else {
console.log(`\n⚠ Warning: Could not reach model — ${v.error}`);
console.log(
" Config saved, but you may want to try a different model or check your API key.\n",
);
}
}
console.log("Setup complete! Get started:\n");
console.log(" uwf workflow put <workflow.yaml> Register a workflow");
console.log(' uwf thread start <name> -p "..." Start a thread');
@@ -673,6 +673,27 @@ export async function cmdThreadStep(
storageRoot: string,
threadId: ThreadId,
agentOverride: string | null,
count: number,
): Promise<StepOutput[]> {
if (count < 1 || !Number.isInteger(count)) {
fail(`--count must be a positive integer, got: ${count}`);
}
const results: StepOutput[] = [];
for (let i = 0; i < count; i++) {
const result = await cmdThreadStepOnce(storageRoot, threadId, agentOverride);
results.push(result);
if (result.done) {
break;
}
}
return results;
}
async function cmdThreadStepOnce(
storageRoot: string,
threadId: ThreadId,
agentOverride: string | null,
): Promise<StepOutput> {
const index = await loadThreadsIndex(storageRoot);
const headHash = index[threadId];
@@ -141,6 +141,28 @@ describe("buildOutputFormatInstruction", () => {
expect(result).toContain("shared: <string> # required");
});
test("explicitly forbids extra frontmatter fields", () => {
const result = buildOutputFormatInstruction(PLANNER_SCHEMA);
expect(result).toMatch(/\b(only|exclusively)\b.*fields/i);
expect(result).toMatch(/do not add (extra|additional|other) fields/i);
});
test("forbids extra fields even for empty schema", () => {
const result = buildOutputFormatInstruction({});
expect(result).toMatch(/do not add (extra|additional|other) fields/i);
});
test("forbids extra fields for anyOf/oneOf schemas", () => {
const schema = {
anyOf: [
{ type: "object", properties: { alpha: { type: "string" } } },
{ type: "object", properties: { beta: { type: "number" } } },
],
};
const result = buildOutputFormatInstruction(schema);
expect(result).toMatch(/do not add (extra|additional|other) fields/i);
});
test("includes focus reminder about role scope", () => {
const result = buildOutputFormatInstruction({});
expect(result).toContain("Focus exclusively on YOUR role");
@@ -191,5 +191,7 @@ Your meta output must satisfy these fields:
${fieldList}
Output ONLY the fields listed above. Do not add extra fields that are not specified in the schema.
Focus exclusively on YOUR role's deliverable. Do not perform actions outside your role's scope.`;
}