Skip to content

fix(v0.2): transport-aware liveness at 5 callers of isPaneAlive (complements #1167) #1186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
automagik-genie merged 4 commits into from
Apr 18, 2026
Merged

fix(v0.2): transport-aware liveness at 5 callers of isPaneAlive (complements #1167) #1186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99fee15
fix(v0.2): add resolveWorkerLivenessByTransport helper
Apr 18, 2026
8e47d35
fix(v0.2): transport-aware liveness at sites 1-3 in agents.ts
Apr 18, 2026
d7e0710
fix(v0.2): transport-aware liveness at sites 4-7
Apr 18, 2026
83a5b57
test(v0.2): regression coverage for transport-aware liveness sweep
Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/__tests__/tui-spawn-dx.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ async function killPane(id: string): Promise<void> {
}

/** Controllable isAliveFn — returns true for any paneId seeded in the alive set. */
function alivePanes(...alive: string[]): (paneId: string) => Promise<boolean> {
function alivePanes(...alive: string[]): (agent: { id: string; paneId: string }) => Promise<boolean> {
const set = new Set(alive);
return async (paneId: string) => set.has(paneId);
return async (agent: { id: string; paneId: string }) => set.has(agent.paneId);
Comment on lines +110 to +112

@gemini-code-assist gemini-code-assist Bot Apr 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The signature of alivePanes should be updated to allow paneId to be nullable, matching the updated signature of resolveWorkerLivenessByTransport and the underlying Agent type.

function alivePanes(...alive: string[]): (agent: { id: string; paneId: string | null | undefined }) => Promise<boolean> {
  const set = new Set(alive);
  return async (agent: { id: string; paneId: string | null | undefined }) => !!agent.paneId && set.has(agent.paneId);

}

/** Stub crypto.randomUUID in a scoped fashion — restore after each test. */
Expand Down
8 changes: 6 additions & 2 deletions src/hooks/handlers/auto-spawn.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,16 @@ function extractAutoSpawnTarget(payload: HookPayload): { recipient: string; team
/** Find and execute a spawn template for the recipient. */
async function executeAutoSpawn(recipient: string, teamName: string): Promise<void> {
const registryMod = await import('../../lib/agent-registry.js');
const tmuxMod = await import('../../lib/tmux.js');
const executorRegistryMod = await import('../../lib/executor-registry.js');
const directoryMod = await import('../../lib/agent-directory.js');

const agents = await registryMod.list();
const existing = agents.find((a) => (a.role === recipient || a.id === recipient) && a.team === teamName);
if (existing && (await tmuxMod.isPaneAlive(existing.paneId))) return;
// Transport-aware liveness: a plain `isPaneAlive` check treats live SDK/
// omni/inline recipients (synthetic paneIds like 'sdk', '', 'inline') as
// dead and triggers a duplicate spawn on every message. Dispatch by
// paneId shape so non-tmux transports consult `executors.state` instead.
if (existing && (await executorRegistryMod.resolveWorkerLivenessByTransport(existing))) return;

const dirEntry = await directoryMod.resolve(recipient);
const templates = await registryMod.listTemplates();
Expand Down
308 changes: 308 additions & 0 deletions src/lib/__tests__/transport-aware-liveness.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,308 @@
/**
* Regression tests for the v0.2 transport-aware-liveness sweep.
*
* Complements PR #1167 (`buildWorkerStatusMap` / `resolveWorkerLiveness`) by
* covering the parallel callers that still called `isPaneAlive` blindly for
* every worker — mis-reporting SDK/omni/inline agents (synthetic paneIds like
* 'sdk', 'inline', '') as dead and triggering clobbers, dup-spawns, and
* misrouted messages.
*
* Reference pattern: `scheduler-daemon.ts:countActiveWorkers` (PR #1181) and
* `term-commands/agents.ts:resolveWorkerLiveness`. The shared helper lives in
* `executor-registry.ts:resolveWorkerLivenessByTransport`.
*
* Site coverage:
* - Helper: `resolveWorkerLivenessByTransport` dispatch (unit, no DB).
* - Site 3: `resolveSpawnIdentity` with live SDK canonical → parallel, not
* canonical clobber (integration, DB).
* - Site 6: `isAgentAlive` with live SDK agent → true (integration, DB).
* - Site 2 + Site 7: covered transitively by the helper dispatch tests —
* both callers pass their registry row straight to
* `resolveWorkerLivenessByTransport`, so the helper contract is the fix.
*/

import { afterAll, beforeAll, beforeEach, describe, expect, test } from 'bun:test';
import * as registry from '../agent-registry.js';
import { resolveWorkerLivenessByTransport } from '../executor-registry.js';
import { DB_AVAILABLE, setupTestSchema } from '../test-db.js';

// ---------------------------------------------------------------------------
// Helper unit tests — exhaustive dispatch coverage without DB.
// ---------------------------------------------------------------------------

describe('resolveWorkerLivenessByTransport (shared helper)', () => {
test('dispatches tmux paneIds (%N) to isPaneAliveFn', async () => {
const paneCalls: string[] = [];
const execCalls: string[] = [];
const alive = await resolveWorkerLivenessByTransport(
{ id: 'alice', paneId: '%42' },
{
isPaneAliveFn: async (p) => {
paneCalls.push(p);
return true;
},
isExecutorAliveFn: async (id) => {
execCalls.push(id);
return false;
},
},
);

expect(alive).toBe(true);
expect(paneCalls).toEqual(['%42']);
expect(execCalls).toEqual([]); // never consulted
});

test('dispatches synthetic paneIds (sdk) to isExecutorAliveFn', async () => {
const paneCalls: string[] = [];
const execCalls: string[] = [];
const alive = await resolveWorkerLivenessByTransport(
{ id: 'alice-sdk', paneId: 'sdk' },
{
isPaneAliveFn: async (p) => {
paneCalls.push(p);
return false;
},
isExecutorAliveFn: async (id) => {
execCalls.push(id);
return true;
},
},
);

expect(alive).toBe(true);
expect(paneCalls).toEqual([]); // never consulted — would have returned false
expect(execCalls).toEqual(['alice-sdk']);
});

test('dispatches empty paneId to isExecutorAliveFn', async () => {
const execCalls: string[] = [];
const alive = await resolveWorkerLivenessByTransport(
{ id: 'alice-inline', paneId: '' },
{
isPaneAliveFn: async () => {
throw new Error('should not call tmux for empty paneId');
},
isExecutorAliveFn: async (id) => {
execCalls.push(id);
return true;
},
},
);

expect(alive).toBe(true);
expect(execCalls).toEqual(['alice-inline']);
});

test('inline paneId routes to isExecutorAliveFn', async () => {
const alive = await resolveWorkerLivenessByTransport(
{ id: 'alice-inline', paneId: 'inline' },
{
isPaneAliveFn: async () => {
throw new Error('should not call tmux for inline paneId');
},
isExecutorAliveFn: async () => true,
},
);
expect(alive).toBe(true);
});

test('dead synthetic worker → false (no tmux call)', async () => {
let paneCalled = false;
const alive = await resolveWorkerLivenessByTransport(
{ id: 'alice-sdk', paneId: 'sdk' },
{
isPaneAliveFn: async () => {
paneCalled = true;
return true;
},
isExecutorAliveFn: async () => false,
},
);
expect(alive).toBe(false);
expect(paneCalled).toBe(false);
});

test('%N with dead pane stays dead (no executor fallback)', async () => {
// Regression: the tmux branch must NOT fall through to isExecutorAlive
// when the pane is dead. Prevents false-alive for a tmux-transport agent
// that orphaned its executor row.
let execCalled = false;
const alive = await resolveWorkerLivenessByTransport(
{ id: 'alice', paneId: '%99' },
{
isPaneAliveFn: async () => false,
isExecutorAliveFn: async () => {
execCalled = true;
return true;
},
},
);
expect(alive).toBe(false);
expect(execCalled).toBe(false);
});
});

// ---------------------------------------------------------------------------
// Integration tests — DB-backed, exercises real executors rows.
// ---------------------------------------------------------------------------

describe.skipIf(!DB_AVAILABLE)('transport-aware liveness — integration', () => {
let cleanupSchema: () => Promise<void>;

beforeAll(async () => {
cleanupSchema = await setupTestSchema();
});

afterAll(async () => {
await cleanupSchema();
});

beforeEach(async () => {
const { getConnection } = await import('../db.js');
const sql = await getConnection();
await sql`TRUNCATE TABLE agents CASCADE`;
await sql`TRUNCATE TABLE executors CASCADE`;
});

// ---------------------------------------------------------------------
// Site 6 — `isAgentAlive` in team-auto-spawn.ts
// Before: SDK agent with live executor → `isPaneAlive('sdk')` returned
// false → inbox-watcher misrouted messages as "agent dead".
// After: same scenario → transport-aware helper consults executors.state,
// returns true, routing proceeds normally.
// ---------------------------------------------------------------------
test('Site 6: isAgentAlive reports live SDK agent as alive', async () => {
const { isAgentAlive } = await import('../team-auto-spawn.js');
const { getConnection } = await import('../db.js');
const sql = await getConnection();

// Seed SDK agent row (synthetic paneId='sdk').
await registry.register({
id: 'alice-sdk',
paneId: 'sdk',
session: 'alice',
worktree: null,
startedAt: new Date().toISOString(),
state: 'idle',
lastStateChange: new Date().toISOString(),
repoPath: '/tmp/alice-sdk',
role: 'alice-sdk',
team: 'alice',
provider: 'claude-sdk',
});
// Seed live executor + FK link (running state is in the "alive" set).
await sql`
INSERT INTO executors (id, agent_id, provider, transport, state, started_at)
VALUES ('exec-1', 'alice-sdk', 'claude-sdk', 'api', 'running', now())
`;
await sql`UPDATE agents SET current_executor_id = 'exec-1' WHERE id = 'alice-sdk'`;

expect(await isAgentAlive('alice-sdk')).toBe(true);
});

test('Site 6: isAgentAlive reports dead SDK agent as dead', async () => {
const { isAgentAlive } = await import('../team-auto-spawn.js');
const { getConnection } = await import('../db.js');
const sql = await getConnection();

await registry.register({
id: 'bob-sdk',
paneId: 'sdk',
session: 'bob',
worktree: null,
startedAt: new Date().toISOString(),
state: 'done',
lastStateChange: new Date().toISOString(),
repoPath: '/tmp/bob-sdk',
role: 'bob-sdk',
team: 'bob',
provider: 'claude-sdk',
});
// Seed terminated executor — getLiveExecutorState returns null.
await sql`
INSERT INTO executors (id, agent_id, provider, transport, state, started_at, ended_at)
VALUES ('exec-2', 'bob-sdk', 'claude-sdk', 'api', 'terminated', now(), now())
`;
await sql`UPDATE agents SET current_executor_id = 'exec-2' WHERE id = 'bob-sdk'`;

expect(await isAgentAlive('bob-sdk')).toBe(false);
});

// ---------------------------------------------------------------------
// Site 3 — `resolveSpawnIdentity` default liveness fn is transport-aware
// Before: SDK canonical with paneId='sdk' → isPaneAlive('sdk')=false →
// resolveSpawnIdentity returned 'canonical' → ON CONFLICT UPDATE
// rewrote the live SDK row's session UUID (clobber).
// After: same scenario → resolveWorkerLivenessByTransport consults
// executors.state, returns true → 'parallel', live row preserved.
// ---------------------------------------------------------------------
test('Site 3: resolveSpawnIdentity routes live SDK canonical to parallel', async () => {
const { resolveSpawnIdentity } = await import('../../term-commands/agents.js');
const { getConnection } = await import('../db.js');
const sql = await getConnection();

// Seed SDK canonical + live executor.
await registry.register({
id: 'stefani',
paneId: 'sdk',
session: 'stefani',
worktree: null,
startedAt: new Date().toISOString(),
state: 'working',
lastStateChange: new Date().toISOString(),
repoPath: '/tmp/stefani',
claudeSessionId: 'original-canonical-uuid-0000-000000000000',
role: 'stefani',
team: 'stefani',
provider: 'claude-sdk',
});
await sql`
INSERT INTO executors (id, agent_id, provider, transport, state, started_at)
VALUES ('exec-stefani', 'stefani', 'claude-sdk', 'api', 'working', now())
`;
await sql`UPDATE agents SET current_executor_id = 'exec-stefani' WHERE id = 'stefani'`;

const parallelUuid = 'feedface-1234-5678-9abc-abcdef012345';
const identity = await resolveSpawnIdentity('stefani', 'stefani', () => parallelUuid);

// Must route to parallel — canonical row is live and must not be rewritten.
expect(identity.kind).toBe('parallel');
if (identity.kind === 'parallel') {
expect(identity.canonicalId).toBe('stefani');
expect(identity.workerId).toBe('stefani-feed');
}

// Canonical row is byte-identical to the seed.
const canonical = await registry.get('stefani');
expect(canonical?.claudeSessionId).toBe('original-canonical-uuid-0000-000000000000');
expect(canonical?.paneId).toBe('sdk');
});

test('Site 3: resolveSpawnIdentity treats dead SDK canonical as dead', async () => {
// Complementary regression: if the executor is terminated, the dead-row
// branch must still fire (returns canonical for recovery).
const { resolveSpawnIdentity } = await import('../../term-commands/agents.js');

await registry.register({
id: 'zombie',
paneId: 'sdk',
session: 'zombie',
worktree: null,
startedAt: new Date().toISOString(),
state: 'error',
lastStateChange: new Date().toISOString(),
repoPath: '/tmp/zombie',
claudeSessionId: 'stale-uuid-0000-0000-0000-000000000000',
role: 'zombie',
team: 'zombie',
provider: 'claude-sdk',
});
// No current_executor_id → getLiveExecutorState returns null → dead.

const identity = await resolveSpawnIdentity('zombie', 'zombie', () => 'fresh-uuid');
expect(identity.kind).toBe('canonical');
expect(identity.workerId).toBe('zombie');
expect(identity.sessionUuid).toBe('fresh-uuid');
});
});
36 changes: 36 additions & 0 deletions src/lib/executor-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,39 @@ export async function getLiveExecutorState(agentId: string): Promise<ExecutorSta
export async function isExecutorAlive(agentId: string): Promise<boolean> {
return (await getLiveExecutorState(agentId)) !== null;
}

/**
* Transport-aware liveness check for a worker row.
*
* Dispatches on paneId shape:
* - tmux pane (`%N`) → `isPaneAliveFn(paneId)` (authoritative for tmux)
* - synthetic id (`sdk`, `inline`, `''`, etc.) → `isExecutorAliveFn(agentId)`
* which consults `executors.state` — the live signal for non-tmux transports.
*
* Unifies the five parallel call-sites that previously called `isPaneAlive`
* blindly (PR #1167 + this sweep). Mirrors the `%\d+` regex-guard pattern from
* `scheduler-daemon.ts:countActiveWorkers` and
* `term-commands/agents.ts:resolveWorkerLiveness`.
*
* Test injection: both `isPaneAliveFn` and `isExecutorAliveFn` are overridable
* so unit tests can exercise the branch logic without real tmux or PG.
*/
export async function resolveWorkerLivenessByTransport(
worker: { id: string; paneId: string },
Comment on lines +304 to +305

@gemini-code-assist gemini-code-assist Bot Apr 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The paneId property in the worker parameter should be nullable to match the Agent type in the registry, where paneId can be null or undefined (e.g., for agents that haven't been assigned a transport yet). Updating the type here will prevent potential TypeScript errors at call sites like rejectDuplicateRole in agents.ts or autoSpawn in auto-spawn.ts.

export async function resolveWorkerLivenessByTransport(
  worker: { id: string; paneId: string | null | undefined },

opts?: {
isPaneAliveFn?: (paneId: string) => Promise<boolean>;
isExecutorAliveFn?: (agentId: string) => Promise<boolean>;
},
): Promise<boolean> {
if (/^%\d+$/.test(worker.paneId)) {

@gemini-code-assist gemini-code-assist Bot Apr 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The regex /^%\d+$/ is used to identify tmux pane IDs. Since paneId can be nullable, it's safer to check for its existence before testing. Additionally, this regex is duplicated in src/term-commands/agents.ts. Consider defining a shared utility or constant for this check to improve maintainability.

  if (worker.paneId && /^%\d+$/.test(worker.paneId)) {

const fn =
opts?.isPaneAliveFn ??
(async (pane: string) => {
const { isPaneAlive } = await import('./tmux.js');
return isPaneAlive(pane);
});
return fn(worker.paneId);
}
const fn = opts?.isExecutorAliveFn ?? isExecutorAlive;
return fn(worker.id);
}
Loading
Loading