Repository navigation
Dev - #1159
Dev#1159
Conversation
…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>
fix(spawn): remove last '?? genie' literal — buildResumeParams hierarchy (truly 100%)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughVersion updated across manifest and package files from 4.260416.3 to 4.260416.5. Team resolution logic in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs✨ Finishing Touches📝 Generate docstrings
🧪 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 updates the project version to 4.260416.5 across multiple configuration files and modifies the agent resumption logic in src/term-commands/agents.ts to dynamically discover the team name instead of using a hardcoded default. It also adds a validation step to ensure a team context is available. A review comment suggests passing the agent's repository path to the discovery function to ensure the correct directory context is used during lookup.
| const agentName = agent.role ?? agent.id; | ||
| const provider = (template?.provider ?? agent.provider ?? 'claude') as ProviderName; | ||
| const team = template?.team ?? agent.team ?? 'genie'; | ||
| const team = template?.team ?? agent.team ?? (await nativeTeams.discoverTeamName()); |
There was a problem hiding this comment.
When falling back to team discovery, it is recommended to pass agent.repoPath to discoverTeamName. This ensures that the discovery logic (which scans for Claude session files) uses the agent's actual working directory rather than the current process's CWD, which may differ if the command is invoked from a different location.
| const team = template?.team ?? agent.team ?? (await nativeTeams.discoverTeamName()); | |
| const team = template?.team ?? agent.team ?? (await nativeTeams.discoverTeamName(agent.repoPath)); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f00a9aaf8
ℹ️ 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".
| const agentName = agent.role ?? agent.id; | ||
| const provider = (template?.provider ?? agent.provider ?? 'claude') as ProviderName; | ||
| const team = template?.team ?? agent.team ?? 'genie'; | ||
| const team = template?.team ?? agent.team ?? (await nativeTeams.discoverTeamName()); |
There was a problem hiding this comment.
Use agent repo path for team discovery on resume
When template?.team and agent.team are missing, this fallback calls discoverTeamName() without a cwd, so it resolves team context from process.cwd() instead of the agent’s stored workspace. In practice, resuming from a different directory can select the wrong team or return null and raise the new hard error, even though agent.repoPath contains the correct project context. Passing agent.repoPath here would avoid these false failures for legacy/migrated agents that rely on discovery.
Useful? React with 👍 / 👎.
Summary by CodeRabbit
Chores
Bug Fixes