fix(workflows): validate honors method args set in model definition (swamp-club#359)

[keeb]

Summary

• swamp workflow validate falsely reported Missing required inputs: <arg> for model_method steps whose argument was supplied by the model definition (methods.<method>.arguments or globalArguments) instead of the step's inputs: block.
• Validation check Fix PR merges #8 inspected only the step inputs: and ignored definition-supplied args, even though the runtime executor merges them as fallbacks — workflows ran fine; only validate was wrong.
• Fixes swamp-club#359.

Approach

• Enriches the MethodResolution (resolved variant) with an optional definitionProvidedArgs: string[].
• createModelMethodResolver populates it from definition.getMethodArguments(method) ∪ definition.globalArguments, mirroring the runtime merge in DefaultMethodExecutionService.execute.
• validateModelMethodInputs unions step inputs with definitionProvidedArgs when computing the missing-arg set.
• Optional field with ?? [] fallback preserves backward compatibility for any existing MethodResolution constructions.

Test Plan

• 5 new unit tests in validation_service_test.ts covering: definition supplies all, partial split with step inputs, partial unsatisfied (error names only the truly-missing key), step+definition overlap, and legacy-mock backward compat.
• End-to-end repro in /tmp/swamp-repro-issue-359 (command/shell model with methods.execute.arguments.run set, workflow step with no inputs:): swamp workflow validate now passes 8/8 and swamp workflow run still succeeds.
• Negative repro (no definition args, no step inputs): still correctly fails with Missing required inputs: run.
• deno check, deno lint, deno fmt, deno run test (5952 passed, 0 failed), deno run compile — all green.

Follow-up

• Will file a UAT issue in systeminit/swamp-uat to cover this scenario in tests/cli/workflow/validate_test.ts.

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR is a pure internal bug fix. No flags, help text, renderers, output shapes, or JSON structures were changed. The only user-visible effect is the correct behavior: swamp workflow validate no longer emits a false Missing required inputs: <arg> error when the argument is already supplied by the model definition, making validate consistent with run. Error message format, exit codes, and both log/json output modes are unaffected.

[github-actions]

Code Review

Blocking Issues

None.

Suggestions

None — this is a clean, well-scoped fix.

Summary: The PR correctly fixes the false-positive Missing required inputs error in swamp workflow validate by enriching MethodResolution with definitionProvidedArgs and unioning them with step-level inputs when computing the missing-arg set. The approach mirrors the runtime merge in DefaultMethodExecutionService.execute.

What I checked:

• DDD conformance: MethodResolution is a domain value type enriched with an optional field. The resolver port stays in the domain; the infrastructure populates it in createModelMethodResolver. Clean separation.
• Libswamp import boundary: validate.ts is libswamp-internal code importing from domain paths — allowed. MethodResolution is not re-exported from mod.ts (correctly, since it's an internal contract between the domain service and its port implementation).
• Runtime parity: Verified the resolver populates definitionProvidedArgs from the same sources (definition.getMethodArguments(method) + definition.globalArguments) that DefaultMethodExecutionService.execute merges at runtime (lines 245–257 of method_execution_service.ts).
• Backward compatibility: The definitionProvidedArgs?: string[] field is optional with ?? [] fallback — existing MethodResolution constructions (e.g., pre-existing tests that omit the field) continue to work unchanged, confirmed by the dedicated backward-compat test.
• Test coverage: 5 new unit tests cover all key scenarios: definition supplies all args, partial split with step inputs, partial unsatisfied (error names only the truly-missing key), overlap, and legacy mock backward compat.
• Code style: License headers present, named exports, no any types, test naming follows functionName: describes behavior convention.
• Security: No concerns — no user input handling, no new external calls.

[github-actions]

Adversarial Review

Critical / High

None.

Medium

None.

Low

1. Validator/runtime asymmetry on expression-filtered global args — src/libswamp/workflows/validate.ts:129-134

   The runtime (DefaultMethodExecutionService.execute at method_execution_service.ts:266-271) filters out global arguments whose values contain ${{ ... }} expressions before merging them into method args. The validator counts all global arg keys as "provided" regardless of whether their values are unresolved expressions.

   Breaking scenario: A model definition has globalArguments: { run: "${{ workflow.input.command }}" } and the method requires run. The workflow step supplies no step-level run input. The validator sees run as a key in globalArguments and passes. At runtime, run is filtered from the merge because it contains an unresolved expression, causing Zod schema validation to fail.

   Mitigating factors: (a) The PR comment at line 126–127 explicitly documents this as intentional ("invalid CEL is a runtime concern, not a validator concern"). (b) The unresolved expression is still accessible via the context.globalArgs Proxy at runtime, so the intent is for it to be provided. (c) This only affects global args with expressions — method-specific args with expressions are included in the runtime merge and are consistent with the validator.

   Verdict on this finding: Documented design choice, not a bug. The validator's purpose is "did the user forget to wire this argument?" not "will the expression resolve?" — so this behavior is defensible.

2. definitionProvidedArgs always populated even when empty — src/libswamp/workflows/validate.ts:129-134

   When neither getMethodArguments(methodName) nor globalArguments have any keys, definitionProvidedArgs is set to []. This is harmless — the ?? [] fallback in the consumer (validation_service.ts:454) handles undefined, and [] spreads into an empty set — but it means the field is always present on newly-created resolutions, making the ? on the type (definitionProvidedArgs?: string[]) slightly misleading. Not worth changing; the optional type correctly models backward compatibility with existing code that constructs MethodResolution without the field.

Verdict

PASS — The fix is correct and well-scoped. It accurately mirrors the runtime's merge of definition-provided arguments (method + global) into the set of "provided" inputs during validation. The type change is backward-compatible via the optional field with ?? [] fallback. Test coverage is thorough across five scenarios (definition-only, split, partial-miss, overlap, legacy compat). The one asymmetry with expression filtering is documented and intentional.