Skip to content

refactor: use oclif variadic args instead of strict=false#782

Draft
rexxars wants to merge 2 commits intomainfrom
rexxars/oclif-varargs
Draft

refactor: use oclif variadic args instead of strict=false#782
rexxars wants to merge 2 commits intomainfrom
rexxars/oclif-varargs

Conversation

@rexxars
Copy link
Copy Markdown
Member

@rexxars rexxars commented Mar 24, 2026

Summary

  • Replaces strict = false + manual argv parsing with oclif's native multiple: true on Args.string() in doctor, documents delete, and install commands
  • Uses oclif's built-in validation for the doctor command's check names via options: [...] instead of manual validation
  • exec remains strict = false since it's a genuine passthrough case
  • Lockfile updated from oclif dep bumps that landed on main

Test plan

  • Existing tests updated and passing
  • doctor command validates check names via oclif's options (error message changed from custom to oclif's Expected X to be one of...)
  • Manual: sanity doctor cli studio, sanity documents delete id1 id2, sanity install pkg1 pkg2

🤖 Generated with Claude Code

Replace `strict = false` + manual argv parsing with `multiple: true` on
Args.string() in doctor, documents delete, and install commands. This
uses oclif's built-in variadic argument support (added in @oclif/core
4.10.0) for proper parsing, validation, and help output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Mar 24, 2026

Claude finished @rexxars's task in 49s —— View job


Review

  • Read changed files
  • Analyze diff
  • Post review

packages/@sanity/cli/src/commands/doctor.ts, line 149

The isDoctorCheckName filter in getChecks is dead code. Since options: [...KNOWN_CHECKS] is now set on the oclif arg, oclif validates all values before getChecks is ever called — no invalid name can reach this filter, so the false branch is unreachable. This is what's causing the 7.2% coverage drop reported by CI.

Replace the filter with a type assertion:

// before
return checkNames.filter((name) => isDoctorCheckName(name)).map((check) => doctorChecks[check])

// after
return (checkNames as DoctorCheckName[]).map((check) => doctorChecks[check])

And remove the now-unused isDoctorCheckName function (lines 140–142). Fix this →

@socket-security
Copy link
Copy Markdown

socket-security bot commented Mar 24, 2026

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​@​inquirer/​prompts@​8.3.0991009896100
Updatednpm/​@​sanity/​pkg-utils@​10.4.12 ⏵ 10.4.1397100100100 +1100

View full report

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 24, 2026

Coverage Delta

File Statements
packages/@sanity/cli/src/commands/doctor.ts 88.9% (- 7.2%)
packages/@sanity/cli/src/commands/documents/delete.ts 100.0% (+ 2.7%)
packages/@sanity/cli/src/commands/install.ts 100.0% (±0%)

Comparing 3 changed files against main @ 56f2c0ad9debf4324b7f736ba2f19a2421f4eb7a

Overall Coverage

Metric Coverage
Statements 83.0% (- 0.0%)
Branches 72.7% (±0%)
Functions 83.1% (- 0.1%)
Lines 83.4% (- 0.0%)

The simulateOAuthCallback helper used a single 100ms delay before
connecting to the local auth server. On Windows CI, the server may not
be listening yet, causing ECONNREFUSED. Add retry logic with backoff
and use 127.0.0.1 directly to avoid IPv6 resolution delays.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant