Repository navigation
feat: team-lead liveness checks, inbox watcher daemon, CI coverage gate - #645
Conversation
- Wire isPaneAlive() into isTeamActive() so dead team-lead processes are detected - Store team-lead pane ID in agent-registry for tracking and auto-respawn - Add 30s grace period to prevent false negatives during slow startup - New inbox-watcher daemon polls native inboxes every 30s, spawns offline team-leads - Backoff after 3 failed spawn attempts to prevent crash loops - CI coverage gate enforces 68% minimum line coverage threshold - Add saveTeamLeadEntry/getTeamLeadEntry to agent-registry - Add listTeamsWithUnreadInbox to claude-native-teams
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the robustness and automation of team-lead management by introducing liveness checks, persistent tracking of team-lead processes, and an inbox-driven auto-respawn mechanism. It also strengthens code quality by enforcing a minimum test coverage in the continuous integration pipeline. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several significant features, including a liveness check for team leads, an inbox watcher daemon for auto-spawning, and agent registry tracking. The implementation is generally robust, leveraging dependency injection for testability which is a great practice. I've identified a couple of areas for performance improvement by parallelizing sequential operations. More critically, there's a bug in team-auto-spawn.ts where incorrect sanitization logic is used to locate tmux windows, which could cause liveness checks and stale window cleanup to fail. My review includes suggestions to fix this and to improve performance.
| const sanitized = sanitizeTeamName(teamName); | ||
| return windows.some((w) => w.name === sanitized || w.name === teamName); | ||
| const matchingWindow = windows.find((w) => w.name === sanitized || w.name === teamName); |
There was a problem hiding this comment.
There's a potential bug in how the tmux window is located. The code uses sanitizeTeamName to find the window, but windows are created using sanitizeWindowName. These two functions have different logic (e.g., sanitizeTeamName lowercases, sanitizeWindowName does not), which can lead to isTeamActive failing to find an existing window for team names with uppercase letters or periods. You should consistently use sanitizeWindowName for finding tmux windows.
| const sanitized = sanitizeTeamName(teamName); | |
| return windows.some((w) => w.name === sanitized || w.name === teamName); | |
| const matchingWindow = windows.find((w) => w.name === sanitized || w.name === teamName); | |
| const windowName = sanitizeWindowName(teamName); | |
| const matchingWindow = windows.find((w) => w.name === windowName); |
| const sanitized = sanitizeTeamName(teamName); | ||
| const staleWindow = windows.find((w) => w.name === sanitized || w.name === teamName || w.name === windowName); |
There was a problem hiding this comment.
Similar to the issue in isTeamActive, the logic to find a stale window is overly complex and uses sanitizeTeamName, which can be incorrect. It should be simplified to only use sanitizeWindowName, which is how window names are created.
| const sanitized = sanitizeTeamName(teamName); | |
| const staleWindow = windows.find((w) => w.name === sanitized || w.name === teamName || w.name === windowName); | |
| const staleWindow = windows.find((w) => w.name === windowName); |
| const results: Array<{ teamName: string; unreadCount: number; workingDir: string | null }> = []; | ||
|
|
||
| for (const name of teamDirs) { | ||
| // Read inbox messages | ||
| const inboxFile = join(base, name, 'inboxes', 'team-lead.json'); | ||
| let messages: NativeInboxMessage[]; | ||
| try { | ||
| const content = await readFile(inboxFile, 'utf-8'); | ||
| messages = JSON.parse(content); | ||
| } catch { | ||
| continue; // No inbox or invalid JSON | ||
| } | ||
|
|
||
| if (!Array.isArray(messages)) continue; | ||
|
|
||
| const unreadCount = messages.filter((m) => m.read === false).length; | ||
| if (unreadCount === 0) continue; | ||
|
|
||
| // Get workingDir from config.json → members → team-lead → cwd | ||
| let workingDir: string | null = null; | ||
| try { | ||
| const cfgContent = await readFile(join(base, name, 'config.json'), 'utf-8'); | ||
| const config: NativeTeamConfig = JSON.parse(cfgContent); | ||
| const leadMember = config.members.find((m) => m.name === 'team-lead' || m.agentId.startsWith('team-lead@')); | ||
| if (leadMember?.cwd) { | ||
| workingDir = leadMember.cwd; | ||
| } | ||
| } catch { | ||
| // Config missing or malformed — workingDir stays null | ||
| } | ||
|
|
||
| results.push({ teamName: name, unreadCount, workingDir }); | ||
| } | ||
|
|
||
| return results; |
There was a problem hiding this comment.
The for...of loop processes each team directory sequentially due to the await calls inside for reading files. This could be slow if there are many teams. You can improve performance by processing teams in parallel using Promise.all.
const results = await Promise.all(
teamDirs.map(async (name) => {
// Read inbox messages
const inboxFile = join(base, name, 'inboxes', 'team-lead.json');
let messages: NativeInboxMessage[];
try {
const content = await readFile(inboxFile, 'utf-8');
messages = JSON.parse(content);
} catch {
return null; // No inbox or invalid JSON
}
if (!Array.isArray(messages)) return null;
const unreadCount = messages.filter((m) => m.read === false).length;
if (unreadCount === 0) return null;
// Get workingDir from config.json → members → team-lead → cwd
let workingDir: string | null = null;
try {
const cfgContent = await readFile(join(base, name, 'config.json'), 'utf-8');
const config: NativeTeamConfig = JSON.parse(cfgContent);
const leadMember = config.members.find((m) => m.name === 'team-lead' || m.agentId.startsWith('team-lead@'));
if (leadMember?.cwd) {
workingDir = leadMember.cwd;
}
} catch {
// Config missing or malformed — workingDir stays null
}
return { teamName: name, unreadCount, workingDir };
}),
);
return results.filter((r): r is { teamName: string; unreadCount: number; workingDir: string | null } => r !== null);| const spawned: string[] = []; | ||
|
|
||
| for (const { teamName, workingDir } of teamsWithUnread) { | ||
| // Skip teams that have exceeded max spawn failures | ||
| const failures = spawnFailures.get(teamName) ?? 0; | ||
| if (failures >= MAX_SPAWN_FAILURES) { | ||
| deps.warn(`[inbox-watcher] Skipping team "${teamName}" — ${failures} consecutive spawn failures`); | ||
| continue; | ||
| } | ||
|
|
||
| // Skip teams that already have an active team-lead | ||
| const active = await deps.isTeamActive(teamName); | ||
| if (active) continue; | ||
|
|
||
| // No working dir means we can't spawn | ||
| if (!workingDir) { | ||
| deps.warn(`[inbox-watcher] Cannot spawn team-lead for "${teamName}" — no workingDir in config`); | ||
| continue; | ||
| } | ||
|
|
||
| // Attempt to spawn team-lead | ||
| try { | ||
| await deps.ensureTeamLead(teamName, workingDir); | ||
| spawnFailures.set(teamName, 0); // Reset on success | ||
| spawned.push(teamName); | ||
| } catch (err) { | ||
| const newCount = failures + 1; | ||
| spawnFailures.set(teamName, newCount); | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| deps.warn( | ||
| `[inbox-watcher] Failed to spawn team-lead for "${teamName}" (attempt ${newCount}/${MAX_SPAWN_FAILURES}): ${message}`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return spawned; |
There was a problem hiding this comment.
The for...of loop processes each team with unread messages sequentially. Since each team check is independent, you can parallelize these operations using Promise.all to improve performance, especially when multiple teams need spawning.
const spawnPromises = teamsWithUnread.map(async ({ teamName, workingDir }) => {
// Skip teams that have exceeded max spawn failures
const failures = spawnFailures.get(teamName) ?? 0;
if (failures >= MAX_SPAWN_FAILURES) {
deps.warn(`[inbox-watcher] Skipping team "${teamName}" — ${failures} consecutive spawn failures`);
return null;
}
// Skip teams that already have an active team-lead
const active = await deps.isTeamActive(teamName);
if (active) return null;
// No working dir means we can't spawn
if (!workingDir) {
deps.warn(`[inbox-watcher] Cannot spawn team-lead for "${teamName}" — no workingDir in config`);
return null;
}
// Attempt to spawn team-lead
try {
await deps.ensureTeamLead(teamName, workingDir);
spawnFailures.set(teamName, 0); // Reset on success
return teamName;
} catch (err) {
const newCount = failures + 1;
spawnFailures.set(teamName, newCount);
const message = err instanceof Error ? err.message : String(err);
deps.warn(
`[inbox-watcher] Failed to spawn team-lead for "${teamName}" (attempt ${newCount}/${MAX_SPAWN_FAILURES}): ${message}`,
);
return null;
}
});
const spawned = (await Promise.all(spawnPromises)).filter((t): t is string => t !== null);
return spawned;![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: ced87a7640
ℹ️ 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".
| try { | ||
| const cfgContent = await readFile(join(base, name, 'config.json'), 'utf-8'); | ||
| const config: NativeTeamConfig = JSON.parse(cfgContent); | ||
| const leadMember = config.members.find((m) => m.name === 'team-lead' || m.agentId.startsWith('team-lead@')); |
There was a problem hiding this comment.
listTeamsWithUnreadInbox() assumes the lead entry is named team-lead, but teams created through the normal session bootstrap path register the leader as basename(cwd) (see ensureNativeTeamForLeader() in src/genie-commands/session.ts), so this lookup often returns no match and leaves workingDir null. In that case checkInboxes() skips spawning entirely, which means unread inboxes for those existing teams will never trigger auto-respawn.
Useful? React with 👍 / 👎.
| const message = err instanceof Error ? err.message : String(err); | ||
| deps.warn(`[inbox-watcher] Poll error: ${message}`); | ||
| }); | ||
| }, getInboxPollIntervalMs()); |
There was a problem hiding this comment.
The module documents GENIE_INBOX_POLL_MS=0 as disabled, and checkInboxes() returns early for that case, but startInboxWatcher() still creates setInterval(..., 0). That schedules a tight loop that repeatedly wakes up and immediately returns, causing unnecessary CPU churn in the exact configuration intended to disable polling.
Useful? React with 👍 / 👎.
| run: | | ||
| COVERAGE_OUTPUT=$(bun test --coverage 2>&1) | ||
| echo "$COVERAGE_OUTPUT" | ||
| LINE_COV=$(echo "$COVERAGE_OUTPUT" | grep "All files" | awk -F'|' '{print $2}' | tr -d ' ') |
There was a problem hiding this comment.
The coverage row format is All files | % Funcs | % Lines | ..., but the script extracts field 2 (awk -F'|' '{print $2}'), which is function coverage, then labels it as line coverage. This makes the gate enforce the wrong metric and can fail or pass builds contrary to the intended 68% line-coverage policy.
Useful? React with 👍 / 👎.
| run: | | ||
| COVERAGE_OUTPUT=$(bun test --coverage 2>&1) | ||
| echo "$COVERAGE_OUTPUT" | ||
| LINE_COV=$(echo "$COVERAGE_OUTPUT" | grep "All files" | awk -F'|' '{print $2}' | tr -d ' ') |
There was a problem hiding this comment.
The fallback branch for unparseable coverage is unreachable when grep "All files" finds no match, because this step runs with -e -o pipefail and the command substitution fails before if [ -z "$LINE_COV" ] executes. As a result, any Bun output format change hard-fails CI instead of taking the intended graceful-degradation path.
Useful? React with 👍 / 👎.
|
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)
📝 Coding Plan
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 |
1 participant
Summary
Fixes #531, #526, partial #574 (Phase 1-2).
isTeamActive()now verifies process liveness viaisPaneAlive(), not just tmux window existence. 30s grace period prevents false negatives during startup.saveTeamLeadEntry()/getTeamLeadEntry()helpers.inbox-watcher.tspolls~/.claude/teams/*/inboxes/team-lead.jsonevery 30s. Auto-spawns offline team-leads on unread messages. 3-attempt backoff prevents crash loops.Files Changed (8 files, +772/-40)
src/lib/team-auto-spawn.ts— Liveness check + pane ID storagesrc/lib/agent-registry.ts— Team-lead entry helperssrc/lib/team-auto-spawn.test.ts— 5 new liveness/registry testssrc/lib/inbox-watcher.ts— NEW: inbox polling daemonsrc/lib/inbox-watcher.test.ts— NEW: 5 watcher unit testssrc/lib/claude-native-teams.ts—listTeamsWithUnreadInbox()helper.github/workflows/ci.yml— Coverage threshold enforcementknip.json— Updated ignore listTest plan