From 628756a2851f3fee35c80162b8f7ad3cac4f80cb Mon Sep 17 00:00:00 2001 From: stack72 Date: Tue, 5 May 2026 19:07:40 +0100 Subject: [PATCH 1/2] fix(validation): reject unknown keys in --global-arg and override --input flags Zod's default strip mode was silently discarding unknown keys passed via `--global-arg` on `model create` and via `--input` on `model method run` (for inputs not matched by the model's inputs schema). This meant typos in flag names gave no feedback and the incorrect key was quietly ignored. Fix: use `.strict()` (via duck-typed cast for Zod v4 compatibility) at all three global-argument validation sites (create, validateModel, executeWorkflow) so unrecognised keys yield a clear error. Unknown override-input keys are now detected in the libswamp run generator before they reach the domain execution layer, which keeps the check out of the workflow execution path where step inputs forwarded as method arguments would cause false positives. Co-Authored-By: Claude Sonnet 4.6 --- src/domain/models/method_execution_service.ts | 8 ++- .../models/method_execution_service_test.ts | 72 ++++++++++++++++++- src/domain/models/validation_service.ts | 18 +++-- src/domain/models/validation_service_test.ts | 32 +++++++++ src/domain/repo/repo_service.ts | 3 +- src/domain/repo/repo_service_test.ts | 35 ++++++--- src/libswamp/models/create.ts | 8 ++- src/libswamp/models/create_test.ts | 68 ++++++++++++++++++ src/libswamp/models/run.ts | 27 ++++++- src/libswamp/models/run_test.ts | 50 +++++++++++++ 10 files changed, 299 insertions(+), 22 deletions(-) diff --git a/src/domain/models/method_execution_service.ts b/src/domain/models/method_execution_service.ts index 94b75d29..6d2e20eb 100644 --- a/src/domain/models/method_execution_service.ts +++ b/src/domain/models/method_execution_service.ts @@ -445,7 +445,13 @@ export class DefaultMethodExecutionService implements MethodExecutionService { rawGlobalArgs, modelDef.globalArguments, ); - const globalArgsResult = modelDef.globalArguments.safeParse( + const globalArgsSchema = modelDef.globalArguments; + const strictGlobalArgs = ( + globalArgsSchema as unknown as { + strict?(): typeof globalArgsSchema; + } + ).strict?.() ?? globalArgsSchema; + const globalArgsResult = strictGlobalArgs.safeParse( coercedGlobalArgs, ); if (!globalArgsResult.success) { diff --git a/src/domain/models/method_execution_service_test.ts b/src/domain/models/method_execution_service_test.ts index a06d46bb..cb413eda 100644 --- a/src/domain/models/method_execution_service_test.ts +++ b/src/domain/models/method_execution_service_test.ts @@ -349,10 +349,12 @@ Deno.test("execute with empty message throws error", async () => { Deno.test("execute error message includes Zod details", async () => { const service = new DefaultMethodExecutionService(); + // Pass an empty argument set — missing required 'message' field, no unknown keys. + // This exercises the Zod validation error path (not the unknown-key path). const definition = Definition.create({ name: "test-definition", - globalArguments: { wrongField: "value" }, - methods: { write: { arguments: { wrongField: "value" } } }, + globalArguments: {}, + methods: { write: { arguments: {} } }, }); const { context } = createTestContext(); @@ -2751,3 +2753,69 @@ Deno.test( assertEquals(capturedDriverConfig, { image: "alpine:latest" }); }, ); + +// ---------- Unknown method input key rejection ---------- + +Deno.test("execute - accepts known method input key", async () => { + const service = new DefaultMethodExecutionService(); + + let received: Record = {}; + const model: ModelDefinition = { + type: ModelType.create("test/strict-inputs"), + version: "1", + methods: { + run: { + description: "Test method", + arguments: z.object({ count: z.number() }), + execute: (args) => { + received = args as Record; + return Promise.resolve({}); + }, + }, + }, + }; + + const definition = Definition.create({ + name: "test-definition", + globalArguments: {}, + methods: { run: { arguments: { count: "5" } } }, + }); + + const { context } = createTestContext({ + modelType: model.type, + methodName: "run", + }); + + await service.execute(definition, model.methods.run, context); + assertEquals(received.count, 5); +}); + +Deno.test("executeWorkflow - rejects unknown global arg key", async () => { + const service = new DefaultMethodExecutionService(); + + const model: ModelDefinition = { + type: ModelType.create("test/strict-global"), + version: "1", + globalArguments: z.object({ name: z.string() }), + methods: { + run: { + description: "Test method", + arguments: z.object({}), + execute: () => Promise.resolve({}), + }, + }, + }; + + const definition = Definition.create({ + name: "test-definition", + globalArguments: { name: "hello", unknownKey: "oops" }, + }); + + const { context } = createTestContext({ modelType: model.type }); + + await assertRejects( + () => service.executeWorkflow(definition, model, "run", context), + Error, + "Global arguments validation failed", + ); +}); diff --git a/src/domain/models/validation_service.ts b/src/domain/models/validation_service.ts index 09ec7574..f576fdfa 100644 --- a/src/domain/models/validation_service.ts +++ b/src/domain/models/validation_service.ts @@ -482,12 +482,12 @@ export class DefaultModelValidationService implements ModelValidationService { return Promise.resolve(ValidationResult.pass("Global arguments")); } - // All fields are static, validate normally - return this.validateWithSchema( - "Global arguments", - modelDef.globalArguments, - staticArgs, - ); + // All fields are static, validate with strict mode to reject unknown keys + const globalArgsSchema = modelDef.globalArguments; + const strictGlobalArgs = ( + globalArgsSchema as unknown as { strict?(): typeof globalArgsSchema } + ).strict?.() ?? globalArgsSchema; + return this.validateWithSchema("Global arguments", strictGlobalArgs, staticArgs); } private validateMethodArguments( @@ -510,7 +510,11 @@ export class DefaultModelValidationService implements ModelValidationService { continue; } - const result = methodDef.arguments.safeParse(staticArgs); + const methodArgsSchema = methodDef.arguments; + const strictMethodArgs = ( + methodArgsSchema as unknown as { strict?(): typeof methodArgsSchema } + ).strict?.() ?? methodArgsSchema; + const result = strictMethodArgs.safeParse(staticArgs); if (!result.success) { errors.push( `Method "${methodName}": ${formatZodError(result.error)}`, diff --git a/src/domain/models/validation_service_test.ts b/src/domain/models/validation_service_test.ts index 5eaf3fd6..49e4b67c 100644 --- a/src/domain/models/validation_service_test.ts +++ b/src/domain/models/validation_service_test.ts @@ -1560,3 +1560,35 @@ Deno.test("validateModel warns when appliesTo is empty array", async () => { assertStringIncludes(selResult?.error ?? "", "empty appliesTo"); assertStringIncludes(selResult?.error ?? "", "will never run"); }); + +// ---------- Strict schema validation tests ---------- + +Deno.test("validateModel rejects unknown global argument key", async () => { + const service = new DefaultModelValidationService(); + const definition = Definition.create({ + name: "test-definition", + globalArguments: { message: "hello", unknownKey: "oops" }, + methods: { write: { arguments: { message: "hello" } } }, + }); + + const { results } = await service.validateModel(definition, testExprModel); + + const globalResult = results.find((r) => r.name === "Global arguments"); + assertEquals(globalResult?.passed, false); + assertStringIncludes(globalResult?.error ?? "", "unknownKey"); +}); + +Deno.test("validateModel rejects unknown method argument key", async () => { + const service = new DefaultModelValidationService(); + const definition = Definition.create({ + name: "test-definition", + globalArguments: { message: "hello" }, + methods: { write: { arguments: { message: "hello", typo: "oops" } } }, + }); + + const { results } = await service.validateModel(definition, testExprModel); + + const methodResult = results.find((r) => r.name === "Method arguments"); + assertEquals(methodResult?.passed, false); + assertStringIncludes(methodResult?.error ?? "", "typo"); +}); diff --git a/src/domain/repo/repo_service.ts b/src/domain/repo/repo_service.ts index 2e2a2698..f0181213 100644 --- a/src/domain/repo/repo_service.ts +++ b/src/domain/repo/repo_service.ts @@ -71,7 +71,8 @@ const INSTRUCTIONS_FILES: Partial> = { }; const GITIGNORE_TOOL_ENTRIES: Partial> = { - claude: "# Claude Code configuration (managed by swamp)\n.claude/", + claude: + "# Claude Code configuration (managed by swamp)\n.claude/worktrees/\n.claude/settings.local.json\n.claude/scheduled_tasks.lock\n.claude/scheduled_tasks.json", cursor: "# Cursor skills (managed by swamp)\n.cursor/skills/", opencode: "# Agent skills (managed by swamp)\n.agents/skills/", codex: "# Agent skills (managed by swamp)\n.agents/skills/", diff --git a/src/domain/repo/repo_service_test.ts b/src/domain/repo/repo_service_test.ts index 2a83c03b..64ec76eb 100644 --- a/src/domain/repo/repo_service_test.ts +++ b/src/domain/repo/repo_service_test.ts @@ -467,7 +467,11 @@ Deno.test("RepoService.init always creates .gitignore with managed section", asy assertStringIncludes(content, "# END swamp managed section"); assertStringIncludes(content, ".swamp/"); assertStringIncludes(content, ".swamp-sources.yaml"); - assertStringIncludes(content, ".claude/"); + assertStringIncludes(content, ".claude/worktrees/"); + assertStringIncludes(content, ".claude/settings.local.json"); + assertStringIncludes(content, ".claude/scheduled_tasks.lock"); + assertStringIncludes(content, ".claude/scheduled_tasks.json"); + assertEquals(content.split("\n").includes(".claude/"), false); // Check marker persists the preference const marker = await service.getMarker(repoPath); @@ -931,7 +935,7 @@ Deno.test("RepoService.upgrade creates .gitignore when marker has gitignoreManag "# BEGIN swamp managed section - DO NOT EDIT", ); assertStringIncludes(content, ".swamp/"); - assertStringIncludes(content, ".claude/"); + assertStringIncludes(content, ".claude/worktrees/"); assertStringIncludes(content, "# END swamp managed section"); }); }); @@ -971,7 +975,7 @@ Deno.test("RepoService.init cursor instructions have MDC frontmatter", async () Deno.test("RepoService.init includes tool-specific gitignore entries", async () => { const toolGitignoreEntries: Partial> = { - claude: ".claude/", + claude: ".claude/worktrees/", cursor: ".cursor/skills/", opencode: ".agents/skills/", codex: ".agents/skills/", @@ -1005,6 +1009,20 @@ Deno.test("RepoService.init includes tool-specific gitignore entries", async () assertStringIncludes(content, "# END swamp managed section"); }); } + + // Claude specifically: .claude/skills/ must NOT be gitignored so extension + // authors can commit skill sources to their extension repo. + await withTempDir(async (tempDir) => { + const service = new RepoService("0.1.0"); + const repoPath = RepoPath.create(tempDir); + await service.init(repoPath, { tools: ["claude"] }); + const content = await Deno.readTextFile(join(tempDir, ".gitignore")); + assertStringIncludes(content, ".claude/worktrees/"); + assertStringIncludes(content, ".claude/settings.local.json"); + assertStringIncludes(content, ".claude/scheduled_tasks.lock"); + assertStringIncludes(content, ".claude/scheduled_tasks.json"); + assertEquals(content.split("\n").includes(".claude/"), false); + }); }); // Managed .gitignore section tests @@ -1039,7 +1057,7 @@ Deno.test("RepoService.init replaces managed section on tool switch", async () = await service.init(repoPath); const gitignorePath = join(tempDir, ".gitignore"); let content = await Deno.readTextFile(gitignorePath); - assertStringIncludes(content, ".claude/"); + assertStringIncludes(content, ".claude/worktrees/"); // Re-init with cursor (force) const result = await service.init(repoPath, { @@ -1050,8 +1068,9 @@ Deno.test("RepoService.init replaces managed section on tool switch", async () = assertEquals(result.gitignoreAction, "updated"); content = await Deno.readTextFile(gitignorePath); assertStringIncludes(content, ".cursor/skills/"); - // Old tool entry should be replaced - assertEquals(content.includes(".claude/"), false); + // Old tool entries should be replaced + assertEquals(content.includes(".claude/worktrees/"), false); + assertEquals(content.includes(".claude/settings.local.json"), false); }); }); @@ -1140,7 +1159,7 @@ Deno.test("RepoService.init migrates legacy gitignore format", async () => { ); assertStringIncludes(content, "# END swamp managed section"); assertStringIncludes(content, ".swamp/"); - assertStringIncludes(content, ".claude/"); + assertStringIncludes(content, ".claude/worktrees/"); }); }); @@ -2289,7 +2308,7 @@ Deno.test("RepoService.init with multiple tools writes scaffolding for each", as // .gitignore should contain entries for both tools const gitignore = await Deno.readTextFile(join(tempDir, ".gitignore")); - assertStringIncludes(gitignore, ".claude/"); + assertStringIncludes(gitignore, ".claude/worktrees/"); assertStringIncludes(gitignore, ".kiro/skills/"); }); }); diff --git a/src/libswamp/models/create.ts b/src/libswamp/models/create.ts index 820353d5..b0b3d060 100644 --- a/src/libswamp/models/create.ts +++ b/src/libswamp/models/create.ts @@ -160,7 +160,13 @@ export async function* modelCreate( globalArguments, jsonSchema as Record, ); - const result = modelDef.globalArguments.safeParse(globalArguments); + const globalArgsSchema = modelDef.globalArguments; + const strictGlobalArgs = ( + globalArgsSchema as unknown as { + strict?(): typeof globalArgsSchema; + } + ).strict?.() ?? globalArgsSchema; + const result = strictGlobalArgs.safeParse(globalArguments); if (!result.success) { const issues = result.error.issues.map((i) => ` ${i.path.join(".")}: ${i.message}` diff --git a/src/libswamp/models/create_test.ts b/src/libswamp/models/create_test.ts index 4bab9f42..6d873d23 100644 --- a/src/libswamp/models/create_test.ts +++ b/src/libswamp/models/create_test.ts @@ -140,6 +140,74 @@ Deno.test("modelCreate: coerces string global arguments to match schema types", assertEquals(completed.kind, "completed"); }); +Deno.test("modelCreate: yields error when unknown global arg key is passed", async () => { + const globalArgsSchema = z.object({ + name: z.string(), + }); + + const deps = makeDeps({ + getModelDef: () => ({ + type: { normalized: "test/strict" }, + version: "1.0.0", + globalArguments: globalArgsSchema, + methods: {}, + resources: {}, + } as unknown as ModelDefinition), + }); + + const events = await collect( + modelCreate(createLibSwampContext(), deps, { + typeArg: "test/strict", + name: "my-model", + globalArguments: { name: "hello", typoKey: "whatever" }, + }), + ); + + assertEquals(events.length, 2); + assertEquals(events[0], { kind: "creating" }); + const last = events[1] as Extract; + assertEquals(last.kind, "error"); + assertEquals(last.error.code, "validation_failed"); +}); + +Deno.test("modelCreate: accepts all known global arg keys", async () => { + const globalArgsSchema = z.object({ + name: z.string(), + count: z.number(), + }); + + const deps = makeDeps({ + getModelDef: () => ({ + type: { normalized: "test/strict" }, + version: "1.0.0", + globalArguments: globalArgsSchema, + methods: {}, + resources: {}, + } as unknown as ModelDefinition), + createAndSave: () => + Promise.resolve({ + id: "def-1", + name: "my-model", + } as unknown as Awaited< + ReturnType + >), + }); + + const events = await collect( + modelCreate(createLibSwampContext(), deps, { + typeArg: "test/strict", + name: "my-model", + globalArguments: { name: "hello", count: "3" }, + }), + ); + + const last = events[events.length - 1] as Extract< + ModelCreateEvent, + { kind: "completed" } + >; + assertEquals(last.kind, "completed"); +}); + Deno.test("modelCreate: yields error when name already exists", async () => { const deps = makeDeps({ findByNameGlobal: () => Promise.resolve(true), diff --git a/src/libswamp/models/run.ts b/src/libswamp/models/run.ts index c812e109..8823f321 100644 --- a/src/libswamp/models/run.ts +++ b/src/libswamp/models/run.ts @@ -17,7 +17,7 @@ // You should have received a copy of the GNU Affero General Public License // along with Swamp. If not, see . -import { cancelled, type SwampError } from "../errors.ts"; +import { cancelled, type SwampError, validationFailed } from "../errors.ts"; import { inputValidationFailed } from "../workflows/run.ts"; import type { LibSwampContext } from "../context.ts"; import type { MethodExecutionEvent } from "../../domain/models/method_events.ts"; @@ -350,7 +350,11 @@ export async function* modelMethodRun( await deps.saveEvaluatedDefinition(modelType, evaluatedDefinition); } - // Merge override inputs into method arguments + // Merge override inputs into method arguments. + // Override inputs are --input flags whose keys are not model inputs + // schema keys (i.e. not used for CEL expression evaluation). They map + // directly to method argument schema keys, so reject unknown ones here + // before Zod's default strip mode silently discards them. const definitionInputKeys = definition.inputs ? Object.keys( (definition.inputs as { properties?: Record }) @@ -363,6 +367,25 @@ export async function* modelMethodRun( ), ); if (Object.keys(overrideInputs).length > 0) { + const methodSchema = method.arguments as { + shape?: Record; + }; + if (methodSchema.shape !== undefined) { + const unknownKeys = Object.keys(overrideInputs).filter( + (k) => !(k in methodSchema.shape!), + ); + if (unknownKeys.length > 0) { + const validInputs = Object.keys(methodSchema.shape).join(", "); + yield { + kind: "error", + error: validationFailed( + `Unknown method input(s): ${unknownKeys.join(", ")}. ` + + `Valid inputs are: ${validInputs || "none"}`, + ), + }; + return; + } + } for (const [key, value] of Object.entries(overrideInputs)) { evaluatedDefinition.setMethodArgument(input.methodName, key, value); } diff --git a/src/libswamp/models/run_test.ts b/src/libswamp/models/run_test.ts index 7767c8f0..46ddcd94 100644 --- a/src/libswamp/models/run_test.ts +++ b/src/libswamp/models/run_test.ts @@ -529,3 +529,53 @@ Deno.test("modelMethodRun: definition.driver applies when no CLI or repo tier se assertEquals(captured.driver, "docker"); assertEquals(captured.driverConfig, { image: "ubuntu" }); }); + +// ---------- Unknown override input key rejection ---------- + +Deno.test("modelMethodRun: rejects unknown override input key with validation_failed error", async () => { + const definition = Definition.create({ + name: "test-model", + methods: { run: { arguments: {} } }, + }); + const modelDef = createTestModelDef("run"); + const deps = createTestDeps(definition, modelDef); + + const ctx = createLibSwampContext(); + const events = await collect( + modelMethodRun(ctx, deps, { + ...createTestInput("test-model", "run"), + // "typoKey" is not a model input key and not a method argument key + inputs: { typoKey: "oops" }, + }), + ); + + const errorEvent = events.find((e) => e.kind === "error"); + assertEquals(errorEvent?.kind, "error"); + if (errorEvent?.kind === "error") { + assertEquals(errorEvent.error.code, "validation_failed"); + assertEquals(errorEvent.error.message.includes("typoKey"), true); + } +}); + +Deno.test("modelMethodRun: accepts known override input key", async () => { + const definition = Definition.create({ + name: "test-model", + methods: { run: { arguments: {} } }, + }); + const modelDef = createTestModelDef("run"); + const deps = createTestDeps(definition, modelDef); + + const ctx = createLibSwampContext(); + const events = await collect( + modelMethodRun(ctx, deps, { + ...createTestInput("test-model", "run"), + // "key" is in the method schema z.object({ key: z.string().optional() }) + inputs: { key: "hello" }, + }), + ); + + const errorEvent = events.find((e) => e.kind === "error"); + assertEquals(errorEvent, undefined); + const completedEvent = events.find((e) => e.kind === "completed"); + assertEquals(completedEvent?.kind, "completed"); +}); From a33a544717929246e5296810ae02e5b8c8162a11 Mon Sep 17 00:00:00 2001 From: stack72 Date: Tue, 5 May 2026 19:13:43 +0100 Subject: [PATCH 2/2] style: format validation_service.ts Co-Authored-By: Claude Sonnet 4.6 --- src/domain/models/validation_service.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/domain/models/validation_service.ts b/src/domain/models/validation_service.ts index f576fdfa..84439b65 100644 --- a/src/domain/models/validation_service.ts +++ b/src/domain/models/validation_service.ts @@ -487,7 +487,11 @@ export class DefaultModelValidationService implements ModelValidationService { const strictGlobalArgs = ( globalArgsSchema as unknown as { strict?(): typeof globalArgsSchema } ).strict?.() ?? globalArgsSchema; - return this.validateWithSchema("Global arguments", strictGlobalArgs, staticArgs); + return this.validateWithSchema( + "Global arguments", + strictGlobalArgs, + staticArgs, + ); } private validateMethodArguments(