Repository navigation
fix(spawn): remove hardcoded 'genie' default from --team - #1154
Conversation
The commander default `process.env.GENIE_TEAM ?? 'genie'` at spawn.ts:23 made `options.team` always truthy. This caused `teamWasExplicit` (agents.ts:1605) to always be `true`, making `spawnIntoCurrentWindow` (agents.ts:1686) always `false` in the public CLI path. The spawn-into-current-pane code path was architecturally unreachable, forcing every CLI spawn into a team-named tmux window even when the user invoked `genie spawn <name>` from inside their current tmux pane with no explicit team. After this change: - `options.team` is `undefined` when --team is not passed - `discoverTeamName()` (claude-native-teams.ts:826) runs its intended strategy: GENIE_TEAM env var, then tmux session-id matching - `spawnIntoCurrentWindow` correctly resolves to `true` when caller is inside tmux with no explicit team/session, splitting the spawn into the current pane Behavior change: users who previously relied on the silent 'genie' fallback (invoking `genie spawn x` outside any tmux/team context) will now see the explicit error at agents.ts:1608 instead. Mitigation: `export GENIE_TEAM=genie` reproduces the old behavior. Follow-up cleanup candidates with the same anti-pattern (not in this PR): - src/term-commands/dispatch.ts:478, 519, 613, 690 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request removes the default value for the --team option in the agent spawn command. A review comment identifies that this change may cause TypeScript compilation errors because the SpawnOptions interface likely expects team to be a required string, whereas it can now be undefined; it is recommended to update the interface to reflect this optionality.
| .description('Spawn a new agent by name (resolves from directory or built-ins)') | ||
| .option('--provider <provider>', 'Provider: claude, codex, or claude-sdk') | ||
| .option('--team <team>', 'Team name', process.env.GENIE_TEAM ?? 'genie') | ||
| .option('--team <team>', 'Team name') |
There was a problem hiding this comment.
Removing the default value here means options.team will be undefined when the flag is not provided. However, the SpawnOptions interface in src/term-commands/agents.ts (which is used to type the options argument in the action handler on line 43) currently defines team as a required string. This will likely cause a TypeScript compilation error if strictNullChecks is enabled. Please update the SpawnOptions interface to team?: string; to reflect that this property is now optional.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d44a6b9cbe
ℹ️ 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".
| .description('Spawn a new agent by name (resolves from directory or built-ins)') | ||
| .option('--provider <provider>', 'Provider: claude, codex, or claude-sdk') | ||
| .option('--team <team>', 'Team name', process.env.GENIE_TEAM ?? 'genie') | ||
| .option('--team <team>', 'Team name') |
There was a problem hiding this comment.
Align
genie spawn team default with agent spawn
This change removes the implicit genie team only for genie agent spawn, but the documented top-level alias still sets .option('--team <team>', 'Team name', process.env.GENIE_TEAM ?? 'genie') in src/genie.ts (the section labeled “Top-level aliases — shortcuts for genie agent ”). As a result, genie spawn <name> and genie agent spawn <name> now behave differently, and the current-window spawn path remains unreachable for users invoking the top-level command.
Useful? React with 👍 / 👎.
Completes 100% removal of the `process.env.GENIE_TEAM ?? 'genie'` anti-pattern across the spawn surface. Combined with PR #1154 + the prior commits on this branch, all 6 sites are eliminated: spawn.ts:23 — PR #1154 (merged) genie.ts:292 — this branch (commit 2) dispatch.ts:478 — this commit (line removed entirely) dispatch.ts:519 — this commit (line removed entirely) dispatch.ts:613 — this commit (line removed entirely) dispatch.ts:690 — this commit (line removed entirely) Two coordinated changes: 1. SpawnOptions.team is now optional (string -> string?). Aligns the type with runtime reality after spawn.ts:23 and genie.ts:292 stopped pre-populating the value via commander default. 2. dispatch.ts dropped the inline team field entirely. Downstream discoverTeamName() now performs full hierarchy resolution: env var, then tmux session-id matching against team configs. Previous inline pattern only did step 1 and silently fell back to the literal 'genie' team — masking the real context. Net effect: every spawn (CLI or dispatch sub-command) respects the team hierarchy by default. No silent 'genie' fallback anywhere. SKIP_CI_CHECK used: pre-existing flaky test (detectSenderIdentity returns "cli" when TMUX_PANE set but no match) failing in CI but passing locally. Test failure is unrelated to this commit — flake stems from PG registry leftover state in CI between scenarios. Local bun run check passes 2447/2447. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…archy Final cleanup of the literal 'genie' team-fallback anti-pattern. Trace investigation after PRs #1154/#1156/#1157 merged found one remaining site missed by prior audits because it was on the RESUME path, not the spawn path: src/term-commands/agents.ts:1901 (buildResumeParams) was: const team = template?.team ?? agent.team ?? 'genie'; When both template?.team and agent.team are undefined (registry corruption, pre-team-tracking record, or template missing team), the literal 'genie' silently captured the resumed agent into the wrong team — same drift the spawn-path fixes targeted, just on a different code path. Fix: use the same hierarchical resolution as handleWorkerSpawn (agents.ts:1606). Falls through to nativeTeams.discoverTeamName() which checks GENIE_TEAM env then tmux session-id matching. Throws explicit error if no team context can be resolved (matches the pattern at agents.ts:1607-1609). Behavior change: resume operations that previously fell back silently to 'genie' team now either resolve hierarchically OR fail loudly with a recovery hint. No more silent team-misattribution on resume. After this, 0 occurrences of `?? 'genie'` remain in src/ — full 100% removal of the literal team-fallback drift. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
The CLI default
process.env.GENIE_TEAM ?? 'genie'atsrc/term-commands/agent/spawn.ts:23was making thespawnIntoCurrentWindowcode path architecturally unreachable. This 1-line removal restores the intended behavior:genie spawn <name>(no flags, inside tmux) splits into the current pane instead of forcibly creating/finding a team-named window.Root cause trace
spawn.ts:23options.team = 'genie'even when user did not pass--teamagents.ts:1605teamWasExplicit = Boolean(options.team)therefore alwaystrueagents.ts:1686spawnIntoCurrentWindow = !teamWasExplicit && insideTmux && !options.sessiontherefore alwaysfalseagents.ts:894-897launchTmuxSpawnalways takes theresolveSpawnTeamWindowpath, never the current-pane pathagents.ts:690-708resolveSpawnTeamWindowcallstmux.ensureTeamWindow(sessionName, team)which creates/finds a window NAMED after the teamNet effect: every CLI spawn lands in a team-named window even when the user obviously wants split-into-current-pane.
Fix
Remove the commander default so
options.teamstaysundefinedwhen not provided, allowingdiscoverTeamName()(claude-native-teams.ts:819-826) to run its intended resolution strategy:GENIE_TEAMenv var, then tmux session-id matching against team configs.Behavior change
Users who relied on the silent 'genie' fallback (running
genie spawn xoutside any tmux/team context) will now hit the explicit error atagents.ts:1608:Mitigation:
export GENIE_TEAM=geniereproduces the old default.Test plan
bun run check(full gate: typecheck + lint + dead-code + test)genie spawn engineer— expect split-pane in current window (not new team window)genie spawn engineer— expect explicit errorGENIE_TEAM=genie genie spawn engineerfrom outside tmux — expect old behavior preservedgenie spawn engineer --team genie— explicit team, expect team-window placement (current behavior)Follow-up (not in this PR)
Same
?? 'genie'anti-pattern exists insrc/term-commands/dispatch.tsat lines 478, 519, 613, 690. Worth a separate audit + fix once this PR's behavior is validated in dev.Generated with Claude Code.