Repository navigation
fix(native-teams): unregisterNativeMember actually removes the entry - #1180
Conversation
Closes #1179. `unregisterNativeMember` marked `isActive: false` and left the member in the array, citing "preserves history". No reader consults inactive entries for history. Two real readers do pick them up, silently: 1. `resolveNativeMemberName` has an explicit active→inactive fallback (test: "prefers active members over inactive"). A stale entry from a prior team membership silently claims messages addressed by name. 2. `findTeamsContainingAgent` — the spawn-resolver tier 5 — matches members by `name` regardless of `isActive`. A stale entry routes future spawns into the wrong team. Observed on a live server: `~/.claude/teams/genie/config.json` had 5 members from a spawn-storm — 2 real plus 3 killed workers (simone@genie, patch-test@genie, genie-3@genie) all `isActive: false`. Every bare `genie spawn <name>` hit the stale fallback chain before resolving. Fix: remove the member from the array. The per-member inbox at ~/.claude/teams/<team>/inboxes/<name>.json is cleared by `clearNativeInbox` (called sibling to this in the kill path), so no residual state is worth preserving. Tests: 5 new cases in claude-native-teams.test.ts (basic removal, no-op on missing member, no-op on missing team, findTeamsContainingAgent regression, resolveNativeMemberName regression). Full suite: 2601/2601. 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 modifies the unregisterNativeMember function to completely remove member entries from the team configuration instead of simply marking them as inactive. This change addresses issues where stale, inactive entries were still being processed by other system components. Accompanying tests have been added to ensure correct behavior and prevent regressions. Feedback suggests that the isActive property and related fallback logic may now be obsolete and should be considered for removal in future maintenance.
| * Removes the member entry from the `members` array. The prior implementation | ||
| * marked `isActive: false` "to preserve history", but no call site ever | ||
| * consults inactive members for history — meanwhile two active readers | ||
| * (`resolveNativeMemberName`'s active→inactive fallback and | ||
| * `findTeamsContainingAgent`'s team-resolver tier 5) silently pick up stale | ||
| * entries, routing messages and spawn-team resolution to the wrong worker. |
There was a problem hiding this comment.
Since unregisterNativeMember now removes the member from the array instead of marking it as inactive, the isActive property on the NativeTeamMember interface and its associated logic in other functions (such as the fallback in resolveNativeMemberName) appear to be redundant. While out of scope for this immediate fix, consider cleaning these up in a future PR to improve maintainability and avoid confusion about the existence of 'inactive' members.
Closes #1179.
Summary
`unregisterNativeMember` marked `isActive: false` and left the member in the array. No reader consults inactive entries for history, but two active readers silently pick them up:
Observed live: `~/.claude/teams/genie/config.json` had 5 members after a spawn-storm — 2 real plus 3 killed workers all `isActive: false`. Every bare `genie spawn ` hit the stale fallback chain before resolving. Manually purging the file stopped the ghost `teammate-message` announcements.
Fix
`unregisterNativeMember` now removes the member from the array. The per-member inbox at `~/.claude/teams//inboxes/.json` is already cleared by `clearNativeInbox` (called sibling to this in the kill path), so no residual state is worth preserving.
Tests
5 new cases in `claude-native-teams.test.ts`:
The existing `'prefers active members over inactive'` test still passes — `resolveNativeMemberName`'s fallback logic is unchanged; it just never fires because unregistered members are no longer in the array.
Full suite: 2601 pass / 0 fail.
Test plan
🤖 Generated with Claude Code