Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion skills/rig/rig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1815,7 +1815,8 @@ function validateSchema(value: unknown, schema: Schema, path: string, optional:
if (typeof value !== "string") return bad(path, "string", value);
const { minLength, format } = schema as StringSchema;
if (minLength !== undefined && value.length < minLength) {
return { ok: false, error: `${path}: expected string with minLength ${minLength}, got empty string` };
const gotDesc = value.length === 0 ? "empty string" : `string of length ${value.length}`;
return { ok: false, error: `${path}: expected string with minLength ${minLength}, got ${gotDesc}` };
}
if (format === "uri") {
try { new URL(value); } catch { return { ok: false, error: `${path}: expected a valid URL, got ${JSON.stringify(value)}` }; }
Expand Down
21 changes: 20 additions & 1 deletion src/rig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1338,11 +1338,30 @@ describe("s.nonEmptyString", () => {
expect(result.ok).toBe(true);
});

it("rejects empty strings", () => {
it("rejects empty strings with 'empty string' in message", () => {
const result = analyzeResponse(JSON.stringify(""), s.nonEmptyString, "test", 1);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error.message).toContain("minLength");
expect(result.error.message).toContain("empty string");
}
});

it("rejects short non-empty strings with actual length in message", () => {
// s.object with a string field that has minLength 5 via s.string-based shape
const schema = s.object({ code: s.nonEmptyString });
// Provide a non-empty but 2-char value to trigger the too-short branch
// We need a schema with minLength > 2; build one from scratch via toJsonSchema round-trip isn't possible,
// so verify via analyzeResponse with a direct schema literal accepted by the public API.
// Use s.nonEmptyString (minLength:1) against "" for the empty branch,
// and build a custom schema object for minLength > 1:
const minLen5Schema = { type: "string" as const, minLength: 5 };
const wrappedSchema = s.object({ code: minLen5Schema as ReturnType<typeof s.string> });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The test uses minLen5Schema as ReturnType<typeof s.string> to force a raw schema object past TypeScript — a type-unsound workaround that the inline comment itself flags as a limitation.

If s.string does not support a minLength argument through the public API, that gap is worth addressing. Using a type cast here means the test validates behaviour that callers cannot legally trigger through the public API.

💡 Cleaner alternatives

Option A — expose a minLength option on s.string (or a s.string({ minLength: N }) overload) so the test is type-safe:

const schema = s.object({ code: s.string({ minLength: 5 }) });

Option B — if this path is intentionally internal, test validateSchema directly (if exported) without going through analyzeResponse with a cast:

const result = validateSchema("ab", { type: "string", minLength: 5 }, "code", false);
expect(result).toEqual({ ok: false, error: "code: expected string with minLength 5, got string of length 2" });

Either option removes the cast, keeps the test type-safe, and makes the test read as a specification of real, reachable behaviour.

const result = analyzeResponse(JSON.stringify({ code: "ab" }), wrappedSchema, "test", 1);
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error.message).toContain("minLength 5");
expect(result.error.message).toContain("length 2");
}
});
});
Expand Down