Skip to content

refactor: define template commands with citty - #150

Merged
adelrodriguez merged 8 commits into
mainfrom
t3code/replace-yargs-with-citty
Aug 19, 2026
Merged

refactor: define template commands with citty#150
adelrodriguez merged 8 commits into
mainfrom
t3code/replace-yargs-with-citty

Conversation

@adelrodriguez

@adelrodriguez adelrodriguez commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

The template command runner depended on yargs and local CLI wrappers. This replaces the command tree with direct Citty command definitions and uses Citty for command routing, help, argument parsing, required positional arguments, and Boolean negation.

The script-specific test files, custom argument helpers, and template Faultier layer are removed. The root template tooling now depends on Citty and Consola only; Faultier remains a dependency of the application package that uses it.

Validation: bun run format, bun run check, bun run analyze, and CLI help smoke checks.

Implemented by gpt-5.6-sol in T3 Code through the Codex harness.

@vercel

vercel Bot commented Aug 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
init Ready Ready Preview Aug 19, 2026 3:33pm
init-docs Ready Ready Preview Aug 19, 2026 3:33pm

Request Review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

Two behaviors the description lists as preserved did not survive the port: TemplateFault errors now print a raw stack dump instead of the formatted message, and --keepApps is silently ignored by template setup.

Reviewed changes — the full port of the scripts/ command tree from yargs to citty@0.2.2, verified by running the CLI against the checked-out branch and by reading citty's parser in node_modules/citty/dist/index.mjs.

  • Entry point rewrittenscripts/index.ts drops the yargs builder chain and the top-level faultier error handler in favour of await runMain(main).
  • Command definitions portedtemplate, add, rename, and setup move from yargs builder/handler to citty args/meta/run.
  • Positional choices replacedadd validates kind with an explicit TemplateFault throw because citty positionals cannot carry options.
  • Array option hand-rolledgetOptionValues re-scans rawArgs for --keep-apps / --keep-packages, since citty resolves repeated flags last-wins.
  • Dependenciesyargs@18 and @types/yargs removed, citty@0.2.2 added, scripts/utils.ts deleted. The 566-line lockfile diff is hoisting reshuffle from dropping a direct dependency (cliui, wrap-ansi, string-width, eslint-scope, estraverse move to their remaining consumers' versions) and looks benign.

Verified as intact: no-argument and bare template both print usage and exit 1, --help works at all three nesting levels, missing required positionals fail before run, and --no-git / --no-install still resolve to false.

ℹ️ Unknown flags are no longer rejected

.strict() was applied at both the root and template levels and has no citty equivalent — citty parses with strict: false (index.mjs:90), so unrecognized flags land in values and are ignored. Under yargs@18 the same invocation failed with Unknown argument: bogus. The practical cost is that a typo in a destructive command goes unnoticed: bun template setup --no-instal --yes runs bun install instead of skipping it.

There is no drop-in fix, so this is a decision rather than a defect: accept the looser parsing as the cost of the migration, or validate rawArgs against the declared arg names in the commands where a silent miss is expensive.

Technical details
# `.strict()` has no replacement in the citty command tree

## Affected sites
- `scripts/index.ts:15` — root command no longer rejects unknown flags
- `scripts/template/index.ts:8-16``template` command no longer rejects unknown flags

## Required outcome
- Decide whether unknown-flag rejection is still a requirement for `template setup`, whose `run` prunes workspaces, renames the project, and deletes template files. If it is, unrecognized `--flags` must produce a non-zero exit before any of that work starts.

## Open questions for the human
- Is losing strict parsing an acceptable trade for the migration, or should `setup` specifically validate its own `rawArgs`?

ℹ️ Nitpicks

  • add's kind positional lost the yargs choices: ["app", "package"], so bun template add --help prints KIND Workspace type to add (Required) with no hint of the accepted values. citty positionals cannot carry options, so naming them in description is the only place left for them.
  • The root command's epilogue("Add your own project commands to scripts/index.ts.") is gone with no citty equivalent. docs/template-commands.md:50 still covers it, so this is only a loss of in-help discoverability.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/index.ts
Comment thread scripts/template/setup.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ Both blocking findings from the previous review are genuinely fixed — verified by running the CLI on this branch. One residual strictness gap and a few nits below.

Reviewed changes — the second commit, which restores the validation behavior the yargs → citty port dropped.

  • Reinstated scripts/utils.ts as a compatibility layerdefineTemplateCommand wraps each leaf command's run so a thrown TemplateFault renders through consola.error(error.flatten()) plus its details field and exits 1, and unknown --flags are rejected before run starts.
  • Restored .strict()-equivalent rejectiongetUnknownOption scans the leaf command's rawArgs against the declared arg names and aliases, normalizing kebab/camel spellings, stripping a leading no-, and stopping at --.
  • Fixed the --keepApps dropgetOptionValues now compares through normalizeOptionName, so --keepApps and --keep-apps are equivalent, and repeated plus =-inline forms accumulate.
  • Added the first scripts/** testscripts/template/__tests__/setup.test.ts pins the flag spellings getOptionValues claims to support.
  • Clarified add's kind help text — the description now names the accepted values, recovering what the yargs choices used to print.

Confirmed on the branch: template rename and template add widget foo print only the flattened message and details with no stack trace and exit 1; template rename --bogus, --no-bogus, and template setup --keepApp x all report Unknown option: --<name> and exit 1 before any filesystem work; --help never reaches the new guard because citty's runMain intercepts it ahead of runCommand; tsc --noEmit -p tsconfig.json, bunx oxlint scripts, and bun test scripts are all clean.

ℹ️ Flag position now changes the outcome, and misplacement is silent

The guard reads context.rawArgs, which citty has already sliced past the subcommand name, so a flag written before the subcommand is dropped by the parser and never reaches the guard. bun scripts --bogus template rename prints rename's own fault with no unknown-option error, and bun scripts template --yes setup reaches setup with rawArgs of []. Under yargs@18 options were parsed globally, so both positions worked; the worst realistic case is bun template --keep-apps app setup --yes, which discards the selection and keeps every workspace.

Technical details
# Pre-subcommand flags escape both citty's parser and the new unknown-option guard

## Affected sites
- `scripts/utils.ts:13``getUnknownOption(context.rawArgs, ...)`; `runCommand` passes `opts.rawArgs.slice(subCommandArgIndex + 1)` (`citty/dist/index.mjs:217`), so anything before the subcommand name is already gone
- `scripts/index.ts:5-13` and `scripts/template/index.ts:7-17` — the two `subCommands`-only levels use citty's plain `defineCommand`, so they perform no flag validation at all

## Reproduction
```
$ bun run scripts/index.ts --bogus template rename
[error] Provide --name when renaming a project.        # no "Unknown option: --bogus"

$ bun run scripts/index.ts template --name foo rename
Unknown command foo                                    # flag value read as a command name
```

## Required outcome
- Decide whether a flag in a position citty ignores should fail loudly. If it should, an unrecognized or misplaced `--flag` anywhere in `argv` must exit non-zero before `setup` prunes workspaces, renames the project, or deletes template files.

## Suggested approach (optional)
- The leaf command is the only place that knows its own arg names, so the check needs the untruncated `argv` (e.g. `process.argv.slice(2)` filtered of the command path) rather than `context.rawArgs`.
- If the ordering constraint is accepted instead, `docs/template-commands.md` is the place to state that options follow the subcommand.

## Open questions for the human
- Is "options must follow the subcommand" an acceptable documented constraint, or should misplacement be an error?

ℹ️ Nitpicks

  • The unknown-option path prints only consola.error(...), where yargs' .fail handler showed help before the message. Calling citty's showUsage(context.cmd) first would restore the hint about which flags are valid.
  • getOptionValues is now covered, but getUnknownOption and the fault wrapper are not, even though a false positive in the guard silently skips the command. They are also the pieces whose correctness depends on citty internals (--help interception happening before run), which is exactly what a test would pin.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/utils.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

The new guard closes the misplaced-flag hole, including the dangerous --keep-apps app template setup case. But it decides "is this word a command?" from a hand-maintained set, so it misreports unknown commands and will reject valid flags on any command added later.

Reviewed changes — the third commit, which makes a misplaced option a hard error instead of a silent drop. Verified by running the CLI and the helpers on the branch.

  • Added a pre-parse guard in the entry pointscripts/index.ts runs getOptionBeforeCommand(process.argv.slice(2), templateCommandNames) before runMain(main, { rawArgs }) and exits 1 with Place --<name> after the template subcommand.
  • Restored usage output on an unknown optiondefineTemplateCommand calls showUsage(context.cmd) ahead of consola.error("Unknown option: --x"), recovering what yargs' .fail used to print.
  • Removed the args castTemplateCommandDefinition<T> narrows CommandDef<T>["args"] from Resolvable<T> to T, so Object.entries(definition.args) no longer needs as T and the undefined / function-form shapes are rejected at compile time.
  • Covered the two argument helpersscripts/__tests__/utils.test.ts pins getUnknownOption (kebab/camel/no- spellings) and getOptionBeforeCommand (before vs. after the leaf command, --help exemption).

Confirmed on the branch: --bogus template rename, template --yes setup, and --keep-apps app template setup all exit 1 before any filesystem work; --help still exits 0 at every nesting level and reaches neither guard; bun test scripts passes 6 tests and bun run check reports 0 warnings and 0 errors.

ℹ️ Nitpicks

  • showUsage(context.cmd) passes no parent, and renderUsage builds its header from parentMeta.name + cmdMeta.name (citty/dist/index.mjs:304). So template rename --bogus prints USAGE rename [OPTIONS] while template rename --help prints USAGE template rename [OPTIONS] — citty's own help path gets the parent from resolveSubCommand.
  • getOptionBeforeCommand returns null where its sibling getUnknownOption returns undefined, and it does not break on -- the way getUnknownOption does. getOptionBeforeCommand(["template", "--", "rename"], names) returns "", which passes the caller's if (misplacedOption) only because the empty string happens to be falsy.
  • Both guards inspect only -- tokens, and findSubCommandIndex (citty/dist/index.mjs:269-278) skips any --prefixed token without consuming a value unless _isValueFlag matches — so bun scripts -v template setup drops -v and runs setup. Exposure is small today because no arg declares a short alias and -h is intercepted globally, but the guard's contract reads as "misplaced options are rejected".
  • The guard exempts --version, yet main.meta carries no version, so bun scripts --version prints the root usage plus No version specified and exits 1. Either set meta.version from the root package.json or drop the exemption.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/index.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

Deriving the command names from template.subCommands fixes the drift the last review flagged, but the same commit narrows the guard to one argv position — so an option typed before template is silently dropped again and the leaf command still runs.

Reviewed changes — the fourth commit, which replaces the hand-maintained command-name set in the misplaced-option guard. Verified by tracing citty's parser and by driving a stub command tree that imports the real getOptionBeforeCommand.

  • Derived the command names from the command treescripts/template/index.ts exports templateSubCommands and passes the same object to subCommands, so scripts/index.ts builds the guard's set with new Set(Object.keys(templateSubCommands)) and adding a leaf can no longer start rejecting valid flags on it.
  • Scoped the guard to the template commandgetOptionBeforeCommand now takes the parent command name and bails unless rawArgs[0] matches it, so bun scripts nope --flag reaches citty's Unknown command nope instead of being misdiagnosed as a misplaced option.
  • Narrowed the scan to a single argv position — only rawArgs[1] is inspected; it now also catches single-dash tokens and returns null on a bare --.
  • Updated the helper's tests — added a case pinning that unknown and root-level commands fall through to citty, and removed the case asserting that an option before template is rejected.

Confirmed on the branch: template --yes setup, template -x setup, and template --version setup still exit 1 before any filesystem work; nope --flag, template nope --flag, --keep-apps app template setup, and --name foo template setup all end in citty's own Unknown command error; bun test scripts passes 7 tests and bun run check reports 0 warnings and 0 errors.

ℹ️ Nitpicks

  • The new single-dash branch strips every leading dash before the message is formatted, so bun scripts template -x setup prints Place --x after the template subcommand. — a flag spelling the user never typed.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/utils.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ The silent-drop hole from the last review is genuinely closed — I could not construct an argv where a leaf runs with a typed option missing. One piece of dead code and two message-quality nits below.

Reviewed changes — the fifth commit, which widens the misplaced-option guard from a single argv position to everything ahead of the parent command.

  • Located the parent command the way citty doesgetOptionBeforeCommand now takes the first token that does not start with - as the parent position, mirroring findSubCommandIndex, and reports the first --prefixed token before it (a bare -- is ignored).
  • Moved the help exemption to the whole argv--help / -h anywhere disables the guard, matching citty's runMain, which intercepts help before runCommand ever runs.
  • Preserved the spelling the user typed — the guard returns the raw token and scripts/index.ts prints Place ${misplacedOption} after the template subcommand., so template -x setup no longer reports --x.
  • Re-pinned the reopened position in testsscripts/__tests__/utils.test.ts covers ["--bogus", "template", "setup"] and ["template", "-x", "setup"] alongside the updated --yes expectation.

Verified on the branch with an enumeration harness that feeds the real getOptionBeforeCommand and then runCommand over a stub tree reusing the real defineTemplateCommand and args definitions: across 280 argv shapes, 208 are rejected by the guard, 69 end in citty's own error, 3 are legitimate leaf runs, and 0 reach a leaf with a typed option absent from both args and rawArgs. bun test scripts passes 7 tests, and the working tree stays clean after the CLI runs.

ℹ️ Nitpicks

  • The leading-option branch (scripts/utils.ts:73) returns the token verbatim, so --name=foo template rename prints Place --name=foo after the template subcommand., while the after-parent branch strips at = (scripts/utils.ts:79) and prints --name. Worth picking one.
  • An option whose value is a separate token still escapes the guard, because the value is the first non-- token: --keep-apps app template setup bails at scripts/utils.ts:70 and surfaces as citty's Unknown command app. Exit is still 1 before any run, and mirroring citty's _isValueFlag is not possible while the root command declares no args, so this is message quality rather than a hole.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/utils.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found. Both nits from the last review are fixed, and the removed branch really was unreachable.

Reviewed changes — the sixth commit, which trims dead code from the misplaced-option guard and makes the token it reports spell consistently.

  • Deleted the unreachable --version / -v exemptionscripts/utils.ts drops (argument === "--version" || argument === "-v") && rawArgs.length === 1. Reaching that line required rawArgs[parentCommandIndex + 1] to be truthy, so rawArgs.length was always at least 2 and the condition could never hold.
  • Stripped =value from the leading-option token — the pre-parent branch now returns leadingOption.split("=", 1)[0], matching the after-parent branch, so --name=foo template rename and template --name=foo rename both report --name.
  • Pinned the new spellingscripts/__tests__/utils.test.ts adds an exact toBe("--bogus") assertion for ["--bogus=value", "template", "setup"], which fails against the previous verbatim return.

Confirmed on the branch: --bogus=value template rename and --name=foo template rename now print Place --bogus … / Place --name … and exit 1 before any filesystem work; template --version, template --version rename, bun scripts --version, and template --help all behave exactly as they did at bf7a2e8c, so the branch removal is a verified no-op. bun test scripts passes 7 tests, tsc --noEmit -p tsconfig.json and oxlint scripts are clean, and the working tree stays clean. Every invocation documented under docs/ places its options after the subcommand, so the guard rejects nothing the docs advertise.

Pullfrog  | View workflow run | Using Claude Opus𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

template setup can now delete every app or package workspace with no prompt and no error when --keep-apps arrives without a value, and expected validation errors print raw stack traces.

Reviewed changes — the seventh commit (dffef2a6), which reverts the validation and error-handling layers added in commits two through six and hands argument parsing entirely to citty. Verified against citty 0.2.2's parser in node_modules/citty/dist/index.mjs and by running the non-destructive CLI paths on this branch.

  • Deleted scripts/utils.ts and the defineTemplateCommand wrapperadd, rename, and setup import defineCommand straight from citty, so unknown --flags, misplaced options, and fault rendering are no longer intercepted anywhere.
  • Removed the Faultier layer from the template scripts — the six tagged error classes and the TemplateFault registry are gone from scripts/template/shared.ts, every throw is a plain Error, and faultier is dropped from the root devDependencies.
  • Replaced the hand-rolled array optiongetOptionValues and getSelectedNames are gone; keep-apps / keep-packages are plain citty string args read as args["keep-apps"]?.split(",").
  • Declared boolean negation through cittynegativeDescription on git and install, which citty 0.2.2 renders in --help.
  • Deleted both scripts/** test files — the helpers they covered no longer exist, so scripts/** is back to zero tests.

Confirmed intact on the branch: bun scripts and bare template both print usage and exit 1 through citty's own No command specified., --help works at all three nesting levels, add's required positionals fail before run, --no-git / --no-install resolve to false, and both --keep-apps and --keepApps resolve because citty 0.2.2 registers camel and kebab aliases itself. tsc --noEmit -p tsconfig.json and oxlint scripts are clean, and bun run analyze reports nothing new — the faultier drop leaves no unused or unresolved dependency, since packages/core carries its own. In bun.lock the top-level yargs entry falls back from the root's direct 18.0.0 to the transitive 17.7.2 its remaining consumers already share, which is the expected shape for dropping a direct dependency.

ℹ️ Strict parsing and the misplaced-option guard are gone again, and the ordering rule is still undocumented

This commit removes the two behaviors that commits two through six were written to restore, so the gaps the earlier reviews described are open again: bun template setup --no-instal --yes runs bun install instead of skipping it, and bun template --yes setup silently drops --yes and falls back to prompting. Choosing plain citty over the compatibility layer is a defensible call — citty hardcodes strict: false and there is no drop-in equivalent — but with the guard gone, docs/template-commands.md is now the only place left to state that options must follow the subcommand, and it does not state it.

Technical details
# The option-ordering constraint is now unenforced and undocumented

## Affected sites
- `scripts/index.ts:5-13` and `scripts/template/index.ts:13-19` — both `subCommands`-only levels declare no `args`, so `findSubCommandIndex` (`citty/dist/index.mjs:269-278`) skips any `-`-prefixed token without consuming a value and `runCommand` slices `rawArgs` past the subcommand name; a flag typed before the subcommand never reaches any parser
- `docs/template-commands.md:9-46` — documents `bun template rename --name <name>` and friends but states no ordering rule

## Required outcome
- A reader of the docs should be able to tell that `bun template --yes setup` does not mean the same thing as `bun template setup --yes`, and that an unrecognized flag on `setup` is ignored rather than rejected.

## Open questions for the human
- Is dropping strict parsing an accepted cost of the migration, given that `setup` prunes workspaces, renames the project, and deletes template files?
- The template scripts no longer follow the Faultier convention that `packages/core/src/errors/` still uses. Is `scripts/**` deliberately exempt, or should the two converge?

ℹ️ Nitpicks

  • bun template add app web extra silently ignores the trailing positional, where the removed .strict() reported Unknown argument: extra. citty only reads as many positionals as the command declares.
  • Mixing spellings resolves inconsistently: --keep-apps app --keep-apps web yields web (last wins) while --keep-apps app --keepApps web yields app (first wins), because citty reconciles aliases only when one side is undefined.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread scripts/template/setup.ts Outdated

const keepApps =
getSelectedNames(args.keepApps) ??
args["keep-apps"]?.split(",").filter(Boolean) ??

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

--keep-apps with no value parses to "", which is not undefined, so the ?? fallback never fires: "".split(",").filter(Boolean) is [], validateSelection accepts an empty selection, and pruneWorkspaces rm -rfs every app workspace with no prompt and no error. Repeated flags are also last-wins now — --keep-apps app --keep-apps web keeps only web and deletes app, where yargs' array: true and the deleted getOptionValues both accumulated. Line 239 has the same shape for --keep-packages.

Technical details
# An empty or repeated `--keep-apps` deletes workspaces the user asked to keep

## Affected sites
- `scripts/template/setup.ts:231``args["keep-apps"]?.split(",").filter(Boolean) ??`; `?.` only guards nullish, and citty coerces a valueless `string` arg to `""` (`citty/dist/index.mjs:101`)
- `scripts/template/setup.ts:239` — same expression for `--keep-packages`
- `scripts/template/setup.ts:86-92``validateSelection` only rejects names that are not real workspaces, so `[]` passes
- `scripts/template/setup.ts:162-168``scripts/template/shared.ts:149-155``pruneWorkspaces` removes every workspace not in the selection through `Bun.$` `rm -rf`

## Reproduction
Driving `parseArgs` from `citty` against this command's own `args` definition:

```
["--keep-apps"]                            -> { "keep-apps": "" }     -> selection []
["--keep-apps","app","--keep-apps","web"]  -> { "keep-apps": "web" }  -> selection ["web"]
```

## Required outcome
- An empty `--keep-apps` / `--keep-packages` must not mean "delete everything". Either treat an empty parsed list as "option not provided" so the `--yes` default or the prompt takes over, or fail before any workspace is removed.
- A second occurrence of the flag must either accumulate (matching the pre-PR `array: true` behavior) or be rejected, not silently discard the first value.

## Suggested approach (optional)
- A small helper that returns `undefined` for an empty result restores the `??` chain's intent for both options and both failure modes.

Comment thread scripts/index.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ Both findings from the last review are largely fixed and I verified them on the branch. One coverage gap below, plus nits — and the repeated---keep-apps half of the open thread is still live.

Reviewed changes — the eighth commit (74a66381), which replaces the thrown validation errors with in-run reporting and closes the empty---keep-apps deletion hole. Verified by running the non-destructive CLI paths and by driving citty's parseArgs against setup's own args.

  • Validation errors now render as one line — every deliberate argument-validation failure in add, rename, and setup does consola.error(message), sets process.exitCode = 1, and returns instead of throwing, so citty's runMain catch is never reached.
  • validateSelection became getSelectionError — it returns string | null and the caller reports it, keeping the unknown-workspace check on the same non-throwing path.
  • Empty --keep-apps / --keep-packages is rejected — a rawArgs scan catches a flag whose next token is missing or starts with -, and a second check catches a parsed value that filters to an empty list.
  • The project name is validated before anything is deletednormalizeScope(projectName) runs in a try/catch at setup.ts:308, ahead of pruneWorkspaces. Previously an invalid name typed at the prompt threw from renameProject after the workspace directories were already removed, so this closes a real half-migration window.
  • rename pre-checks its scope — the same try/catch around normalizeScope(scope) before any file is touched.

Confirmed on the branch: template rename, template add widget foo, template rename --name 'bad scope!', template setup --keep-apps, and template setup --keep-apps= each print a single [error] line with no stack trace or source excerpt and exit 1 — runMain does not call process.exit(0) on success, so process.exitCode survives. bunx tsc --noEmit -p tsconfig.json and bunx oxlint scripts are clean and the working tree stays clean. Driving parseArgs against setup.args, the rawArgs guard is not redundant with the parsed-value guard: --keep-apps --yes resolves to {"keep-apps": "--yes"} with yes swallowed, which without the guard would drop through to a package prompt and hang on a non-TTY.

ℹ️ The guards protecting the delete path depend on argv shapes nothing pins

hasEmptyKeepAppsOption (setup.ts:228) is the only thing standing between --keep-apps --yes and a hung prompt, and its correctness rests on a citty/util.parseArgs detail — that a valueless string option consumes the next flag as its literal value — which is not documented anywhere in the repo and is not obvious from the code. scripts/** has no tests, so a citty bump that changes value consumption, or a later edit to the predicate, changes what bun template setup deletes with nothing failing. The two __tests__ files this PR added at 9f000149 and 2af22928 were removed at dffef2a6, so the coverage that existed mid-PR is gone.

Technical details
# The `--keep-apps` / `--keep-packages` guards are untested

## Affected sites
- `scripts/template/setup.ts:228-248``hasEmptyKeepAppsOption` / `hasEmptyKeepPackagesOption`; the predicate `rawArgs[index + 1]?.startsWith("-") !== false` encodes three cases (absent token, flag-like token, real value) with no test
- `scripts/template/setup.ts:255-267` — the parsed-value guard, which is the only thing catching `--keep-apps=` and `--keep-apps ,,`
- `scripts/template/setup.ts:325-326``pruneWorkspaces`, the `rm -rf` these guards protect
- `scripts/` — no `__tests__` folder, so `bun test scripts` covers nothing

## Required outcome
- The argv shapes that decide whether `setup` deletes a workspace are pinned by a test that fails if the parse result changes.

## Suggested approach (optional)
- `docs/agents/testing.md` asks for `bun:test` in a `__tests__` folder beside the file. `parseArgs` is exported from `citty`, so `parseArgs(argv, setup.args)` exercises the real declaration without ever reaching `runMain`, and the guard predicates can be lifted into a named helper that a test can call directly.
- Table-drive the shapes that already differ: `["--keep-apps"]`, `["--keep-apps","--yes"]`, `["--keep-apps="]`, `["--keep-apps",",,"]`, `["--keep-apps","app","--keep-apps","web"]`, `["--keep-apps","app","--keepApps","web"]`. Assert the exact resolved selection, not just that it is non-empty.

ℹ️ Nitpicks

  • rawArgs[index + 1]?.startsWith("-") !== false (setup.ts:231, setup.ts:236) reads as its own opposite — it means "absent or flag-like". A named helper such as isMissingValue(rawArgs[index + 1]) would say what the two branches are without the reader having to work out that undefined !== false is the absent case.
  • The message Provide at least one workspace name with --keep-apps. is emitted from two places 20 lines apart (setup.ts:240 / setup.ts:259, and setup.ts:245 / setup.ts:264) for two different causes. It is also inaccurate for --keep-apps --yes and --keep-apps -x, where the user did type something and it was silently eaten as the value.
  • Errors that are not argument validation still reach citty's console.error(error, "\n") and print as a raw object dump. From a directory with no workspaces, template add app web prints ENOENT: no such file or directory, open '.../apps' with a path/syscall/errno block and no indication of which command failed. The two-tier convention this commit establishes is reasonable, but getProjectScope and getWorkspaces failing are things a user can cause by running from the wrong cwd.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@adelrodriguez
adelrodriguez force-pushed the t3code/replace-yargs-with-citty branch from 74a6638 to f354929 Compare August 19, 2026 15:32
@adelrodriguez
adelrodriguez merged commit bc91754 into main Aug 19, 2026
10 checks passed
@adelrodriguez
adelrodriguez deleted the t3code/replace-yargs-with-citty branch August 19, 2026 16:46
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