Skip to content

feat(cli): derive variadic CLI flags from z.array() types [OS-615]#810

Merged
galligan merged 2 commits into
mainfrom
fix/cli/derive-array-flags
Mar 26, 2026
Merged

feat(cli): derive variadic CLI flags from z.array() types [OS-615]#810
galligan merged 2 commits into
mainfrom
fix/cli/derive-array-flags

Conversation

@galligan

@galligan galligan commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • deriveFlags in @outfitter/cli/schema-input silently skipped z.array() types, requiring manual cli.options entries for array fields like tags and refs
  • Now generates variadic --flag <value...> Commander options for z.array() types, matching how Commander handles repeatable flags
  • Optional arrays and arrays with defaults are correctly handled (not marked as required)

Fixes https://linear.app/outfitter/issue/OS-615/deriveflags-should-handle-zarray-types-for-variadic-cli-flags

What changed

packages/cli/src/schema-input.ts: Moved "array" out of the skip-list in the deriveFlags switch and added a case that generates <value...> variadic flag syntax. Updated module-level and function-level TSDoc.

Test plan

  • 3 new tests: basic array derivation, optional arrays, arrays with defaults
  • All 30 schema-input tests pass
  • Typecheck passes

🤘🏻 In-collaboration-with: Claude Code

@linear

linear Bot commented Mar 25, 2026

Copy link
Copy Markdown

@github-actions github-actions Bot added docs Documentation improvement package/cli @outfitter/cli testing Test-related release:minor Contains new features (minor version bump) labels Mar 25, 2026

galligan commented Mar 25, 2026

Copy link
Copy Markdown
Contributor Author

@galligan galligan marked this pull request as ready for review March 25, 2026 17:50
@greptile-apps

greptile-apps Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR implements variadic CLI flag generation for z.array() types in @outfitter/cli's deriveFlags — previously these were silently skipped, requiring manual cli.options overrides for every array field. The change is minimal and well-scoped: one new case "array" in the switch, a recursive unwrapZodField call to extract the element type, and accumulator-style argParser coercion in createCommanderOption for numeric arrays. All previous review feedback has been addressed, including the wrapped-element-type recursion fix, the accumulating argParser, and the --help default description for number arrays.

Key changes:

  • ZodFieldInfo and DerivedFlag gain arrayElementType?: string to carry element type through the pipeline
  • unwrapZodField now recursively calls itself on def.element so wrapped element types (e.g. z.coerce.number(), z.number().optional()) are correctly resolved
  • deriveFlags generates --flag <value...> for array fields; optional and defaulted arrays are not marked required
  • createCommanderOption installs an accumulating argParser for number element arrays to prevent Number(val) from clobbering the accumulator; string arrays use Commander's native variadic collection
  • Number-array Commander defaults are cleared (option.default(undefined, ...)) so Zod applies the real default during validation rather than seeding the accumulator
  • 7 new unit tests cover all branches including element-type unwrapping, multi-value accumulation, and the string-array native path

Confidence Score: 4/5

Safe to merge — the feature is additive, all previous review issues are addressed, and the accumulating argParser correctly handles multi-value variadic options

The implementation is logically correct: the recursive element-type unwrapping handles z.coerce.number() and other wrappers; the accumulating argParser avoids the Number-clobber bug; the option.default(undefined, description) trick correctly separates Commander's live default from the help text. The one-point deduction is for the still-present else { option.default(undefined); } for required number arrays (a no-op but slightly noisy) and the lack of an integration test driving Commander parsing through validateInput.

No files require special attention; schema-input.ts is the only substantive change and it is well-tested

Important Files Changed

Filename Overview
packages/cli/src/schema-input.ts Adds z.array() support to deriveFlags and createCommanderOption with proper variadic flag syntax, accumulating argParser for numeric arrays, and recursive element-type unwrapping; all previous review issues addressed
packages/cli/src/tests/schema-input.test.ts Adds 7 new tests covering basic array derivation, optional arrays, arrays with defaults, element-type tracking, wrapped element types, accumulating number-array argParser, and string-array native collection — all previously requested test cases are present
.changeset/cli-derive-array-flags.md Correctly marks @outfitter/cli as a minor semver bump with an accurate description of the variadic array flag feature

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["deriveFlags(schema, explicitLongFlags)"] --> B["unwrapZodField(field)"]
    B --> C{def.type?}
    C -->|default| D["extract defaultValue, walk innerType"]
    C -->|optional/nullable| E["set isOptional, walk innerType"]
    C -->|pipe| F["walk out type (z.coerce.*)"]
    C -->|array| G["return baseType='array'\narrayElementType=unwrapZodField(element).baseType"]
    C -->|string/number/boolean/enum| H["return base ZodFieldInfo"]
    G --> I["deriveFlags switch(baseType)"]
    H --> I
    I -->|array| J["flagString = '--flag <value...>'"]
    I -->|number| K["flagString = '--flag <n>'"]
    I -->|string/enum| L["flagString = '--flag <value>'"]
    I -->|boolean| M["flagString = '--flag'"]
    J --> N["createCommanderOption(flag, schema)"]
    K --> N
    L --> N
    M --> N
    N --> O{isNumberArray?}
    O -->|yes| P["accumulating argParser\n(val, prev[]) => [...prev, Number(val)]"]
    O -->|no| Q{baseType===number?}
    Q -->|yes| R["argParser(Number) — scalar coercion"]
    Q -->|no| S[no argParser]
    P --> T{flag.defaultValue?}
    T -->|set| U["option.default(undefined, JSON.stringify(defaultValue))"]
    T -->|unset| V["option.default(undefined)"]
    N --> W{isRequired?}
    W -->|yes| X["makeOptionMandatory(true)"]
Loading

Reviews (8): Last reviewed commit: "chore: add changeset" | Re-trigger Greptile

Comment thread packages/cli/src/__tests__/schema-input.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 869116cb92

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/cli/src/schema-input.ts
Comment thread packages/cli/src/schema-input.ts
Comment thread packages/cli/src/schema-input.ts Outdated
@galligan

Copy link
Copy Markdown
Contributor Author

@greptileai review

Comment thread packages/cli/src/schema-input.ts Outdated
@galligan galligan force-pushed the fix/cli/derive-array-flags branch from ebf3344 to bd832f2 Compare March 26, 2026 02:35
Comment thread packages/cli/src/schema-input.ts Outdated
@galligan galligan force-pushed the fix/cli/derive-array-flags branch from bd832f2 to 58b1d54 Compare March 26, 2026 15:41
Comment thread packages/cli/src/__tests__/schema-input.test.ts
@galligan galligan force-pushed the fix/cli/derive-array-flags branch from 58b1d54 to 1395a41 Compare March 26, 2026 16:20

galligan commented Mar 26, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Mar 26, 11:48 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 26, 11:48 PM UTC: @galligan merged this pull request with Graphite.

@galligan galligan merged commit c3946d3 into main Mar 26, 2026
12 checks passed
@galligan galligan deleted the fix/cli/derive-array-flags branch March 26, 2026 23:48
galligan added a commit that referenced this pull request Mar 26, 2026
…6] (#813)

## Summary

CI Summary job fails intermittently even when all individual jobs pass. Observed 3-4 times in a single session across PRs #806, #810, #811, #812.

**Root cause:** When `gt submit` force-pushes a branch, GitHub Actions cancels the in-flight run. The `ci-summary` job checks `contains(needs.*.result, 'cancelled')`, which catches cancelled jobs from the superseded run — not actual failures.

Fixes https://linear.app/outfitter/issue/OS-616/ci-summary-job-fails-spuriously-after-force-push-due-to-cancelled-job

## What changed

`.github/workflows/ci.yml`:

1. **Dropped `cancelled` from the failure check** — only `failure` triggers the exit. Cancelled jobs from superseded runs aren't real failures.

2. **Added `concurrency` group** — `ci-${{ github.ref }}` with `cancel-in-progress: true` ensures only one CI run per branch exists at a time, preventing the race structurally.

## Test plan

- [x] Workflow YAML is syntactically valid
- [x] The concurrency group scopes to the branch ref, so main and PR branches don't cancel each other
- [x] `cancel-in-progress: true` only cancels runs on the same ref (safe for stacked PRs on different branches)

🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation improvement package/cli @outfitter/cli release:minor Contains new features (minor version bump) testing Test-related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant