Conversation
…owershell Add `workos completion <shell>` command that generates shell-specific completion scripts. A hidden `--get-yargs-completions` fast path is intercepted before yargs parses to avoid validation errors on partial tab input. The completion engine walks the existing help-json.ts command registry, normalizing its mixed flat/nested naming into a uniform tree so both `auth login` style entries and `skills > install` style entries resolve correctly for subcommand drilling. Supports descriptions alongside candidates in shells that render them (zsh with fzf-tab, fish, powershell).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds shell autocompletion: a completion engine that normalizes the command registry, computes completions for commands and options, emits a server-format response, and generates Bash/Zsh/Fish/PowerShell scripts. Adds an early CLI fast-path for yargs completion requests and a ChangesShell Completion Engine
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
- Hoist completion intercept above heavy imports (yargs, clack, semver) so Tab presses skip ~150ms of module loading - Export raw command/option arrays from help-json.ts, eliminating double buildCommandTree() call and 'in' type narrowing in completion.ts - Remove dead DIRECTIVE.DEFAULT constant - Remove unused _resetCache export - Remove duplicate shell validation in bin.ts (yargs choices + generateShellScript throw already cover it) - Add 7 new tests (option value skipping, empty args, partial prefix filtering, hidden commands, descriptions)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3311d846-0107-405b-8cfa-5f37121338ed
📒 Files selected for processing (5)
README.mdsrc/bin.tssrc/utils/completion.spec.tssrc/utils/completion.tssrc/utils/help-json.ts
| if (word.startsWith('-')) { | ||
| usedOptions.add(word); | ||
| const opt = findOption(current, globalOptions, word); | ||
| i += opt && optionTakesValue(opt) ? 2 : 1; | ||
| continue; |
There was a problem hiding this comment.
Used-option filtering misses aliases and can emit duplicate flags.
Line 143 stores flags exactly as typed, but Line 174 only checks canonical --name. If a user already used -D, --direct is still suggested. Also, command + global option merge can duplicate entries like --json.
✅ Suggested normalization + dedupe
function walkCommandTree(
@@
if (word.startsWith('-')) {
- usedOptions.add(word);
const opt = findOption(current, globalOptions, word);
+ if (opt) {
+ usedOptions.add(`--${opt.name}`);
+ if (opt.alias) usedOptions.add(`-${opt.alias}`);
+ } else {
+ usedOptions.add(word);
+ }
i += opt && optionTakesValue(opt) ? 2 : 1;
continue;
}
@@
function completeOptions(
@@
const opts = [...(command?.options ?? []), ...globalOptions];
const completions: Completion[] = [];
+ const emitted = new Set<string>();
for (const opt of opts) {
if (opt.hidden) continue;
const flag = `--${opt.name}`;
+ if (emitted.has(flag)) continue;
+ emitted.add(flag);
if (usedOptions.has(flag)) continue;
if (!flag.startsWith(partial)) continue;
completions.push({ name: flag, description: opt.description });
}Also applies to: 168-176
| local directive | ||
| directive=$(echo "$output" | tail -n1 | tr -d ':') | ||
| output=$(echo "$output" | head -n-1) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does BSD/macOS headsupporthead -n-1? What POSIX-portable alternative is recommended to drop the last line from shell output?
💡 Result:
No. BSD/macOS head does not support the GNU extension head -n -1 (or generally -n with a negative count). On macOS/BSD head, using a negative line count produces an error like “head: illegal line count -- -4”. [1] What’s POSIX-portable to drop the last line from shell output: Use sed '$d' (delete the last line of the stream): sed '$d' This is a common POSIX-style approach; e.g., it’s suggested as “If you want to see all but the last line … use: sed '$d'”. [2] Notes: - POSIX head is specified with -n number where the number must be a positive decimal integer, so head -n -1 is not POSIX-conforming. [3] - If you specifically install GNU coreutils on macOS, ghead -n -1 can work, but that’s not the native BSD/macOS head. [1]
Citations:
- 1: https://stackoverflow.com/questions/16206507/head-command-to-skip-last-few-lines-of-file-on-mac-osx
- 2: https://stackoverflow.com/questions/18127726/getting-head-to-display-all-but-the-last-line-of-a-file-command-substitution-an
- 3: https://www.unix.com/man-page/posix/1P/HEAD?os=posix§ion=1P&query=HEAD
🏁 Script executed:
# First, check if the file exists and read the specific lines mentioned
if [ -f "src/utils/completion.ts" ]; then
echo "=== Lines 245-255 from src/utils/completion.ts ==="
sed -n '245,255p' src/utils/completion.ts
else
echo "File not found, searching for completion.ts"
find . -name "completion.ts" -type f
fiRepository: workos/cli
Length of output: 326
🏁 Script executed:
# Search for all instances of 'head -n-' patterns in the file to understand scope
rg 'head\s+-n-' src/utils/completion.ts -A 2 -B 2Repository: workos/cli
Length of output: 196
🏁 Script executed:
# Check file type and context - is this generating shell scripts?
rg -i 'shell|bash|zsh|completion|script' src/utils/completion.ts | head -20Repository: workos/cli
Length of output: 1132
Shell completion script uses non-portable head -n-1 that breaks on macOS.
The generated bash completion script at line 251 uses head -n-1, which is a GNU extension not supported by BSD head (macOS default). This causes the completion function to fail when parsing directives on macOS systems with error: "head: illegal line count -- -1".
The POSIX-portable fix is to use sed '$d' instead, which is compatible across all POSIX-compliant systems.
💡 Portable fix
- directive=$(echo "$output" | tail -n1 | tr -d ':')
- output=$(echo "$output" | head -n-1)
+ directive=$(printf '%s\n' "$output" | tail -n 1 | tr -d ':')
+ output=$(printf '%s\n' "$output" | sed '$d')
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge after fixing the PowerShell tab-split bug; all other shells are unaffected One P1 bug (PowerShell src/utils/completion.ts — specifically the Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| directive=$(echo "$output" | tail -n1 | tr -d ':') | ||
| output=$(echo "$output" | head -n-1) |
There was a problem hiding this comment.
head -n-1 not portable on macOS (BSD utilities)
head -n-1 is a GNU extension. On macOS, BSD head only accepts positive line counts and returns an error for negative values, so $output would silently be set to empty, breaking bash completions entirely for macOS users without GNU coreutils. The portable equivalent is sed '$d' (delete the last line).
| if [[ "$comp" == "$desc" ]]; then | ||
| candidates+=("$comp") | ||
| else | ||
| candidates+=("$comp:$desc") | ||
| fi |
There was a problem hiding this comment.
_describe breaks when descriptions contain :
_describe uses the first : in each array entry as the separator between the completion name and its description. Several existing descriptions in help-json.ts contain literal colons (e.g. "Format: sk_live_* or sk_test_*", "domain:state"). When such an entry is assembled as "$comp:$desc" and passed to _describe, zsh truncates the description at the first colon, producing garbled output. Colons in descriptions must be escaped as \: before building the candidates array.
| if [[ "$comp" == "$desc" ]]; then | |
| candidates+=("$comp") | |
| else | |
| candidates+=("$comp:$desc") | |
| fi | |
| if [[ "$comp" == "$desc" ]]; then | |
| candidates+=("$comp") | |
| else | |
| local escaped_desc="${desc//:/'\\:'}" | |
| candidates+=("$comp:$escaped_desc") | |
| fi |
| if (seen.has(prefix)) { | ||
| const existing = result.find((c) => c.name === prefix)!; | ||
| existing.commands = [...(existing.commands ?? []), ...normalizeRegistry(children)]; |
There was a problem hiding this comment.
normalizeRegistry mutates original commandRegistry objects in-place
When a flat compound name (e.g. auth login) matches an already-seen non-compound entry (auth), existing.commands is written directly onto the object pushed from the input array — the same reference that lives in the exported commandRegistry. It is harmless today because the completion fast-path calls process.exit(0) immediately, but a shallow clone before mutation eliminates the silent coupling.
| if (seen.has(prefix)) { | |
| const existing = result.find((c) => c.name === prefix)!; | |
| existing.commands = [...(existing.commands ?? []), ...normalizeRegistry(children)]; | |
| if (seen.has(prefix)) { | |
| const idx = result.findIndex((c) => c.name === prefix); | |
| const existing = { ...result[idx]! }; | |
| result[idx] = existing; | |
| existing.commands = [...(existing.commands ?? []), ...normalizeRegistry(children)]; |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 completion command not excluded from unclaimed-env middleware
The completion command is missing from the middleware exclusion list at src/bin.ts:200. The middleware comment explicitly states that utility commands like skills, doctor, env, and debug are excluded because the warning is unnecessary — completion is also a utility command but was not added. When completion runs through the middleware, maybeWarnUnclaimed() may make an API call (adding latency) and emit a stderr warning. While it won't corrupt the stdout script output, running eval "$(workos completion bash)" could flash a confusing "Unclaimed environment" warning.
(Refers to lines 200-202)
Was this helpful? React with 👍 or 👎 to provide feedback.
| if ($line -match '^:(\\d+)$') { | ||
| $directive = [int]$matches[1] | ||
| } elseif ($line.Trim()) { | ||
| $parts = $line.Split("\\t", 2) |
There was a problem hiding this comment.
🔴 PowerShell completion script splits on literal \t instead of tab character
The generated PowerShell completion script uses $line.Split("\t", 2) to parse tab-separated completions. However, in PowerShell, "\t" is the literal two-character string backslash-t — PowerShell uses backtick escapes ("`t") for tab characters, not backslash escapes. The .Split() method is not regex-based, so it won't interpret \t as a tab.
Impact on different PowerShell versions
On PowerShell 7+ (.NET 6+): .Split("\t", 2) matches String.Split(String, Int32) and looks for the literal two-character string \t, which never appears in the output (the actual output uses real tab characters from src/utils/completion.ts:29). The entire line becomes $comp, including the tab and description text.
On PowerShell 5 (.NET Framework): .Split("\t", 2) resolves to Split(Char[], Int32), splitting on either \ or t as individual characters. This incorrectly splits any completion containing the letter t (e.g., --git-check splits into --gi and -check...).
Prompt for agents
In src/utils/completion.ts, the generatePowershell function at line 364 outputs `$parts = $line.Split("\\t", 2)` which generates PowerShell code `$parts = $line.Split("\t", 2)`. In PowerShell, `"\t"` is a literal backslash-t, not a tab character. PowerShell uses backtick escapes for special characters.
The fix needs to produce PowerShell code that splits on an actual tab character. Two options:
1. Use PowerShell's [char]9 for a tab: change the line to produce `$parts = $line.Split([char]9, 2)` — this works on both PowerShell 5 and 7.
2. Use PowerShell's backtick escape: change to produce `$parts = $line.Split("`t", 2)` — but this requires careful escaping in the JavaScript template literal since backtick is the template delimiter.
Option 1 is simpler and avoids JS escaping complexity. In the JS template literal, the line should be: `$parts = $line.Split([char]9, 2)`
Was this helpful? React with 👍 or 👎 to provide feedback.
Add parameterized test that asserts every public bin.ts command has a matching entry in the help-json registry. Catches forgotten registry updates at CI time (which would cause missing tab completions and missing --help --json entries). Also add 7 completion edge case tests: option value skipping, empty args, partial prefix filtering, hidden commands, descriptions.
| if ($line -match '^:(\\d+)$') { | ||
| $directive = [int]$matches[1] | ||
| } elseif ($line.Trim()) { | ||
| $parts = $line.Split("\\t", 2) |
There was a problem hiding this comment.
.Split("\t") never splits on tab — completions are broken
In PowerShell, double-quoted strings use backtick (`) for escape sequences, not backslash. "\t" is the two-character literal \t (backslash + t), not a tab. The .Split() .NET method performs a literal (non-regex) split, so it will never find a tab character in the line. As a result $parts always has exactly one element: the entire name\tdescription string gets used as $comp, inserting garbage text on tab completion.
Use "`t" (PowerShell backtick-t) to produce an actual tab character. Note: -split "\\n" is fine because -split uses regex where \n matches a newline.
| $parts = $line.Split("\\t", 2) | |
| $parts = $line.Split("\`t", 2) |
1 participant
Summary
workos completion <shell>command supporting bash, zsh, fish, and powershell--get-yargs-completionsfast path intercepted before yargs parses, avoiding validation errors on partial tab inputhelp-json.tscommand registry with normalization to handle the mixed flat/nested naming (e.g.auth loginvsskills > install)Usage
Test plan
workos completion bashoutputs a valid bash completion scriptworkos completion zshoutputs a valid zsh completion scriptworkos completion fishoutputs a valid fish completion scriptworkos completion powershelloutputs a valid powershell completion scripteval "$(workos completion zsh)"thenworkos <TAB>shows all commands with descriptionsworkos env <TAB>drills into subcommands (add, remove, switch, list, claim)workos doctor --<TAB>shows available optionsworkos auth <TAB>shows login, logout, status (normalized from flat compound names)npx tsc --noEmitpassesnpx vitest run src/utils/completion.spec.ts— 14/14 passSummary by CodeRabbit
New Features
Documentation
Tests