feat: add --global-arg flag to model create command

[stack72]

Summary

• Adds a repeatable --global-arg key=value option to swamp model create, eliminating the need to manually edit definition YAML to set globalArguments after creation
• Reuses the existing parseKeyValueInputs parser, supporting dot notation for nested objects (config.db.host=localhost), file references (key=@path), and values containing =
• Validates provided args against the model type's globalArguments Zod schema (when one exists) at create time, failing early with clear errors

User impact

Before this change, setting globalArguments required a two-step workflow:

swamp model create aws/ec2 my-vpc
swamp model edit my-vpc  # manually add globalArguments in YAML

Now it's a single command:

swamp model create aws/ec2 my-vpc \
  --global-arg region=us-east-1 \
  --global-arg config.timeout=30

Nested objects are supported via dot notation:

--global-arg config.db.host=localhost --global-arg config.db.port=5432
# → { config: { db: { host: "localhost", port: "5432" } } }

Invalid args are caught immediately:

swamp model create mytype foo --global-arg badformat
# Error: Invalid input format: "badformat". Expected key=value format.

Test plan

• deno check — type checking passes
• deno lint && deno fmt — clean
• deno run test src/cli/commands/model_create_test.ts — 12 tests pass (9 new)
• deno run test — full suite passes (2978 tests)
• deno run compile — binary compiles
• Manual testing: simple args, multiple args, nested dot notation, values with =, error cases (missing =, empty key), default behavior without flag

Closes #699

🤖 Generated with Claude Code

Co-authored-by: Blake Irvin blakeirvin@me.com

[github-actions]

Review Summary

The HIGH priority issue from the previous review has been addressed. The code now correctly uses result.data after Zod validation, ensuring transforms are applied.

No blocking issues.

Code Quality ✅

• TypeScript: Uses let correctly for reassignment; type assertion as Record<string, unknown> is acceptable after Zod validation
• Named exports used correctly
• AGPLv3 license headers present
• Pre-existing AnyOptions = any has documented lint-ignore (not introduced by this PR)

Domain-Driven Design ✅

• CLI layer handles input parsing and boundary validation appropriately
• Domain layer (Definition.create) receives validated data
• Uses UserError for user-facing errors

Test Coverage ✅

• 9 new tests covering option registration, parsing behavior, error cases, and schema validation
• Comment notes integration tests exist in integration/echo_model_test.ts

Security ✅

• Validated input via Zod schemas
• Uses existing validated parsing infrastructure

---

Suggestions (non-blocking)

1. Pre-existing prototype pollution concern: The setNestedValue function in input_parser.ts doesn't block __proto__/constructor/prototype keys. This is pre-existing code outside this PR's scope, but worth a follow-up hardening PR:

   const BLOCKED = new Set(["__proto__", "constructor", "prototype"]);
   // throw if any part matches

2. Test naming clarity: Tests like "--global-arg key=value populates globalArguments" call parseKeyValueInputs directly rather than the CLI option. Minor naming nit.

3. Test redundancy: Some tests duplicate coverage from input_parser_test.ts. Not incorrect, just redundant.

LGTM! 🚀

[github-actions]

Review: feat: add --global-arg flag to model create command

No blocking issues found. This PR is well-implemented and ready to merge.

Code Quality ✅

• Follows TypeScript strict mode conventions
• Uses named exports as required
• AGPLv3 headers present in all source files
• Proper error handling with UserError from the domain layer
• Reuses existing parseKeyValueInputs infrastructure from input_parser.ts

Domain-Driven Design ✅

• CLI layer properly delegates to domain entities (Definition, ModelType)
• Validation uses the model type's Zod schema (modelDef.globalArguments.safeParse)
• No domain logic leaking into the CLI layer
• Correctly passes globalArguments to Definition.create() which already supports this property

Test Coverage ✅

• 9 new unit tests covering:
  • Option registration
  • Key=value parsing
  • Multiple args accumulation
  • Dot notation for nested objects
  • Error cases (missing =, empty key)
  • Schema validation behavior
• Existing parseKeyValueInputs function is already well-tested in the codebase

Security ✅

• No injection vulnerabilities
• File path handling (@ prefix) uses existing, tested infrastructure
• Proper input validation with clear error messages

Suggestions (non-blocking)

1. The last two tests (lines 104-119) test Zod's safeParse directly rather than exercising the actual validation code path in model_create.ts. An integration test that invokes the command with invalid global args against a real model type would provide stronger coverage. However, the existing tests adequately verify the components work correctly.

Nice clean implementation that improves the user experience by eliminating the two-step workflow!

[github-actions]

Adversarial Review

Critical / High

None.

Medium

1. Prototype pollution exposure via --global-arg (src/cli/input_parser.ts:43-62)

   The setNestedValue function used by parseKeyValueInputs is vulnerable to prototype pollution. When a user passes:

   swamp model create mytype foo --global-arg '__proto__.isAdmin=true'

   The code path is:

   • parts = ["__proto__", "isAdmin"]
   • current["__proto__"] returns Object.prototype (not undefined)
   • typeof Object.prototype === "object" is true, so no new object is created
   • current = Object.prototype
   • current["isAdmin"] = "true" pollutes the global prototype

   After this, {}.isAdmin === "true" for ALL objects in the runtime.

   Impact: Limited because (a) this is a CLI tool where users have shell access anyway, (b) pollution only affects the current process, and (c) this is a pre-existing issue in setNestedValue that predates this PR. The PR expands the attack surface by adding a new code path using this function.

   Suggested fix: Reject prototype-polluting keys in setNestedValue or parseKeyValueInputs:

   const FORBIDDEN_KEYS = ["__proto__", "constructor", "prototype"];
   if (FORBIDDEN_KEYS.includes(part)) {
     throw new UserError(`Invalid key: "${part}" is a reserved property name.`);
   }

   Not blocking this PR since it's pre-existing, but recommend addressing separately.

Low

1. Missing test coverage for @path file references in --global-arg (src/cli/commands/model_create_test.ts)

   The parseKeyValueInputs function supports key=@/path/to/file syntax to read file contents, but no tests in this PR verify this works correctly with --global-arg. The behavior is inherited from existing code, but an integration test would increase confidence.

2. No validation of key segment validity in dot notation

   Keys like a..b=value (double dots) create objects with empty string keys: {a: {"": {b: "value"}}}. This is unlikely in practice but could cause confusing behavior. Not a bug per se, but worth documenting or sanitizing.

Verdict

PASS — The code changes are well-structured with proper Zod schema validation, clear error messages, and backward-compatible API design. The prototype pollution issue is pre-existing and has limited impact in a CLI context, so it should be tracked for a separate fix rather than blocking this feature addition.