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..84439b65 100644 --- a/src/domain/models/validation_service.ts +++ b/src/domain/models/validation_service.ts @@ -482,10 +482,14 @@ export class DefaultModelValidationService implements ModelValidationService { return Promise.resolve(ValidationResult.pass("Global arguments")); } - // All fields are static, validate normally + // 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", - modelDef.globalArguments, + strictGlobalArgs, staticArgs, ); } @@ -510,7 +514,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"); +});