fix(lifecycle): implement stuck detection using agent-stuck threshold (#376)
* fix(lifecycle): implement stuck detection using agent-stuck threshold The agent-stuck reaction config supported a threshold field (e.g. "10m"), but determineStatus() never returned "stuck" — there was no code path that consumed the threshold or transitioned sessions based on idle time. Sessions would stay parked at pr_open/working forever even when the agent had been idle for hours. Added idle-time check in determineStatus(): when getActivityState() reports "idle" or "blocked" with a timestamp, compare the idle duration against the agent-stuck.threshold config. If exceeded, return "stuck" so the reaction system can fire notifications. Also removed the priority !== "info" guard on transition notifications, so all priority levels (including info) are routed through notificationRouting. This lets the config control which notifiers receive each priority level, rather than silently dropping info-level transition events. * fix(lifecycle): add post-PR stuck detection as safety net The original stuck check in step 2 (before PR checks) can be bypassed when getActivityState() returns null (session file not found, cache miss, I/O failure). When this happens, the code falls through to the PR path which returns 'pr_open' without ever checking idle duration. Fix: extract isIdleBeyondThreshold() helper and call it in three places: 1. Step 2: before PR checks (fast path, catches most cases) 2. Step 4b: after PR checks return 'pr_open' (safety net) 3. Step 5: after all checks, for agents that finish without a PR This ensures stuck detection fires even when the JSONL activity detection fails to return idle state. Sessions can no longer get permanently stuck at 'pr_open' when the agent has been idle beyond the threshold. Also removes the debug console.error calls from the previous commit. * fix(lifecycle): treat reviewDecision 'none' as approved for merge readiness PRs with no required reviewers never reached 'mergeable' status because getReviewDecision returned 'none', which was not handled. The lifecycle poll fell through to 'review_pending' or the default, so merge.ready never fired and the approved-and-green reaction never triggered. Also: skip stuck short-circuit when session has an open PR so merge readiness checks in step 4 can still run. Without this, idle agents with open PRs get stuck status and never transition to mergeable. Closes composio#0 (internal fix) * fix(opencode): classify activity state from session timestamps * test(lifecycle): cover opencode idle-threshold stuck transition * fix(lifecycle): preserve global stuck threshold with project overrides * fix(lifecycle): run PR auto-detection before stuck transitions --------- Co-authored-by: Harsh <harshb012@gmail.com>
This commit is contained in:
parent
1d960feb6b
commit
a64c051e9f
|
|
@ -382,6 +382,162 @@ describe("check (single session)", () => {
|
|||
expect(lm.getStates().get("app-1")).toBe("needs_input");
|
||||
});
|
||||
|
||||
it("transitions to stuck when idle exceeds agent-stuck threshold (OpenCode-style activity)", async () => {
|
||||
config.reactions = {
|
||||
"agent-stuck": {
|
||||
auto: true,
|
||||
action: "notify",
|
||||
threshold: "1m",
|
||||
},
|
||||
};
|
||||
|
||||
vi.mocked(mockAgent.getActivityState).mockResolvedValue({
|
||||
state: "idle",
|
||||
timestamp: new Date(Date.now() - 120_000),
|
||||
});
|
||||
|
||||
const session = makeSession({ status: "working", metadata: { agent: "opencode" } });
|
||||
vi.mocked(mockSessionManager.get).mockResolvedValue(session);
|
||||
|
||||
writeMetadata(sessionsDir, "app-1", {
|
||||
worktree: "/tmp",
|
||||
branch: "main",
|
||||
status: "working",
|
||||
project: "my-app",
|
||||
agent: "opencode",
|
||||
});
|
||||
|
||||
const lm = createLifecycleManager({
|
||||
config,
|
||||
registry: mockRegistry,
|
||||
sessionManager: mockSessionManager,
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
|
||||
expect(lm.getStates().get("app-1")).toBe("stuck");
|
||||
});
|
||||
|
||||
it("uses global agent-stuck threshold when project override omits threshold", async () => {
|
||||
config.reactions = {
|
||||
"agent-stuck": {
|
||||
auto: true,
|
||||
action: "notify",
|
||||
threshold: "1m",
|
||||
},
|
||||
};
|
||||
config.projects["my-app"] = {
|
||||
...config.projects["my-app"],
|
||||
reactions: {
|
||||
"agent-stuck": {
|
||||
auto: true,
|
||||
action: "notify",
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
vi.mocked(mockAgent.getActivityState).mockResolvedValue({
|
||||
state: "idle",
|
||||
timestamp: new Date(Date.now() - 120_000),
|
||||
});
|
||||
|
||||
const session = makeSession({ status: "working", metadata: { agent: "opencode" } });
|
||||
vi.mocked(mockSessionManager.get).mockResolvedValue(session);
|
||||
|
||||
writeMetadata(sessionsDir, "app-1", {
|
||||
worktree: "/tmp",
|
||||
branch: "main",
|
||||
status: "working",
|
||||
project: "my-app",
|
||||
agent: "opencode",
|
||||
});
|
||||
|
||||
const lm = createLifecycleManager({
|
||||
config,
|
||||
registry: mockRegistry,
|
||||
sessionManager: mockSessionManager,
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
|
||||
expect(lm.getStates().get("app-1")).toBe("stuck");
|
||||
});
|
||||
|
||||
it("still auto-detects PR before marking idle sessions as stuck", async () => {
|
||||
config.reactions = {
|
||||
"agent-stuck": {
|
||||
auto: true,
|
||||
action: "notify",
|
||||
threshold: "1m",
|
||||
},
|
||||
};
|
||||
|
||||
const mockSCM: SCM = {
|
||||
name: "mock-scm",
|
||||
detectPR: vi.fn().mockResolvedValue(makePR()),
|
||||
getPRState: vi.fn().mockResolvedValue("open"),
|
||||
mergePR: vi.fn(),
|
||||
closePR: vi.fn(),
|
||||
getCIChecks: vi.fn(),
|
||||
getCISummary: vi.fn().mockResolvedValue("passing"),
|
||||
getReviews: vi.fn(),
|
||||
getReviewDecision: vi.fn().mockResolvedValue("none"),
|
||||
getPendingComments: vi.fn(),
|
||||
getAutomatedComments: vi.fn(),
|
||||
getMergeability: vi.fn().mockResolvedValue({
|
||||
mergeable: false,
|
||||
ciPassing: true,
|
||||
approved: false,
|
||||
noConflicts: true,
|
||||
blockers: [],
|
||||
}),
|
||||
};
|
||||
|
||||
const registryWithSCM: PluginRegistry = {
|
||||
...mockRegistry,
|
||||
get: vi.fn().mockImplementation((slot: string) => {
|
||||
if (slot === "runtime") return mockRuntime;
|
||||
if (slot === "agent") return mockAgent;
|
||||
if (slot === "scm") return mockSCM;
|
||||
return null;
|
||||
}),
|
||||
};
|
||||
|
||||
vi.mocked(mockAgent.getActivityState).mockResolvedValue({
|
||||
state: "idle",
|
||||
timestamp: new Date(Date.now() - 120_000),
|
||||
});
|
||||
|
||||
const session = makeSession({
|
||||
status: "working",
|
||||
branch: "feat/test",
|
||||
pr: null,
|
||||
metadata: { agent: "opencode" },
|
||||
});
|
||||
vi.mocked(mockSessionManager.get).mockResolvedValue(session);
|
||||
|
||||
writeMetadata(sessionsDir, "app-1", {
|
||||
worktree: "/tmp",
|
||||
branch: "feat/test",
|
||||
status: "working",
|
||||
project: "my-app",
|
||||
agent: "opencode",
|
||||
});
|
||||
|
||||
const lm = createLifecycleManager({
|
||||
config,
|
||||
registry: registryWithSCM,
|
||||
sessionManager: mockSessionManager,
|
||||
});
|
||||
|
||||
await lm.check("app-1");
|
||||
|
||||
expect(mockSCM.detectPR).toHaveBeenCalledOnce();
|
||||
const meta = readMetadataRaw(sessionsDir, "app-1");
|
||||
expect(meta?.["pr"]).toBe(makePR().url);
|
||||
expect(lm.getStates().get("app-1")).toBe("stuck");
|
||||
});
|
||||
|
||||
it("preserves stuck state when getActivityState throws", async () => {
|
||||
vi.mocked(mockAgent.getActivityState).mockRejectedValue(new Error("probe failed"));
|
||||
|
||||
|
|
|
|||
|
|
@ -198,6 +198,17 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
let polling = false; // re-entrancy guard
|
||||
let allCompleteEmitted = false; // guard against repeated all_complete
|
||||
|
||||
/** Check if idle time exceeds the agent-stuck threshold. */
|
||||
function isIdleBeyondThreshold(session: Session, idleTimestamp: Date): boolean {
|
||||
const stuckReaction = getReactionConfigForSession(session, "agent-stuck");
|
||||
const thresholdStr = stuckReaction?.threshold;
|
||||
if (typeof thresholdStr !== "string") return false;
|
||||
const stuckThresholdMs = parseDuration(thresholdStr);
|
||||
if (stuckThresholdMs <= 0) return false;
|
||||
const idleMs = Date.now() - idleTimestamp.getTime();
|
||||
return idleMs > stuckThresholdMs;
|
||||
}
|
||||
|
||||
/** Determine current status for a session by polling plugins. */
|
||||
async function determineStatus(session: Session): Promise<SessionStatus> {
|
||||
const project = config.projects[session.projectId];
|
||||
|
|
@ -212,6 +223,9 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
const agent = registry.get<Agent>("agent", agentName);
|
||||
const scm = project.scm ? registry.get<SCM>("scm", project.scm.plugin) : null;
|
||||
|
||||
// Track activity state across steps so stuck detection can run after PR checks
|
||||
let detectedIdleTimestamp: Date | null = null;
|
||||
|
||||
// 1. Check if runtime is alive
|
||||
if (session.runtimeHandle) {
|
||||
const runtime = registry.get<Runtime>("runtime", project.runtime ?? config.defaults.runtime);
|
||||
|
|
@ -229,7 +243,16 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
if (activityState) {
|
||||
if (activityState.state === "waiting_input") return "needs_input";
|
||||
if (activityState.state === "exited") return "killed";
|
||||
// active/ready/idle/blocked — proceed to PR checks below
|
||||
|
||||
if (
|
||||
(activityState.state === "idle" || activityState.state === "blocked") &&
|
||||
activityState.timestamp
|
||||
) {
|
||||
detectedIdleTimestamp = activityState.timestamp;
|
||||
}
|
||||
|
||||
// active/ready/idle (below threshold)/blocked (below threshold) —
|
||||
// proceed to PR checks below
|
||||
} else {
|
||||
// getActivityState returned null — fall back to terminal output parsing
|
||||
const runtime = registry.get<Runtime>(
|
||||
|
|
@ -299,21 +322,38 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
// Check reviews
|
||||
const reviewDecision = await scm.getReviewDecision(session.pr);
|
||||
if (reviewDecision === "changes_requested") return "changes_requested";
|
||||
if (reviewDecision === "approved") {
|
||||
// Check merge readiness
|
||||
if (reviewDecision === "approved" || reviewDecision === "none") {
|
||||
// Check merge readiness — treat "none" (no reviewers required)
|
||||
// the same as "approved" so CI-green PRs reach "mergeable" status
|
||||
// and fire the merge.ready event / approved-and-green reaction.
|
||||
const mergeReady = await scm.getMergeability(session.pr);
|
||||
if (mergeReady.mergeable) return "mergeable";
|
||||
return "approved";
|
||||
if (reviewDecision === "approved") return "approved";
|
||||
}
|
||||
if (reviewDecision === "pending") return "review_pending";
|
||||
|
||||
// 4b. Post-PR stuck detection: agent has a PR open but is idle beyond
|
||||
// threshold. This catches the case where step 2's stuck check was
|
||||
// bypassed (getActivityState returned null) or the idle timestamp
|
||||
// wasn't available during step 2 but the session has been at pr_open
|
||||
// for a long time. Without this, sessions get stuck at "pr_open" forever.
|
||||
if (detectedIdleTimestamp && isIdleBeyondThreshold(session, detectedIdleTimestamp)) {
|
||||
return "stuck";
|
||||
}
|
||||
|
||||
return "pr_open";
|
||||
} catch {
|
||||
// SCM check failed — keep current status
|
||||
}
|
||||
}
|
||||
|
||||
// 5. Default: if agent is active, it's working
|
||||
// 5. Post-all stuck detection: if we detected idle in step 2 but had no PR,
|
||||
// still check stuck threshold. This handles agents that finish without creating a PR.
|
||||
if (detectedIdleTimestamp && isIdleBeyondThreshold(session, detectedIdleTimestamp)) {
|
||||
return "stuck";
|
||||
}
|
||||
|
||||
// 6. Default: if agent is active, it's working
|
||||
if (
|
||||
session.status === "spawning" ||
|
||||
session.status === SESSION_STATUS.STUCK ||
|
||||
|
|
@ -729,18 +769,18 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
}
|
||||
}
|
||||
|
||||
// For significant transitions not already notified by a reaction, notify humans
|
||||
// For transitions not already notified by a reaction, notify humans.
|
||||
// All priorities (including "info") are routed through notificationRouting
|
||||
// so the config controls which notifiers receive each priority level.
|
||||
if (!reactionHandledNotify) {
|
||||
const priority = inferPriority(eventType);
|
||||
if (priority !== "info") {
|
||||
const event = createEvent(eventType, {
|
||||
sessionId: session.id,
|
||||
projectId: session.projectId,
|
||||
message: `${session.id}: ${oldStatus} → ${newStatus}`,
|
||||
data: { oldStatus, newStatus },
|
||||
});
|
||||
await notifyHuman(event, priority);
|
||||
}
|
||||
const event = createEvent(eventType, {
|
||||
sessionId: session.id,
|
||||
projectId: session.projectId,
|
||||
message: `${session.id}: ${oldStatus} → ${newStatus}`,
|
||||
data: { oldStatus, newStatus },
|
||||
});
|
||||
await notifyHuman(event, priority);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -520,6 +520,105 @@ describe("detectActivity — terminal output classification", () => {
|
|||
describe("getActivityState", () => {
|
||||
const agent = create();
|
||||
|
||||
function mockOpencodeSessionRows(rows: Array<Record<string, unknown>>) {
|
||||
mockExecFileAsync.mockImplementation((cmd: string) => {
|
||||
if (cmd === "tmux") return Promise.resolve({ stdout: "/dev/ttys003\n", stderr: "" });
|
||||
if (cmd === "ps") {
|
||||
return Promise.resolve({
|
||||
stdout: " PID TT ARGS\n 789 ttys003 opencode\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
if (cmd === "opencode") {
|
||||
return Promise.resolve({
|
||||
stdout: JSON.stringify(rows),
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.reject(new Error("unexpected"));
|
||||
});
|
||||
}
|
||||
|
||||
function mockOpencodeSessionList(updated: string | number) {
|
||||
mockOpencodeSessionRows([{ id: "ses_abc123", updated }]);
|
||||
}
|
||||
|
||||
it("returns idle when last activity is older than ready threshold", async () => {
|
||||
mockOpencodeSessionList(new Date(Date.now() - 120_000).toISOString());
|
||||
|
||||
const state = await agent.getActivityState(
|
||||
makeSession({
|
||||
runtimeHandle: makeTmuxHandle(),
|
||||
metadata: { opencodeSessionId: "ses_abc123" },
|
||||
}),
|
||||
60_000,
|
||||
);
|
||||
|
||||
expect(state?.state).toBe("idle");
|
||||
});
|
||||
|
||||
it("returns ready when last activity is between active window and ready threshold", async () => {
|
||||
mockOpencodeSessionList(new Date(Date.now() - 45_000).toISOString());
|
||||
|
||||
const state = await agent.getActivityState(
|
||||
makeSession({
|
||||
runtimeHandle: makeTmuxHandle(),
|
||||
metadata: { opencodeSessionId: "ses_abc123" },
|
||||
}),
|
||||
60_000,
|
||||
);
|
||||
|
||||
expect(state?.state).toBe("ready");
|
||||
});
|
||||
|
||||
it("returns active when last activity is recent", async () => {
|
||||
mockOpencodeSessionList(new Date(Date.now() - 10_000).toISOString());
|
||||
|
||||
const state = await agent.getActivityState(
|
||||
makeSession({
|
||||
runtimeHandle: makeTmuxHandle(),
|
||||
metadata: { opencodeSessionId: "ses_abc123" },
|
||||
}),
|
||||
60_000,
|
||||
);
|
||||
|
||||
expect(state?.state).toBe("active");
|
||||
});
|
||||
|
||||
it("returns null when matching session has invalid updated timestamp", async () => {
|
||||
mockOpencodeSessionRows([{ id: "ses_abc123", updated: "not-a-date" }]);
|
||||
|
||||
const state = await agent.getActivityState(
|
||||
makeSession({
|
||||
runtimeHandle: makeTmuxHandle(),
|
||||
metadata: { opencodeSessionId: "ses_abc123" },
|
||||
}),
|
||||
60_000,
|
||||
);
|
||||
|
||||
expect(state).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to AO session title when opencodeSessionId metadata is missing", async () => {
|
||||
mockOpencodeSessionRows([
|
||||
{
|
||||
id: "ses_different",
|
||||
title: "AO:test-1",
|
||||
updated: new Date(Date.now() - 5_000).toISOString(),
|
||||
},
|
||||
]);
|
||||
|
||||
const state = await agent.getActivityState(
|
||||
makeSession({
|
||||
runtimeHandle: makeTmuxHandle(),
|
||||
metadata: {},
|
||||
}),
|
||||
60_000,
|
||||
);
|
||||
|
||||
expect(state?.state).toBe("active");
|
||||
});
|
||||
|
||||
it("returns null when opencode session list output is malformed JSON", async () => {
|
||||
mockExecFileAsync.mockImplementation((cmd: string) => {
|
||||
if (cmd === "tmux") return Promise.resolve({ stdout: "/dev/ttys003\n", stderr: "" });
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import {
|
||||
DEFAULT_READY_THRESHOLD_MS,
|
||||
shellEscape,
|
||||
asValidOpenCodeSessionId,
|
||||
type Agent,
|
||||
|
|
@ -19,7 +20,31 @@ const execFileAsync = promisify(execFile);
|
|||
interface OpenCodeSessionListEntry {
|
||||
id: string;
|
||||
title?: string;
|
||||
updated?: string;
|
||||
updated?: string | number;
|
||||
}
|
||||
|
||||
function parseUpdatedTimestamp(updated: string | number | undefined): Date | null {
|
||||
if (typeof updated === "number") {
|
||||
if (!Number.isFinite(updated)) return null;
|
||||
const date = new Date(updated);
|
||||
return Number.isNaN(date.getTime()) ? null : date;
|
||||
}
|
||||
|
||||
if (typeof updated !== "string") return null;
|
||||
|
||||
const trimmed = updated.trim();
|
||||
if (trimmed.length === 0) return null;
|
||||
|
||||
if (/^\d+$/.test(trimmed)) {
|
||||
const epochMs = Number(trimmed);
|
||||
if (!Number.isFinite(epochMs)) return null;
|
||||
const date = new Date(epochMs);
|
||||
return Number.isNaN(date.getTime()) ? null : date;
|
||||
}
|
||||
|
||||
const parsedMs = Date.parse(trimmed);
|
||||
if (!Number.isFinite(parsedMs)) return null;
|
||||
return new Date(parsedMs);
|
||||
}
|
||||
|
||||
function parseSessionList(raw: string): OpenCodeSessionListEntry[] {
|
||||
|
|
@ -218,38 +243,50 @@ function createOpenCodeAgent(): Agent {
|
|||
|
||||
async getActivityState(
|
||||
session: Session,
|
||||
_readyThresholdMs?: number,
|
||||
readyThresholdMs?: number,
|
||||
): Promise<ActivityDetection | null> {
|
||||
const threshold = readyThresholdMs ?? DEFAULT_READY_THRESHOLD_MS;
|
||||
const activeWindowMs = Math.min(30_000, threshold);
|
||||
|
||||
// Check if process is running first
|
||||
const exitedAt = new Date();
|
||||
if (!session.runtimeHandle) return { state: "exited", timestamp: exitedAt };
|
||||
const running = await this.isProcessRunning(session.runtimeHandle);
|
||||
if (!running) return { state: "exited", timestamp: exitedAt };
|
||||
|
||||
if (session.metadata?.opencodeSessionId) {
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
"opencode",
|
||||
["session", "list", "--format", "json"],
|
||||
{ timeout: 30_000 },
|
||||
);
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
"opencode",
|
||||
["session", "list", "--format", "json"],
|
||||
{
|
||||
timeout: 30_000,
|
||||
},
|
||||
);
|
||||
|
||||
const sessions = parseSessionList(stdout);
|
||||
const targetSession = sessions.find((s) => s.id === session.metadata.opencodeSessionId);
|
||||
const sessions = parseSessionList(stdout);
|
||||
const targetSession =
|
||||
(session.metadata?.opencodeSessionId
|
||||
? sessions.find((s) => s.id === session.metadata.opencodeSessionId)
|
||||
: undefined) ?? sessions.find((s) => s.title === `AO:${session.id}`);
|
||||
|
||||
if (targetSession) {
|
||||
const lastActivity = targetSession.updated
|
||||
? new Date(targetSession.updated)
|
||||
: undefined;
|
||||
return {
|
||||
state: "active",
|
||||
...(lastActivity &&
|
||||
!Number.isNaN(lastActivity.getTime()) && { timestamp: lastActivity }),
|
||||
};
|
||||
if (targetSession) {
|
||||
const lastActivity = parseUpdatedTimestamp(targetSession.updated);
|
||||
|
||||
if (lastActivity) {
|
||||
const ageMs = Math.max(0, Date.now() - lastActivity.getTime());
|
||||
if (ageMs <= activeWindowMs) {
|
||||
return { state: "active", timestamp: lastActivity };
|
||||
}
|
||||
if (ageMs <= threshold) {
|
||||
return { state: "ready", timestamp: lastActivity };
|
||||
}
|
||||
return { state: "idle", timestamp: lastActivity };
|
||||
}
|
||||
} catch {
|
||||
|
||||
return null;
|
||||
}
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
||||
return null;
|
||||
|
|
|
|||
Loading…
Reference in New Issue