fix: parse object and array params from JSON strings

[lucianfialho]

Summary

• JSON strings passed via CLI flags for object/array type params are now parsed into proper objects before sending to the API
• Fixed in both index.ts (main flow) and commander-builder.ts (alternate flow)
• Falls back gracefully to raw string if JSON parsing fails

Changes

• src/index.ts — Added object/array case to param type coercion in action handler
• src/executor/commander-builder.ts — Added same object/array case to collectParams() switch
• src/executor/commander-builder.test.ts — 3 new tests: object parsing, array parsing, invalid JSON fallback

Test plan

• All 85 tests pass (82 existing + 3 new)
• Build passes clean
• Manual: --content '{"text":"hello"}' sends parsed object, not string
• Manual: --tags '["a","b"]' sends parsed array
• Manual: --data "not-json" falls back to string without error

Closes #15

---

🤖 Implemented by Claude Code dev-pipeline

[lucianfialho]

@claude Review This PR

[claude]

Claude finished @lucianfialho's task in 1m 15s —— View job

---

PR Review: fix: parse object and array params from JSON strings

Todo:

• Gather context and read changed files
• Review src/index.ts changes
• Review src/executor/commander-builder.ts changes
• Review src/executor/commander-builder.test.ts changes
• Provide comprehensive review feedback

---

Summary

The fix correctly solves the core bug: object/array params passed as CLI strings are now JSON-parsed before being sent to the API. Implementation is clean and tests pass. A few notes below.

---

What's Good

• Consistent fix applied to both code paths (index.ts and commander-builder.ts)
• Graceful fallback to raw string on JSON parse failure — no crash
• Switch statement in commander-builder.ts (collectParams) is clean and readable
• Tests cover all three scenarios: object parsing, array parsing, invalid JSON fallback
• Test structure using vi.stubGlobal("fetch", ...) verifies the actual HTTP body sent

---

Issues & Suggestions

1. Duplicated logic across two code paths (design concern)

The same JSON-parse logic lives in both src/index.ts:151-156 and src/executor/commander-builder.ts:98-109. The PR notes this explicitly, but doesn't address the duplication. If the behavior needs to change (e.g., adding a warning on parse failure), both places need updating.

The two builders appear to coexist (index.ts builds commands dynamically; commander-builder.ts via buildCommands). It's worth considering whether these can be unified or whether a shared coerceParamValue(p, value) helper would prevent drift.

Fix this →

2. Silent fallback may hide user mistakes (src/index.ts:154, commander-builder.ts:103-104)

When the user passes something like --content not-json for an object-typed param, the code silently sends the string "not-json" to the API. The user gets no feedback — the error (if any) will come from the API and may be confusing.

Consider at minimum a console.warn on parse failure:

} catch {
  // warn user their input couldn't be parsed as JSON
  console.warn(`Warning: --${p.name} value is not valid JSON, sending as string`);
  params[p.name] = opts[p.name];
}

Fix this →

3. No tests for the index.ts code path

The 3 new tests only exercise buildCommands (from commander-builder.ts). The equivalent code in src/index.ts:151-156 (the buildDynamicCommands function) has no corresponding coverage. A regression there would go undetected.

4. Minor: inconsistent style between the two files

index.ts uses an if-else if-else if-else chain while commander-builder.ts uses a switch. These can drift further over time. Minor, but worth noting if/when the duplication is addressed.

5. Test cleanup pattern is slightly redundant

beforeEach(() => { vi.restoreAllMocks(); }) + vi.unstubAllGlobals() at the end of each test is fine but inconsistent. Prefer using afterEach for cleanup over inline cleanup:

afterEach(() => {
  vi.restoreAllMocks();
  vi.unstubAllGlobals();
});

---

Verdict

Approve with minor suggestions. The core fix is correct and well-tested for the happy path. The silent fallback behavior and missing index.ts test coverage are the two most worth addressing before shipping.