diff --git a/.changeset/claude-activity-edge-cases.md b/.changeset/claude-activity-edge-cases.md new file mode 100644 index 000000000..b44c5686f --- /dev/null +++ b/.changeset/claude-activity-edge-cases.md @@ -0,0 +1,17 @@ +--- +"@aoagents/ao-plugin-agent-claude-code": patch +--- + +Harden Claude Code activity detection against five real-world edge cases identified during PR #1927's analysis: + +1. **Bookkeeping types → false-active.** `file-history-snapshot`, `attachment`, `pr-link`, `queue-operation`, `permission-mode`, `last-prompt`, `ai-title`, `agent-color`, `agent-name`, `custom-title` were falling through to the `default` switch branch and showing as `active` for 30s after Claude finished a turn. They now correctly map to `ready`/`idle` by age. Likely root cause of "Claude looks busy when it's done" reports. + +2. **Multi-session disambiguation.** `findLatestSessionFile` picked newest-mtime, which is the wrong session's JSONL when two Claude sessions are running in the same workspace. Now prefers the UUID-named file (`/.jsonl`) when `session.metadata.claudeSessionUuid` is set, falling back to newest-mtime otherwise. + +3. **Symlinked workspaces.** `toClaudeProjectPath` was a pure string transform — symlink paths produced different slugs than what Claude itself wrote. Added `resolveWorkspaceForClaude(path)` that runs `realpathSync` (with fallback) and used it in all three slug-computing sites (`getClaudeActivityState`, `getSessionInfo`, `getRestoreCommand`). + +4. **Process regex too narrow.** `(?:^|\/)claude(?:\s|$)` was missing several legitimate install variants — `.claude`, `claude-code`, `claude.exe`, `claude.js`, and npm shims like `node /opt/.../@anthropic-ai/claude-code/cli.js`. Broadened to `(?:^|\/)(?:\.)?claude(?:[-.][\w-]+)*(?:[\s/]|$)`; still rejects look-alikes (`claudia`, `claudine`). + +5. **Silent permission-denied.** `findLatestSessionFile` was swallowing every `readdir` error silently — a missing `~/.claude/projects//` (ENOENT) is normal, but a permission-denied (EACCES/EPERM) or fd-exhausted (EMFILE) misconfig left the session looking permanently `idle` on the dashboard with zero telemetry. Now logs a single `console.warn` for non-ENOENT errors. + +193/193 plugin tests pass. No public-API change. New helper `resolveWorkspaceForClaude` is re-exported from `index.ts` for downstream consumers. diff --git a/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts b/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts index bc2735913..332018d41 100644 --- a/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts +++ b/packages/plugins/agent-claude-code/src/__tests__/activity-detection.test.ts @@ -1,8 +1,17 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { existsSync, mkdtempSync, mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs"; +import { + existsSync, + mkdtempSync, + mkdirSync, + realpathSync, + writeFileSync, + rmSync, + utimesSync, +} from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { toClaudeProjectPath, create } from "../index.js"; +import { resetWarnedReaddirPaths } from "../activity-detection.js"; import { createActivitySignal, readLastActivityEntry, @@ -115,7 +124,12 @@ describe("Claude Code Activity Detection", () => { const agent = create(); beforeEach(() => { - fakeHome = mkdtempSync(join(tmpdir(), "ao-activity-test-")); + // realpathSync because /var/folders/... is a symlink to /private/var/folders/... + // on macOS. getClaudeActivityState resolves symlinks before slugifying (so the + // slug matches what Claude wrote), so the test setup must do the same — otherwise + // the test JSONL lives under one slug and the code looks under another. + // homedir() is mocked to fakeHome, so its realpath flows through naturally. + fakeHome = realpathSync(mkdtempSync(join(tmpdir(), "ao-activity-test-"))); workspacePath = join(fakeHome, "workspace"); mkdirSync(workspacePath, { recursive: true }); @@ -126,6 +140,9 @@ describe("Claude Code Activity Detection", () => { // Mock isProcessRunning to always return true (we test exited separately) vi.spyOn(agent, "isProcessRunning").mockResolvedValue(true); + + // Reset module-level warn dedupe so each test starts fresh. + resetWarnedReaddirPaths(); }); afterEach(() => { @@ -166,6 +183,106 @@ describe("Claude Code Activity Detection", () => { expect(await agent.getActivityState(makeSession({ workspacePath: null }))).toBeNull(); }); + it("logs a warning when ~/.claude/projects/ is unreadable (EACCES)", async () => { + // Make the existing project dir unreadable by stripping owner-read. + // ENOENT (dir missing) is normal and stays silent; EACCES/EPERM must + // surface so users can debug a silent-idle session. + const { chmodSync } = await import("node:fs"); + chmodSync(projectDir, 0o000); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const result = await agent.getActivityState(makeSession()); + expect(result).toBeNull(); + expect(warn).toHaveBeenCalledOnce(); + // Non-capturing group: alternation MUST stay inside `(?:...)` or it + // would parse as `(failed to read.*EACCES) | (EPERM)` and any string + // containing the literal `EPERM` would pass the assertion. + expect(warn.mock.calls[0]?.[0]).toMatch(/failed to read.*(?:EACCES|EPERM)/); + } finally { + chmodSync(projectDir, 0o755); + warn.mockRestore(); + } + }); + + it("only warns ONCE per path across multiple polls (no log flood)", async () => { + // Real bug this guards against: getActivityState runs on a polling + // interval; without the dedupe set, a single EACCES path would log + // 60+ lines/minute indefinitely. + const { chmodSync } = await import("node:fs"); + chmodSync(projectDir, 0o000); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await agent.getActivityState(makeSession()); + await agent.getActivityState(makeSession()); + await agent.getActivityState(makeSession()); + expect(warn).toHaveBeenCalledOnce(); + } finally { + chmodSync(projectDir, 0o755); + warn.mockRestore(); + } + }); + + it("does NOT log when project dir simply doesn't exist (ENOENT is normal)", async () => { + const badPath = join(fakeHome, "this-workspace-never-existed"); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await agent.getActivityState(makeSession({ workspacePath: badPath })); + expect(warn).not.toHaveBeenCalled(); + } finally { + warn.mockRestore(); + } + }); + + it("prefers UUID-named JSONL when session.metadata.claudeSessionUuid is set", async () => { + // Multi-session-in-one-worktree case: two .jsonl files exist, the + // newest-mtime one is from a DIFFERENT session, and we don't want + // to misread that. Use the metadata UUID to pick the right one. + const myUuid = "aaa-111"; + // Newest-mtime file is the OTHER session's JSONL (would be wrong). + writeJsonl([{ type: "progress", status: "doing other work" }], 0, "bbb-222.jsonl"); + // My session's JSONL is older, but it's the one we want. + writeJsonl( + [{ type: "assistant", message: { content: "my session" } }], + 10_000, + `${myUuid}.jsonl`, + ); + + const session = makeSession({ metadata: { claudeSessionUuid: myUuid } }); + const result = await agent.getActivityState(session); + // 10s old + `assistant` → ready (NOT `active` from the other session's progress) + expect(result?.state).toBe("ready"); + }); + + it("falls back to newest-mtime when UUID-named file doesn't exist yet", async () => { + // Session just spawned, getSessionInfo hasn't captured the UUID yet, + // OR the UUID was captured but the file was rotated/removed. + // Should still find activity via newest-mtime fallback. + writeJsonl([{ type: "user", message: { content: "hi" } }], 0, "actual-session.jsonl"); + + const session = makeSession({ metadata: { claudeSessionUuid: "uuid-that-doesnt-exist" } }); + const result = await agent.getActivityState(session); + expect(result?.state).toBe("active"); + }); + + it("resolves symlinked workspace paths so slugs match what Claude wrote", async () => { + // Claude resolves the symlink target before slugifying. If AO records + // the symlink path and slugifies without realpath, the two slugs + // diverge and findLatestSessionFile returns null forever. This test + // confirms the realpath fix: write JSONL under the SLUG OF THE TARGET, + // call getActivityState with the SYMLINK PATH, expect to find it. + const { symlinkSync } = await import("node:fs"); + const target = workspacePath; // the real workspace dir + const link = join(fakeHome, "symlinked-workspace"); + symlinkSync(target, link); + + // JSONL is already in projectDir (which slugifies the real path). + writeJsonl([{ type: "assistant", message: { content: "Done!" } }]); + + // Calling with the SYMLINK should still find the file because of realpath. + const result = await agent.getActivityState(makeSession({ workspacePath: link })); + expect(result?.state).toBe("ready"); + }); + it("returns null when project directory does not exist and AO activity is unavailable", async () => { const badPath = join(fakeHome, "nonexistent-workspace"); expect(await agent.getActivityState(makeSession({ workspacePath: badPath }))).toBeNull(); @@ -276,19 +393,75 @@ describe("Claude Code Activity Detection", () => { expect((await agent.getActivityState(makeSession()))?.state).toBe("ready"); }); - it("returns 'active' for recent 'file-history-snapshot' (bookkeeping)", async () => { + // Bookkeeping types Claude writes AFTER finishing a turn — these are + // turn-end markers, not "Claude is working" signals. They must map to + // ready/idle by age, NOT active. Previously they fell through to the + // default branch and looked "active" for 30s; fixed in this PR. + it("returns 'ready' for recent 'file-history-snapshot' (bookkeeping)", async () => { writeJsonl([{ type: "file-history-snapshot" }]); - expect((await agent.getActivityState(makeSession()))?.state).toBe("active"); + expect((await agent.getActivityState(makeSession()))?.state).toBe("ready"); }); - it("returns 'active' for recent 'queue-operation' (bookkeeping)", async () => { + it("returns 'ready' for recent 'queue-operation' (bookkeeping)", async () => { writeJsonl([{ type: "queue-operation" }]); - expect((await agent.getActivityState(makeSession()))?.state).toBe("active"); + expect((await agent.getActivityState(makeSession()))?.state).toBe("ready"); }); - it("returns 'active' for recent 'pr-link' (bookkeeping)", async () => { - writeJsonl([{ type: "pr-link" }]); - expect((await agent.getActivityState(makeSession()))?.state).toBe("active"); + it("returns 'idle' (not 'ready') for recent 'pr-link' — re-snapshot noise", async () => { + // Empirical evidence (ao-160): the SAME PR (#1911) was written as + // pr-link 33 times in the last 200 lines, with new timestamps each + // time. It's a periodic state snapshot, not a one-shot event. + // Treat as noise so dormant sessions don't show as "ready" forever. + writeJsonl([ + { + type: "pr-link", + prNumber: 1911, + prUrl: "https://github.com/owner/repo/pull/1911", + }, + ]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("idle"); + }); + + it("returns 'ready' for recent 'attachment' (bookkeeping)", async () => { + writeJsonl([{ type: "attachment", attachment: { type: "skill_listing" } }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("ready"); + }); + + // Pure UI-noise types (permission-mode, ai-title, agent-*, custom-title) + // are written by Claude at random times (session attach, mode change, + // title gen) and don't reflect actual activity. Real-world repro: + // ao-144 had 73 trailing permission-mode + 73 trailing ai-title entries + // over 6 dormant days, causing the dashboard to oscillate between + // ready and idle. They now fall through to the AO JSONL pipeline; if + // that has nothing, the stale-native fallback returns idle from + // session.createdAt. + it("returns 'idle' (not 'ready') for recent permission-mode noise — dormant session", async () => { + writeJsonl([{ type: "permission-mode", permissionMode: "bypassPermissions" }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("idle"); + }); + + it("returns 'idle' (not 'ready') for recent ai-title noise — dormant session", async () => { + writeJsonl([{ type: "ai-title", title: "Fix login bug" }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("idle"); + }); + + it("returns 'idle' for agent-color / agent-name / custom-title noise", async () => { + writeJsonl([{ type: "agent-color", color: "#fff" }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("idle"); + writeJsonl([{ type: "agent-name", name: "ao-161" }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("idle"); + writeJsonl([{ type: "custom-title", title: "x" }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("idle"); + }); + + it("noise last entry yields to AO JSONL when AO has actionable state", async () => { + // Repro of the scenario where Claude IS active but the latest native + // JSONL line happens to be noise. Native JSONL gets skipped, AO + // activity JSONL has waiting_input from a recent terminal scrape, + // cascade returns waiting_input. + writeJsonl([{ type: "permission-mode" }]); + writeActivityLog("waiting_input"); + expect((await agent.getActivityState(makeSession()))?.state).toBe("waiting_input"); }); }); @@ -297,19 +470,19 @@ describe("Claude Code Activity Detection", () => { // ----------------------------------------------------------------------- describe("agent interface spec types", () => { - it("returns 'active' for recent 'tool_use' entry", async () => { - writeJsonl([{ type: "tool_use" }]); - expect((await agent.getActivityState(makeSession()))?.state).toBe("active"); - }); - it("returns 'ready' for recent 'summary' entry", async () => { writeJsonl([{ type: "summary", summary: "Implemented login feature" }]); expect((await agent.getActivityState(makeSession()))?.state).toBe("ready"); }); - it("returns 'ready' for recent 'result' entry", async () => { - writeJsonl([{ type: "result" }]); - expect((await agent.getActivityState(makeSession()))?.state).toBe("ready"); + it("unknown types fall through to default branch — fresh → active", async () => { + // Claude doesn't emit `tool_use` or `result` as top-level types (verified + // on disk for #1927). Their explicit switch cases were removed and they + // now fall to `default`, which maps fresh unknown types to active. This + // test locks the default-branch semantics so future Claude type + // additions are handled predictably until someone adds an explicit case. + writeJsonl([{ type: "some-future-claude-type" }]); + expect((await agent.getActivityState(makeSession()))?.state).toBe("active"); }); }); diff --git a/packages/plugins/agent-claude-code/src/activity-detection.ts b/packages/plugins/agent-claude-code/src/activity-detection.ts index 3022c4011..940a4591f 100644 --- a/packages/plugins/agent-claude-code/src/activity-detection.ts +++ b/packages/plugins/agent-claude-code/src/activity-detection.ts @@ -14,7 +14,7 @@ import { type Session, } from "@aoagents/ao-core"; import { execFile } from "node:child_process"; -import { readdir, stat } from "node:fs/promises"; +import { readdir, realpath, stat } from "node:fs/promises"; import { homedir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; @@ -45,16 +45,86 @@ export function toClaudeProjectPath(workspacePath: string): string { return normalized.replace(/[^a-zA-Z0-9-]/g, "-"); } +/** + * Resolve a workspace path through any symlinks BEFORE slugifying so AO's + * computed Claude project dir matches what Claude itself writes. + * + * Without this, if AO records `workspacePath` as a symlink (e.g. + * `/Users/me/symlinks/repo`) and Claude resolves it to the target + * (`/Users/me/code/repo`) before computing its on-disk slug, the two slugs + * diverge — AO looks in an empty `~/.claude/projects//` dir + * forever and the session looks permanently `idle`. Falls back to the + * literal path on error (dangling symlink, race, etc.). + */ +export async function resolveWorkspaceForClaude(workspacePath: string): Promise { + try { + return await realpath(workspacePath); + } catch { + return workspacePath; + } +} + // ============================================================================= // Session file discovery // ============================================================================= -/** Find the most recently modified .jsonl session file in a directory */ -export async function findLatestSessionFile(projectDir: string): Promise { +/** Module-level dedupe so EACCES/EPERM on a project dir warns ONCE per path + * for the process lifetime, not on every poll cycle. getClaudeActivityState + * is called every few seconds per session — without this, a single denied + * path would flood logs at 60+ lines/minute indefinitely. Bounded by the + * number of unique workspace slugs, which is small. */ +const warnedReaddirPaths = new Set(); + +/** Reset the warned-paths dedupe set. Exported for testing only. */ +export function resetWarnedReaddirPaths(): void { + warnedReaddirPaths.clear(); +} + +/** Find Claude's JSONL session file for a project directory. + * + * When `preferredUuid` is provided (e.g. from `session.metadata.claudeSessionUuid` + * captured by getSessionInfo), prefer `/.jsonl` + * if it exists. This disambiguates the common case of multiple Claude + * sessions running in the same workspace, where newest-mtime would pick + * the WRONG session's JSONL whenever its sibling has just written. + * + * Falls back to newest-mtime when no UUID is given or the named file + * doesn't exist yet (e.g. fresh session that hasn't been introspected). + * + * ENOENT on the project dir is normal and silent. Other errors + * (EACCES, EPERM, EMFILE, ...) are logged via console.warn — once per + * path for the process lifetime — so a permission-denied or fd-exhausted + * misconfig doesn't silently mask the session as `idle` forever and + * doesn't flood logs on every poll. */ +export async function findLatestSessionFile( + projectDir: string, + preferredUuid?: string, +): Promise { + // Prefer the UUID-named file when we know it — disambiguates multi-session. + if (preferredUuid) { + const preferred = join(projectDir, `${preferredUuid}.jsonl`); + try { + await stat(preferred); + return preferred; + } catch { + // Fall through to newest-mtime — the UUID-named file may not exist + // yet (session just spawned, hasn't been introspected). + } + } + let entries: string[]; try { entries = await readdir(projectDir); - } catch { + } catch (err: unknown) { + if (err instanceof Error && "code" in err && err.code !== "ENOENT") { + if (!warnedReaddirPaths.has(projectDir)) { + warnedReaddirPaths.add(projectDir); + const code = (err as NodeJS.ErrnoException).code; + console.warn( + `[claude-code] failed to read ${projectDir} (${code}): ${err.message}. Session activity will fall back to AO JSONL only. (This warning is shown once per path for the process lifetime.)`, + ); + } + } return null; } @@ -157,9 +227,15 @@ export async function findClaudeProcess( if (psOut === PROCESS_PROBE_INDETERMINATE) return PROCESS_PROBE_INDETERMINATE; const ttySet = new Set(ttys.map((t) => t.replace(/^\/dev\//, ""))); - // Match "claude" as a word boundary — prevents false positives on - // names like "claude-code" or paths that merely contain the substring. - const processRe = /(?:^|\/)claude(?:\s|$)/; + // Match "claude" plus common variants: + // - bare `claude` / `/usr/local/bin/claude` + // - dot-prefix shim `.claude` + // - file extensions like `claude.exe`, `claude.js`, `claude.cjs` + // - hyphenated names like `claude-code` + // - node-shim cases like `node /path/@anthropic-ai/claude-code/cli.js` + // (matches the path component containing "claude") + // Still anchored at `/` or start-of-line so `claudia` etc. don't match. + const processRe = /(?:^|\/)(?:\.)?claude(?:[-.][\w-]+)*(?:[\s/]|$)/; for (const line of psOut.split("\n")) { const cols = line.trimStart().split(/\s+/); if (cols.length < 3 || !ttySet.has(cols[1] ?? "")) continue; @@ -210,26 +286,120 @@ export function classifyTerminalOutput(terminalOutput: string): ActivityState { const lines = terminalOutput.trim().split("\n"); const lastLine = lines[lines.length - 1]?.trim() ?? ""; - // Check the last line FIRST — if the prompt is visible, the agent is idle - // regardless of historical output (e.g. "Reading file..." from earlier). - // The ❯ is Claude Code's prompt character. + // Empty prompt on the last line is unambiguously idle. if (/^[❯>$#]\s*$/.test(lastLine)) return "idle"; - // Check the bottom of the buffer for permission prompts BEFORE checking - // full-buffer active indicators. Historical "Thinking"/"Reading" text in - // the buffer must not override a current permission prompt at the bottom. + // Use a wider window (last 12 lines) than the bottom-of-buffer prompt + // check above because Claude's spinner+status line, ⎿ tool-result lines, + // and api-error text often sit 6-8 lines above the input area + footer. + // All multi-line state checks (blocked, active) use this window — full + // `terminalOutput` would let scrolled-off error text falsely return + // "blocked" forever after a successful retry pushes the error out of + // view but not out of scrollback. + const wideTail = lines.slice(-12).join("\n"); + + // Check for blocked. Claude's persistent UI footer contains + // "bypass permissions on (shift+tab to cycle)" on every session — the + // tightened waiting_input regex no longer matches it, and the blocked + // patterns below are specific to api-error retry text. + // + // Patterns observed empirically by capturing tmux output during a real + // api-blocked retry loop (see PR #1932 description): + // ⎿ Unable to connect to API (ConnectionRefused) + // Retrying in 19s · attempt 7/10 + if (/Unable to connect to API/i.test(wideTail)) return "blocked"; + if (/Retrying in \d+s.*attempt \d+\/\d+/i.test(wideTail)) return "blocked"; + + // Check the bottom of the buffer for permission prompts. Historical + // "Thinking"/"Reading" text earlier in the buffer must not override a + // current permission prompt at the bottom. const tail = lines.slice(-5).join("\n"); if (/Do you want to proceed\?/i.test(tail)) return "waiting_input"; if (/\(Y\)es.*\(N\)o/i.test(tail)) return "waiting_input"; - if (/bypass.*permissions/i.test(tail)) return "waiting_input"; + // Match the ACTUAL permission-bypass prompt, NOT the static footer toggle. + if (/bypass\s+all\s+future\s+permissions/i.test(tail)) return "waiting_input"; - return "active"; + // Active: only when an explicit active-work indicator is present. Default + // is IDLE — Claude's tmux pane has a persistent input area + footer that + // looks identical between "just finished" and "working". Treating + // unrecognized output as active caused dormant sessions (ao-160 etc.) to + // get an "active" written to AO activity-JSONL every poll cycle, which the + // age-decayed fallback then surfaced as ready forever. + + // Strongest active signal: gerund (present-participle) status verb + // followed by the trailing ellipsis "…". Claude cycles through many + // status words (Germinating, Fluttering, Pondering, Mulling, Crafting, + // Thinking, Reasoning, ...) and many spinner glyphs (✻ ✽ · ⠁ ⠈ etc.) + // depending on animation frame. The gerund+ellipsis combo is the + // consistent signal that survives glyph rotation. Past-tense lines like + // "✻ Worked for 11s" or "✻ Crunched for 11s" lack the ellipsis and are + // turn-complete summaries — they must NOT match (ao-143 repro). + if (/\b\w+ing…/.test(wideTail)) return "active"; + // NOTE — these patterns look "active-ish" but are NOT, and should never + // go back as active indicators: + // - /\bCrunched\s+for\s+\d+s/ — past-tense turn summary (ao-154 repro: + // "✻ Crunched for 22s" was making sessions perpetually "active") + // - /\bWorked\s+for\s+\d+s/ — past-tense turn summary (ao-143 repro) + // - /^\s*⎿\s+/ — prefixes past tool-results too + // - /esc to interrupt/ — in the persistent UI footer + // - bare /\bWorking\b/ — matches "working on issue #N" in recap + // The gerund+ellipsis above catches present-tense forms across all + // status words regardless of spinner glyph rotation. + + // Word-based fallbacks for synthetic test inputs and rare cases where + // the ellipsis isn't captured. Tight patterns to avoid false-firing on + // benign text. + if (/\bGerminating/i.test(wideTail)) return "active"; + if (/\b(?:Thinking|Working)\s*(?:…|\.\.\.)/i.test(wideTail)) return "active"; + if (/\bReading\s+file/i.test(wideTail)) return "active"; + if (/\bWriting\s+to\b/i.test(wideTail)) return "active"; + if (/\bSearching\s+codebase/i.test(wideTail)) return "active"; + if (/Press\s+up\s+to\s+edit\s+queued/i.test(wideTail)) return "active"; + + return "idle"; } // ============================================================================= // Activity-state cascade // ============================================================================= +/** + * Claude writes these types as UI-state snapshots at random times: on session + * attach, on permission-mode change, on title regeneration, etc. They are + * NOT correlated with whether Claude is actively working — a 6-day-dormant + * session will still accumulate dozens of `permission-mode` and `ai-title` + * entries just from being inspected. + * + * When one of these is the literal last JSONL entry, treat it as a "no + * signal" — fall through to the AO activity-JSONL pipeline (terminal- + * derived signal) rather than letting noise mtime decide the activity. + * + * Concrete bug this prevents: ao-144 had 73 trailing `permission-mode` + + * 73 trailing `ai-title` entries written over 6 dormant days. Without + * this skip, dashboard oscillated between `ready` (recent noise mtime) + * and `idle` (old noise mtime) instead of staying `idle`. + * + * Conservative list — only the types that empirically run away. The other + * bookkeeping types (file-history-snapshot, attachment, pr-link, + * queue-operation, last-prompt) plausibly correlate with real activity + * and stay in the explicit ready/idle case. + */ +const NOISE_JSONL_TYPES: ReadonlySet = new Set([ + "permission-mode", + "ai-title", + "agent-color", + "agent-name", + "custom-title", + // pr-link is also re-snapshot noise — verified on ao-160's JSONL where the + // SAME PR (#1911) was written as a `pr-link` entry three times within + // minutes (count: 33 pr-link vs 21 user messages in the last 200 lines). + // The first emission is real; subsequent re-emissions are state snapshots. + // We can't distinguish first vs Nth from the last line alone, so treat + // all pr-link as noise. Real PR creation is still observable via the + // assistant message and the gh-tracker side. + "pr-link", +]); + /** * Determine current activity state for a Claude Code session. * @@ -261,10 +431,16 @@ export async function getClaudeActivityState( if (!session.workspacePath) return null; - const projectPath = toClaudeProjectPath(session.workspacePath); + const projectPath = toClaudeProjectPath(await resolveWorkspaceForClaude(session.workspacePath)); const projectDir = join(homedir(), ".claude", "projects", projectPath); - const sessionFile = await findLatestSessionFile(projectDir); + // Prefer the UUID-named file when getSessionInfo has captured one — this + // disambiguates multi-session-per-worktree, where newest-mtime would pick + // the wrong session's JSONL whenever its sibling has just written. + const rawUuid = session.metadata?.["claudeSessionUuid"]; + const preferredUuid = + typeof rawUuid === "string" && rawUuid.trim() ? rawUuid.trim() : undefined; + const sessionFile = await findLatestSessionFile(projectDir, preferredUuid); let staleNativeState: ActivityDetection | null = null; if (sessionFile) { const entry = await readLastJsonlEntry(sessionFile); @@ -275,14 +451,24 @@ export async function getClaudeActivityState( // Claude writes this session's first native JSONL entry. if (session.createdAt && entry.modifiedAt < session.createdAt) { staleNativeState = { state: "idle", timestamp: session.createdAt }; + } else if (entry.lastType && NOISE_JSONL_TYPES.has(entry.lastType)) { + // Last entry is UI-state noise (permission-mode / ai-title / etc.) + // that doesn't reflect actual activity. Fall through to the AO + // activity-JSONL pipeline for a terminal-derived answer; if that's + // also empty, the staleNativeState below returns idle. + staleNativeState = { state: "idle", timestamp: session.createdAt }; } else { const ageMs = Date.now() - entry.modifiedAt.getTime(); const timestamp = entry.modifiedAt; const activeWindowMs = Math.min(DEFAULT_ACTIVE_WINDOW_MS, threshold); switch (entry.lastType) { + // In-progress turn markers: very recent → active, older → ready/idle. + // Removed `tool_use` and `result` cases that were in the spec but + // never actually emitted by Claude (verified by disk survey for + // #1927). The `default` branch handles them with the same semantics + // if Claude ever introduces them. case "user": - case "tool_use": case "progress": if (ageMs <= activeWindowMs) return { state: "active", timestamp }; return { state: ageMs > threshold ? "idle" : "ready", timestamp }; @@ -301,7 +487,18 @@ export async function getClaudeActivityState( case "assistant": case "summary": - case "result": + return { state: ageMs > threshold ? "idle" : "ready", timestamp }; + + // Bookkeeping types Claude writes AFTER a real event (file edits, + // attachment context, queue housekeeping, prompt submit). Map to + // ready/idle by age, same as assistant/summary. The pure re-snapshot + // types (permission-mode, ai-title, agent-*, custom-title, pr-link) + // are filtered out earlier by NOISE_JSONL_TYPES — they get written + // continuously without indicating activity. + case "file-history-snapshot": + case "attachment": + case "queue-operation": + case "last-prompt": return { state: ageMs > threshold ? "idle" : "ready", timestamp }; default: diff --git a/packages/plugins/agent-claude-code/src/index.test.ts b/packages/plugins/agent-claude-code/src/index.test.ts index e664509a0..be8955c55 100644 --- a/packages/plugins/agent-claude-code/src/index.test.ts +++ b/packages/plugins/agent-claude-code/src/index.test.ts @@ -375,6 +375,45 @@ describe("isProcessRunning", () => { expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false); }); + // Coverage for the broadened process regex — these are real install shapes + // the previous narrow regex `/(?:^|\/)claude(?:\s|$)/` would have missed, + // causing AO to declare sessions `exited` while Claude was still running. + it.each([ + ["bare binary", "claude"], + ["absolute path", "/opt/homebrew/bin/claude"], + ["dot-prefix shim", "/usr/local/lib/.claude"], + ["windows exe", "claude.exe"], + ["js shim", "claude.js"], + ["hyphenated name", "claude-code"], + ["node-shim npm install", "node /opt/homebrew/lib/node_modules/@anthropic-ai/claude-code/cli.js"], + ])("returns true for %s (%s)", async (_label, args) => { + mockExecFileAsync.mockImplementation((cmd: string) => { + if (cmd === "tmux") return Promise.resolve({ stdout: "/dev/ttys001\n", stderr: "" }); + if (cmd === "ps") + return Promise.resolve({ + stdout: ` PID TT ARGS\n 123 ttys001 ${args}\n`, + stderr: "", + }); + return Promise.reject(new Error("unexpected")); + }); + resetPsCache(); + expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(true); + }); + + it("still rejects look-alike names (claudia, claudine)", async () => { + mockExecFileAsync.mockImplementation((cmd: string) => { + if (cmd === "tmux") return Promise.resolve({ stdout: "/dev/ttys001\n", stderr: "" }); + if (cmd === "ps") + return Promise.resolve({ + stdout: " PID TT ARGS\n 123 ttys001 claudia\n 124 ttys001 /bin/claudine\n", + stderr: "", + }); + return Promise.reject(new Error("unexpected")); + }); + resetPsCache(); + expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false); + }); + it("returns false when tmux list-panes returns empty", async () => { mockExecFileAsync.mockResolvedValue({ stdout: "", stderr: "" }); expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false); @@ -441,22 +480,6 @@ describe("isProcessRunning", () => { expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(true); }); - it("does not match similar process names like claude-code", async () => { - mockExecFileAsync.mockImplementation((cmd: string, args: string[]) => { - if (cmd === "tmux" && args[0] === "list-panes") { - return Promise.resolve({ stdout: "/dev/ttys001\n", stderr: "" }); - } - if (cmd === "ps") { - return Promise.resolve({ - stdout: " PID TT ARGS\n 100 ttys001 /usr/bin/claude-code\n", - stderr: "", - }); - } - return Promise.reject(new Error("unexpected")); - }); - expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false); - }); - it("returns false for tmux handle on Windows without spawning ps", async () => { mockIsWindows.mockReturnValue(true); // ps should never be called — getCachedProcessList guards against Windows @@ -515,6 +538,23 @@ describe("detectActivity", () => { ); }); + it("does NOT match Claude's persistent UI footer 'bypass permissions on (shift+tab to cycle)'", () => { + // Regression test: the old `/bypass.*permissions/i` regex matched this + // footer toggle (visible on EVERY Claude session) and falsely fired + // waiting_input for every session that fell through to the AO JSONL + // pipeline. ao-143/144/151 all flipped to waiting_input on dormant + // sessions until this was tightened to require "all future". + const footerOnly = [ + "✻ Crunched for 11s", + "", + "──────────────────────────────────────────────────────────", + "❯ ", + "──────────────────────────────────────────────────────────", + " ⏵⏵ bypass permissions on (shift+tab to cycle)", + ].join("\n"); + expect(agent.detectActivity(footerOnly)).not.toBe("waiting_input"); + }); + it("returns active when queued message indicator is visible", () => { expect(agent.detectActivity("Press up to edit queued messages\n")).toBe("active"); }); @@ -543,8 +583,131 @@ describe("detectActivity", () => { ).toBe("waiting_input"); }); - it("returns active for non-empty output with no special patterns", () => { - expect(agent.detectActivity("some random terminal output\n")).toBe("active"); + it("returns idle for non-empty output with no active-work indicators", () => { + // Default-to-idle (changed from default-to-active in this PR). Claude's + // tmux pane has a persistent input area + footer that looks identical + // between "just finished" and "currently working". Treating + // unrecognized output as active caused dormant sessions to get an + // "active" written to AO activity-JSONL every poll cycle, which the + // age-decayed fallback then surfaced as ready forever (ao-160 repro). + expect(agent.detectActivity("some random terminal output\n")).toBe("idle"); + }); + + it("returns idle for dormant session showing only Claude's input area + footer", () => { + // Real captured output from a dormant session (ao-143 style): assistant + // output above, separator, empty prompt line, separator, footer toggle. + // The empty prompt ❯ is NOT the LAST line (footer is) so the existing + // lastLine check misses it, and previously the default-to-active sent + // every dormant session into the AO-JSONL active-loop. + const dormant = [ + "※ recap: working on issue #143; next: wait for review", + "", + "──────────────────────────────────────────────────────────", + "❯ ", + "──────────────────────────────────────────────────────────", + " ⏵⏵ bypass permissions on (shift+tab to cycle) · esc to interrupt", + ].join("\n"); + expect(agent.detectActivity(dormant)).toBe("idle"); + }); + + it("returns active when spinner+ellipsis is in the tail (✻ Fluttering…)", () => { + // Real captured output from ao-161 mid-active-turn. The ✻ spinner + // followed by a verb and trailing ellipsis is the canonical Claude + // active indicator across all turn-status words (Germinating, + // Fluttering, Thinking, Pondering, etc). + const active = [ + "✻ Fluttering… (6m 49s · ↓ 26.9k tokens)", + " ⎿ Tip: Use /feedback to help us improve!", + "", + "──────", + "❯ ", + "──────", + " ⏵⏵ bypass permissions on (shift+tab to cycle) · esc to interrupt", + ].join("\n"); + expect(agent.detectActivity(active)).toBe("active"); + }); + + it("returns idle for past-tense spinner status like '✻ Worked for 11s' (no ellipsis)", () => { + // Real captured output from ao-143 dormant. The ✻ glyph appears in + // past-tense turn summaries too — without the trailing ellipsis, + // Claude is done, not active. + const dormant = [ + "⏺ Posted: https://github.com/owner/repo/pull/1#comment-1", + "", + "✻ Worked for 11s", + "", + "※ recap: working on issue #143; next: wait for review", + "──────", + "❯ ", + "──────", + " ⏵⏵ bypass permissions on (shift+tab to cycle)", + ].join("\n"); + expect(agent.detectActivity(dormant)).toBe("idle"); + }); + + // Blocked detection from terminal regex — empirically captured from real + // Claude output during api.anthropic.com block (see PR #1932). + it("returns blocked for 'Unable to connect to API' error line", () => { + const real = [ + "❯ what is 2+2? answer in one word.", + " ⎿ Unable to connect to API (ConnectionRefused)", + " Retrying in 19s · attempt 7/10", + "", + "✽ Germinating… (56s)", + "", + ].join("\n"); + expect(agent.detectActivity(real)).toBe("blocked"); + }); + + it("returns blocked for FailedToOpenSocket error variant", () => { + expect( + agent.detectActivity(" ⎿ Unable to connect to API (FailedToOpenSocket)\n"), + ).toBe("blocked"); + }); + + it("returns blocked for retry counter alone (Retrying in Ns · attempt N/M)", () => { + // If only the retry line is in the visible window (error scrolled off), + // the retry counter is still a sufficient signal. + expect(agent.detectActivity(" Retrying in 30s · attempt 9/10\n")).toBe("blocked"); + }); + + it("does NOT return blocked when API error has scrolled out of the visible window after a successful retry", () => { + // Regression test: blocked detection must be bounded to the last 12 + // lines (wideTail), NOT the full terminalOutput buffer. Otherwise an + // api_error that scrolled off the visible area after a successful + // retry but stayed in scrollback would falsely return "blocked" + // forever (Greptile review on PR #1932). + const recoveredAndContinued = [ + " ⎿ Unable to connect to API (ConnectionRefused)", + " Retrying in 1s · attempt 1/10", + " ⎿ ✓ Connected, retry succeeded", + "", + "(many lines of work output below pushing the error off the visible area)", + ...Array.from({ length: 15 }, (_, i) => ` line ${i + 1} of subsequent work`), + "", + "✻ Fluttering… (2m 14s)", + " ⎿ Tip: Use /feedback to help us improve!", + "", + "──────", + "❯ ", + "──────", + " ⏵⏵ bypass permissions on (shift+tab to cycle) · esc to interrupt", + ].join("\n"); + expect(agent.detectActivity(recoveredAndContinued)).toBe("active"); + }); + + it("blocked takes precedence over waiting_input when both 'bypass permissions' footer and api-error are present", () => { + // Claude's static UI footer always contains "bypass permissions on …", + // which the existing waiting_input regex matches. A real blocked state + // must win over that incidental match. + const real = [ + " ⎿ Unable to connect to API (ConnectionRefused)", + " Retrying in 1s · attempt 5/10", + "", + "────────────────────────────────────────", + " ⏵⏵ bypass permissions on (shift+tab to cycle) · esc to interrupt", + ].join("\n"); + expect(agent.detectActivity(real)).toBe("blocked"); }); }); diff --git a/packages/plugins/agent-claude-code/src/index.ts b/packages/plugins/agent-claude-code/src/index.ts index 310ca3add..29e5ce5e3 100644 --- a/packages/plugins/agent-claude-code/src/index.ts +++ b/packages/plugins/agent-claude-code/src/index.ts @@ -26,10 +26,11 @@ import { findLatestSessionFile, getClaudeActivityState, isClaudeProcessAlive, + resolveWorkspaceForClaude, toClaudeProjectPath, } from "./activity-detection.js"; -export { resetPsCache, toClaudeProjectPath } from "./activity-detection.js"; +export { resetPsCache, resolveWorkspaceForClaude, toClaudeProjectPath } from "./activity-detection.js"; // ============================================================================= // Metadata Updater Hook Script @@ -767,7 +768,7 @@ function createClaudeCodeAgent(): Agent { if (!session.workspacePath) return null; // Build the Claude project directory path - const projectPath = toClaudeProjectPath(session.workspacePath); + const projectPath = toClaudeProjectPath(await resolveWorkspaceForClaude(session.workspacePath)); const projectDir = join(homedir(), ".claude", "projects", projectPath); // Find the latest session JSONL file @@ -797,7 +798,7 @@ function createClaudeCodeAgent(): Agent { if (!session.workspacePath) return null; // Find Claude's project directory for this workspace - const projectPath = toClaudeProjectPath(session.workspacePath); + const projectPath = toClaudeProjectPath(await resolveWorkspaceForClaude(session.workspacePath)); const projectDir = join(homedir(), ".claude", "projects", projectPath); // Find the latest session JSONL file