cli: interactive persona picker on bare invocation#83
Conversation
Running `agentworkforce` with no arguments now drops into a TUI when stdin/stderr are TTYs. The picker shows the 3 most recently launched personas by default, fuzzy-matches across name and description when the user types, and labels each row with its cascade source (cwd, user, dir:n, library) so it's obvious where a persona came from. Recents are persisted to ~/.agentworkforce/workforce/recents.json and updated inside runAgentSelector so explicit `agent <id>` launches feed the list too. Non-TTY pipes still print USAGE and exit 1, so scripts that probed the exit code keep working. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| inputValues?: Record<string, string> | ||
| ): Promise<never> { | ||
| const target = parseSelector(selector); | ||
| recordRecent(target.spec.id); |
There was a problem hiding this comment.
🟡 recordRecent persists side-effect during --dry-run
At packages/cli/src/cli.ts:2517, recordRecent(target.spec.id) is called unconditionally before the flags.dryRun check at line 2523. The --dry-run flag's documented intent is to "validate the persona without spawning the harness or burning tier-model tokens" — it should be a side-effect-free validation pass. Instead, every --dry-run invocation writes to the recents file on disk, causing the persona to appear in the interactive TUI's "RECENT" section even though it was never actually launched.
Prompt for agents
The call to recordRecent at line 2517 happens before the dryRun check at line 2523. Move the recordRecent call to after the dryRun early-exit, so that dry-run invocations do not produce side effects. Specifically, in runAgentSelector in packages/cli/src/cli.ts, the recordRecent call should be placed after the if (flags.dryRun) block (after line 2526) so it only runs when the persona is actually launched.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 2f1d8c8 — moved recordRecent(target.spec.id) past the dry-run early-exit. --dry-run no longer touches the recents file, so validation passes leave the MRU list alone.
|
|
||
| function render(): void { | ||
| const cols = stderr.columns ?? 100; | ||
| const showingRecents = !query.trim() && opts.recentIds.length > 0 && visible.length > 0; |
There was a problem hiding this comment.
🟡 TUI shows "RECENT" header when all recent ids are stale and full candidate list is displayed instead
In packages/cli/src/persona-tui.ts:231, showingRecents checks opts.recentIds.length > 0, but computeVisible() (line 220-227) checks whether recentCandidates(...) actually resolved any ids against the current candidate list. When the user has recents on disk but all referenced personas have been uninstalled or renamed, recentCandidates returns [], so computeVisible falls back to the full candidate list. However, showingRecents still evaluates to true (because opts.recentIds.length > 0), causing the header to display "RECENT" even though the full candidate list is shown — not the recents.
| const showingRecents = !query.trim() && opts.recentIds.length > 0 && visible.length > 0; | |
| const showingRecents = !query.trim() && recentCandidates(opts.candidates, opts.recentIds, RECENT_DEFAULT_VISIBLE).length > 0 && visible.length > 0; |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 2f1d8c8 — extracted a pure computeTuiView() helper that returns { mode: 'recents' | 'all' | 'matches', items }. mode is now derived from whether recentCandidates() actually resolved anything, so the header reads "PERSONAS" (not "RECENT") when every previously-used persona has been uninstalled/renamed. Added a regression test in persona-tui.test.ts for the stale-recents case.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e31a382fed
ℹ️ 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".
| if (text === '\x03' || text === '\x1b') { | ||
| settle(undefined); | ||
| return; |
There was a problem hiding this comment.
Treating a single "\x1b" chunk as immediate quit can terminate the picker when users press arrow keys, because terminal escape sequences may arrive split across data events (first ESC, then [A/[B). In that case the first chunk hits this branch and exits with code 130 instead of moving selection, making navigation unreliable on some terminals/SSH sessions unless sequence buffering or a timeout-based disambiguation is added.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f1d8c8 — bare \x1b now goes into a 50ms buffer; if more bytes arrive within the window the combined chunk is dispatched (so split \x1b + [A reads as Arrow Up), and the bare-Esc path only fires after the timer elapses. 50ms is well below human-perceptible Esc latency but plenty of slack for fragmented sequences over SSH/multiplexers.
| inputValues?: Record<string, string> | ||
| ): Promise<never> { | ||
| const target = parseSelector(selector); | ||
| recordRecent(target.spec.id); |
There was a problem hiding this comment.
recordRecent is executed before the dryRun early-exit path, so agentworkforce agent <id> --dry-run marks personas as recently used even though no interactive launch occurred. This pollutes the MRU list used by bare invocation and can push out actually launched personas after repeated validations; moving the write after the dry-run branch (or gating on !flags.dryRun) avoids this behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f1d8c8 — same change as the Devin comment above. recordRecent now runs only on real interactive launches, after the --dry-run early-exit.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughA new interactive terminal persona picker is added to the CLI. When invoked with no subcommand and both stdin/stderr are TTYs, it launches a fuzzy-searchable menu of personas with recent selections promoted at the top. A new ChangesInteractive Persona Picker with TUI and Recents
Sequence DiagramsequenceDiagram
participant User
participant main
participant runInteractivePicker
participant buildTuiCandidates
participant loadRecents
participant runPersonaPickerTui
participant runAgentSelector
participant recordRecent
User->>main: CLI with no subcommand in TTY
main->>runInteractivePicker: no args, TTYs detected
runInteractivePicker->>buildTuiCandidates: enumerate personas
buildTuiCandidates->>runInteractivePicker: return candidates
runInteractivePicker->>loadRecents: fetch recent ids
loadRecents->>runInteractivePicker: return recents
runInteractivePicker->>runPersonaPickerTui: launch picker
User->>runPersonaPickerTui: search, navigate, select
runPersonaPickerTui->>runInteractivePicker: return selected id
runInteractivePicker->>runAgentSelector: forward persona id
runAgentSelector->>runInteractivePicker: resolve target
runInteractivePicker->>recordRecent: persist selection
recordRecent->>User: CLI completes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/persona-tui.ts (1)
288-294: 💤 Low valueBare escape key handling may conflict with partial escape sequences on slow terminals.
In raw mode, escape sequences (e.g., arrow keys:
\x1b[A) can occasionally arrive in multiple chunks on slower connections or under high load. If\x1barrives alone before[A, the picker will quit unexpectedly.This is a known tradeoff in raw-mode TUIs and works fine on modern local terminals. Consider documenting this or adding a short debounce if users report issues over SSH.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/persona-tui.ts` around lines 288 - 294, The handler in function onData currently quits immediately on a lone '\x1b', which can be a partial escape sequence arriving before the rest of the bytes; change onData to accumulate input bytes and/or add a short debounce when receiving exactly '\x1b' so we wait a few milliseconds for additional bytes before calling settle(undefined). Concretely: in onData, when chunk === '\x1b' (or buffer.toString() === '\x1b'), start a short timer (e.g., 30–100ms) and if no further data arrives append to the pending buffer and then call settle(undefined); cancel the timer and process the full sequence if more bytes arrive (treating '['/letters as part of a sequence). Ensure settle is only invoked after the debounce or after confirming the sequence is a true lone Escape; alternatively maintain a small inputBuffer to combine chunks and parse multi-byte escape sequences before deciding to quit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli/src/persona-tui.ts`:
- Around line 288-294: The handler in function onData currently quits
immediately on a lone '\x1b', which can be a partial escape sequence arriving
before the rest of the bytes; change onData to accumulate input bytes and/or add
a short debounce when receiving exactly '\x1b' so we wait a few milliseconds for
additional bytes before calling settle(undefined). Concretely: in onData, when
chunk === '\x1b' (or buffer.toString() === '\x1b'), start a short timer (e.g.,
30–100ms) and if no further data arrives append to the pending buffer and then
call settle(undefined); cancel the timer and process the full sequence if more
bytes arrive (treating '['/letters as part of a sequence). Ensure settle is only
invoked after the debounce or after confirming the sequence is a true lone
Escape; alternatively maintain a small inputBuffer to combine chunks and parse
multi-byte escape sequences before deciding to quit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2101dd77-4662-4be4-b888-9cd491ce3e11
📒 Files selected for processing (3)
packages/cli/src/cli.tspackages/cli/src/persona-tui.test.tspackages/cli/src/persona-tui.ts
- Don't pollute recents under `--dry-run` (Devin/Codex): move recordRecent past the dry-run early-exit so validation runs leave the MRU list alone. - Fix stale "RECENT" header when every recent id has been uninstalled (Devin): factor view-mode selection into a pure computeTuiView() helper that returns mode='all' when recentCandidates() can't resolve anything, and have render() trust the helper's mode field. - Buffer bare Escape for 50ms before treating it as quit (Codex/CodeRabbit): on slow terminals (SSH, mux) arrow keys arrive as `\x1b` then `[A` in separate data events; without buffering the first chunk killed the picker. The debounce window is below human-perceptible Esc latency. Added unit tests for computeTuiView covering recents/all/matches modes and the regression where stale recents previously misreported the header. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
agentworkforcewith no args now opens a TUI (TTY only) instead of dumping help. The top 3 most-recently-used personas are shown first; typing fuzzy-matches across persona name and description.<id> <source> <description>where source is the cascade label (cwd,user,dir:n,library) — same vocabulary asagentworkforce list.~/.agentworkforce/workforce/recents.json(dedup, cap 20) and are updated insiderunAgentSelector, so explicitagent <id>launches feed the list as well.Implementation notes
packages/cli/src/persona-tui.ts: pure helpers (fuzzyScore,rankCandidates,recentCandidates,parseRecents,nextRecents), a small fs-backed recents store, and the interactive runner that uses the alternate screen buffer + raw mode.packages/cli/src/cli.tswires the bare-invocation path, addsbuildTuiCandidates()(carries source labels alongside id/description), and records the chosen persona id insiderunAgentSelector.packages/cli/src/persona-tui.test.tscover fuzzy scoring/ranking edge cases, recents merge logic, garbage-input tolerance, and the fs round-trip.Test plan
pnpm --filter @agentworkforce/cli test— 160/160 passpnpm -r typecheck— clean< /dev/null) falls back to USAGE;-h,-v,listunaffectedagentworkforcein a real terminal — confirm arrows/enter/esc, fuzzy search across name + description, and that the recents list updates after launching an agent🤖 Generated with Claude Code