From f330a1ea6990fb56e207da015f76a85295a4d995 Mon Sep 17 00:00:00 2001 From: Ashish Huddar <73213873+ashish921998@users.noreply.github.com> Date: Mon, 20 Apr 2026 13:05:34 +0530 Subject: [PATCH] feat(cli): filter terminated sessions from ao session ls / ao status by default (#1340) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(cli): filter terminated sessions from ao session ls / ao status by default Closes #1310. Terminated sessions (killed/terminated/done/merged/errored/cleanup, plus lifecycle-driven terminal states) are now hidden from `ao session ls` and `ao status` by default. A dim footer reports how many were hidden and how to surface them. Pass `--include-terminated` to restore the full list. JSON output wraps into `{ data: [...], meta: { hiddenTerminatedCount } }` on both commands so text and machine-readable views tell the same story. This is a breaking change for script consumers of `--json`; `--include-terminated` is the escape hatch. Orthogonal to `-a, --all` (orchestrator visibility, unchanged). Restore of terminated sessions by id is unaffected — that path goes through `sm.get`, not `sm.list`. Docs (`SETUP.md`, `docs/CLI.md`) updated to match. Tests cover both the legacy status branch and the canonical lifecycle branch of `isTerminalSession`. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(cli): drop unused `lc` param in lifecycle-alive test case ESLint's no-unused-vars rejects unprefixed unused args. The "alive — should remain visible" branch of the new lifecycle-driven filter test in `session.test.ts` took `lc` but never touched it. Switch to `()`. Matches the equivalent case in `status.test.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(core): preserve pr.state=merged when legacy metadata lacks pr= URL Review blocker on PR #1340: a metadata file with `status=merged` but no `pr=` URL was still showing as active in `ao session ls` / `ao status` by default. Root cause: `synthesizePRState()` in lifecycle-state.ts short-circuited to `{ state: "none" }` whenever no PR URL was present, ignoring the fact that the legacy `status` column already encodes terminal truth. Once lifecycle was synthesized as `session.state="idle"` + `pr.state="none"`, `deriveLegacyStatus` returned `"idle"` and `isTerminalSession()` (lifecycle branch) returned false. The new CLI filter then let the session through. Fix: when legacy `status === "merged"` and no URL is available, synthesize `pr.state="merged", reason="merged"` with `number: null, url: null`. The terminal signal survives the flat-metadata → canonical-lifecycle round trip. Also: - Export `sessionFromMetadata` from the core barrel. CLI tests and external consumers need it to round-trip metadata through the canonical lifecycle. - Update CLI `buildSessionsFromDir` helpers to route through `sessionFromMetadata` so mocked `sm.list()` reflects production reconstruction (the old shortcut bypassed synthesis entirely and was the reason the bug slipped past the original test suite). - Add regression tests: one at the core level (`parseCanonicalLifecycle` for merged-without-URL) and one integration-style test per CLI command asserting the reviewer's exact repro produces the expected filtered output. - One pre-existing test expectation updated: when metadata has `status=working` and `pr=`, the reconstructed status is `pr_open`, not `working`. That's what production `sm.list()` has always returned; the test was previously hiding behind the reconstruction shortcut. Changeset bumped to include ao-core (patch). Co-Authored-By: Claude Opus 4.7 (1M context) * test(cli): route review-check helper through sessionFromMetadata Last remaining test-fidelity shortcut flagged by codex on PR #1340. The `buildSessionsFromDir` helper in review-check.test.ts fabricated Session objects by hand, bypassing the canonical lifecycle reconstruction that production `sm.list()` runs. Doesn't affect review-check's actual behavior (which reads `session.metadata["pr"]` directly), but aligns this test with the equivalent helpers in session.test.ts and status.test.ts so future lifecycle changes don't silently skip this surface. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../filter-terminated-sessions-from-ls.md | 12 + SETUP.md | 4 +- docs/CLI.md | 11 +- .../__tests__/commands/review-check.test.ts | 31 ++- .../cli/__tests__/commands/session.test.ts | 260 +++++++++++++++--- .../cli/__tests__/commands/status.test.ts | 208 ++++++++++++-- packages/cli/src/commands/session.ts | 39 ++- packages/cli/src/commands/status.ts | 34 ++- .../src/__tests__/lifecycle-state.test.ts | 16 ++ packages/core/src/index.ts | 1 + packages/core/src/lifecycle-state.ts | 7 + 11 files changed, 532 insertions(+), 91 deletions(-) create mode 100644 .changeset/filter-terminated-sessions-from-ls.md diff --git a/.changeset/filter-terminated-sessions-from-ls.md b/.changeset/filter-terminated-sessions-from-ls.md new file mode 100644 index 000000000..744433762 --- /dev/null +++ b/.changeset/filter-terminated-sessions-from-ls.md @@ -0,0 +1,12 @@ +--- +"@aoagents/ao-cli": minor +"@aoagents/ao-core": patch +--- + +`ao session ls` and `ao status` now hide terminated sessions (`killed`, `terminated`, `done`, `merged`, `errored`, `cleanup`) by default. A dim footer reports how many were hidden and how to surface them. Pass `--include-terminated` to restore the previous unfiltered output. + +Core change: `parseCanonicalLifecycle()` now preserves `pr.state="merged"` when reconstructing legacy metadata with `status=merged` but no `pr=` URL (previously collapsed to `pr.state="none"`, which made `isTerminalSession()` return false for those sessions). Also exports `sessionFromMetadata` so consumers can round-trip flat metadata through the canonical lifecycle. + +**Breaking — JSON output shape:** `ao session ls --json` and `ao status --json` now emit `{ data: [...], meta: { hiddenTerminatedCount: number } }` instead of a bare array. Scripts consuming the JSON must read `.data` for the session list. `--include-terminated` restores full data and reports `hiddenTerminatedCount: 0`. + +The existing `-a, --all` flag still only governs orchestrator visibility on `ao session ls` — it does **not** re-enable terminated sessions. Combine with `--include-terminated` when you want both. diff --git a/SETUP.md b/SETUP.md index 0c4019b12..faeabe61a 100644 --- a/SETUP.md +++ b/SETUP.md @@ -812,9 +812,11 @@ ao session ls ao session kill # Cleanup script (example) -ao session ls --json | jq -r '.[] | select(.status == "merged") | .id' | xargs -I{} ao session kill {} +ao session ls --json --include-terminated | jq -r '.data[] | select(.status == "merged") | .id' | xargs -I{} ao session kill {} ``` +> **Note:** `ao session ls --json` and `ao status --json` emit `{ data: [...], meta: { hiddenTerminatedCount } }`. By default terminated sessions (`killed`, `terminated`, `done`, `merged`, `errored`, `cleanup`) are hidden — pass `--include-terminated` to include them in `data`. + ### Can I run multiple orchestrators? Yes! Each orchestrator instance should have: diff --git a/docs/CLI.md b/docs/CLI.md index b4df14980..643b62721 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -23,12 +23,19 @@ ao spawn [issue] # Spawn an agent (project auto-detected f ao spawn 123 --agent codex # Override agent for this session ao batch-spawn 101 102 103 # Spawn agents for multiple issues at once ao send "Fix the tests" # Send instructions to a running agent -ao session ls # List sessions -ao session ls --json # Machine-readable session inventory +ao session ls # List active sessions (terminated hidden) +ao session ls --include-terminated # Include killed/done/merged/errored/cleanup sessions +ao session ls --json # Machine-readable session inventory (see note below) ao session kill # Kill a session ao session restore # Revive a crashed agent ``` +> **JSON output:** `ao session ls --json` and `ao status --json` emit +> `{ "data": [...], "meta": { "hiddenTerminatedCount": N } }`. Terminated sessions +> (`killed`, `terminated`, `done`, `merged`, `errored`, `cleanup`) are filtered from +> `data` by default; `meta.hiddenTerminatedCount` reports how many were dropped. +> Pass `--include-terminated` to include them and reset the count to `0`. + ## Maintenance commands ```bash diff --git a/packages/cli/__tests__/commands/review-check.test.ts b/packages/cli/__tests__/commands/review-check.test.ts index bbe4bc26a..aafac8756 100644 --- a/packages/cli/__tests__/commands/review-check.test.ts +++ b/packages/cli/__tests__/commands/review-check.test.ts @@ -10,7 +10,12 @@ import { } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; -import { type Session, type SessionManager, getSessionsDir } from "@aoagents/ao-core"; +import { + type Session, + type SessionManager, + getSessionsDir, + sessionFromMetadata, +} from "@aoagents/ao-core"; const { mockTmux, mockExec, mockGh, mockConfigRef, mockSessionManager, sessionsDirRef } = vi.hoisted(() => ({ @@ -76,28 +81,24 @@ function parseMetadata(content: string): Record { return meta; } -/** Build Session objects from metadata files in sessionsDir. */ +/** + * Build Session objects from metadata files in sessionsDir. + * + * Routes through the real `sessionFromMetadata()` so lifecycle reconstruction + * matches what production `sm.list()` returns. Previously this helper built + * Session objects by hand, which silently bypassed synthesis and hid bugs like + * the "status=merged without pr= URL" rehydration miss fixed in PR #1340. + */ function buildSessionsFromDir(dir: string, projectId: string): Session[] { if (!existsSync(dir)) return []; const files = readdirSync(dir).filter((f) => !f.startsWith(".") && f !== "archive"); return files.map((name) => { const content = readFileSync(join(dir, name), "utf-8"); const meta = parseMetadata(content); - return { - id: name, + return sessionFromMetadata(name, meta, { projectId, - status: (meta["status"] as Session["status"]) || "spawning", - activity: null, - branch: meta["branch"] || null, - issueId: meta["issue"] || null, - pr: null, - workspacePath: meta["worktree"] || null, runtimeHandle: { id: name, runtimeName: "tmux", data: {} }, - agentInfo: null, - createdAt: new Date(), - lastActivityAt: new Date(), - metadata: meta, - } satisfies Session; + }); }); } diff --git a/packages/cli/__tests__/commands/session.test.ts b/packages/cli/__tests__/commands/session.test.ts index 2032d45ac..768c02590 100644 --- a/packages/cli/__tests__/commands/session.test.ts +++ b/packages/cli/__tests__/commands/session.test.ts @@ -17,8 +17,11 @@ import { type CleanupResult, type SessionManager, SessionNotFoundError, + createInitialCanonicalLifecycle, + createActivitySignal, getSessionsDir, getProjectBaseDir, + sessionFromMetadata, } from "@aoagents/ao-core"; const { @@ -112,28 +115,25 @@ function parseMetadata(content: string): Record { return meta; } -/** Build Session objects from metadata files in sessionsDir. */ +/** + * Build Session objects from metadata files in sessionsDir. + * + * Routes through the real `sessionFromMetadata()` so lifecycle reconstruction + * (parseCanonicalLifecycle → synthesize*State → deriveLegacyStatus) runs + * exactly as it does in production `sm.list()`. Tests that assert filter + * behavior against on-disk metadata therefore exercise the full path, not a + * shortcut that bypasses lifecycle synthesis. + */ function buildSessionsFromDir(dir: string, projectId: string): Session[] { if (!existsSync(dir)) return []; const files = readdirSync(dir).filter((f) => !f.startsWith(".") && f !== "archive"); return files.map((name) => { const content = readFileSync(join(dir, name), "utf-8"); const meta = parseMetadata(content); - return { - id: name, + return sessionFromMetadata(name, meta, { projectId, - status: (meta["status"] as Session["status"]) || "spawning", - activity: null, - branch: meta["branch"] || null, - issueId: meta["issue"] || null, - pr: null, - workspacePath: meta["worktree"] || null, runtimeHandle: { id: name, runtimeName: "tmux", data: {} }, - agentInfo: null, - createdAt: new Date(), - lastActivityAt: new Date(), - metadata: meta, - } satisfies Session; + }); }); } @@ -349,20 +349,25 @@ describe("session ls", () => { await program.parseAsync(["node", "test", "session", "ls", "--json"]); expect(consoleSpy).toHaveBeenCalledTimes(1); - expect(JSON.parse(String(consoleSpy.mock.calls[0][0]))).toEqual([ - { - id: "app-1", - projectId: "my-app", - projectName: "My App", - role: "worker", - branch: "live-branch", - status: "working", - issueId: "INT-100", - pr: "https://github.com/org/repo/pull/42", - workspacePath: "/tmp/wt", - lastActivityAt: "2024-03-09T16:00:00.000Z", - }, - ]); + expect(JSON.parse(String(consoleSpy.mock.calls[0][0]))).toEqual({ + data: [ + { + id: "app-1", + projectId: "my-app", + projectName: "My App", + role: "worker", + branch: "live-branch", + // "working" on disk + a pr= URL reconstructs to pr_open via the + // canonical lifecycle, which is what production sm.list() returns. + status: "pr_open", + issueId: "INT-100", + pr: "https://github.com/org/repo/pull/42", + workspacePath: "/tmp/wt", + lastActivityAt: "2024-03-09T16:00:00.000Z", + }, + ], + meta: { hiddenTerminatedCount: 0 }, + }); }); it("marks metadata-based orchestrators correctly in JSON output", async () => { @@ -377,29 +382,196 @@ describe("session ls", () => { await program.parseAsync(["node", "test", "session", "ls", "--json"]); expect(consoleSpy).toHaveBeenCalledTimes(1); - expect(JSON.parse(String(consoleSpy.mock.calls[0][0]))).toEqual([ - { - id: "app-control", - projectId: "my-app", - projectName: "My App", - role: "orchestrator", - branch: "control", - status: "working", - issueId: null, - pr: null, - workspacePath: null, - lastActivityAt: null, - }, - ]); + expect(JSON.parse(String(consoleSpy.mock.calls[0][0]))).toEqual({ + data: [ + { + id: "app-control", + projectId: "my-app", + projectName: "My App", + role: "orchestrator", + branch: "control", + status: "working", + issueId: null, + pr: null, + workspacePath: null, + lastActivityAt: null, + }, + ], + meta: { hiddenTerminatedCount: 0 }, + }); }); - it("returns an empty JSON array when there are no active sessions", async () => { + it("returns an empty JSON data array when there are no active sessions", async () => { mockTmux.mockResolvedValue(null); await program.parseAsync(["node", "test", "session", "ls", "--json"]); expect(consoleSpy).toHaveBeenCalledTimes(1); - expect(JSON.parse(String(consoleSpy.mock.calls[0][0]))).toEqual([]); + expect(JSON.parse(String(consoleSpy.mock.calls[0][0]))).toEqual({ + data: [], + meta: { hiddenTerminatedCount: 0 }, + }); + }); + + it("hides terminated sessions by default and prints a footer", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); + writeFileSync(join(sessionsDir, "app-3"), "branch=feat/c\nstatus=killed\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "session", "ls"]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + expect(output).toContain("app-1"); + expect(output).not.toContain("app-2"); + expect(output).not.toContain("app-3"); + expect(output).toContain("2 terminated sessions hidden"); + expect(output).toContain("--include-terminated"); + }); + + it("shows terminated sessions when --include-terminated is passed", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync([ + "node", + "test", + "session", + "ls", + "--include-terminated", + ]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + expect(output).toContain("app-1"); + expect(output).toContain("app-2"); + expect(output).not.toContain("terminated sessions hidden"); + }); + + it("reports hiddenTerminatedCount in JSON output when filtering terminal sessions", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=done\n"); + writeFileSync(join(sessionsDir, "app-3"), "branch=feat/c\nstatus=killed\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "session", "ls", "--json"]); + + expect(consoleSpy).toHaveBeenCalledTimes(1); + const parsed = JSON.parse(String(consoleSpy.mock.calls[0][0])); + expect(parsed.data).toHaveLength(1); + expect(parsed.data[0].id).toBe("app-1"); + expect(parsed.meta.hiddenTerminatedCount).toBe(2); + }); + + it("returns hiddenTerminatedCount=0 in JSON when --include-terminated is passed", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync([ + "node", + "test", + "session", + "ls", + "--json", + "--include-terminated", + ]); + + expect(consoleSpy).toHaveBeenCalledTimes(1); + const parsed = JSON.parse(String(consoleSpy.mock.calls[0][0])); + expect(parsed.data).toHaveLength(2); + expect(parsed.meta.hiddenTerminatedCount).toBe(0); + }); + + it("hides legacy on-disk metadata with status=merged even when pr= URL is absent", async () => { + // Regression test for the reviewer's smoke-test case on PR #1340: a metadata + // file with `status=merged` but no `pr=` was still showing as active because + // lifecycle reconstruction (synthesizePRState) collapsed pr.state to "none" + // when the URL was missing, which made isTerminalSession() return false. + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); // no pr= + writeFileSync(join(sessionsDir, "app-3"), "branch=feat/c\nstatus=done\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "session", "ls", "--json"]); + + expect(consoleSpy).toHaveBeenCalledTimes(1); + const parsed = JSON.parse(String(consoleSpy.mock.calls[0][0])); + expect(parsed.data.map((e: { id: string }) => e.id)).toEqual(["app-1"]); + expect(parsed.meta.hiddenTerminatedCount).toBe(2); + }); + + it("filters lifecycle-driven terminal sessions (runtime exited, pr merged, session terminated)", async () => { + // Seed three sessions whose legacy status is non-terminal ("working"), but + // whose canonical lifecycle marks them as terminal in three distinct ways. + // This exercises the lifecycle branch of isTerminalSession (types.ts:250), + // which short-circuits before TERMINAL_STATUSES is consulted. + const makeLifecycleSession = ( + id: string, + mutate: (lc: ReturnType) => void, + ): Session => { + const lifecycle = createInitialCanonicalLifecycle("worker", new Date()); + lifecycle.session.state = "working"; + lifecycle.session.reason = "task_in_progress"; + lifecycle.runtime.state = "alive"; + lifecycle.runtime.reason = "process_running"; + mutate(lifecycle); + return { + id, + projectId: "my-app", + status: "working", + activity: null, + activitySignal: createActivitySignal("unavailable"), + lifecycle, + branch: null, + issueId: null, + pr: null, + workspacePath: null, + runtimeHandle: null, + agentInfo: null, + createdAt: new Date(), + lastActivityAt: new Date(), + metadata: {}, + } satisfies Session; + }; + + mockSessionManager.list.mockResolvedValue([ + makeLifecycleSession("app-1", () => { + // alive — should remain visible + }), + makeLifecycleSession("app-2", (lc) => { + lc.runtime.state = "exited"; + lc.runtime.reason = "process_not_running"; + }), + makeLifecycleSession("app-3", (lc) => { + lc.pr.state = "merged"; + lc.pr.reason = "merged_by_user"; + }), + makeLifecycleSession("app-4", (lc) => { + lc.session.state = "terminated"; + lc.session.reason = "manually_killed"; + }), + ]); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "session", "ls", "--json"]); + + expect(consoleSpy).toHaveBeenCalledTimes(1); + const parsed = JSON.parse(String(consoleSpy.mock.calls[0][0])); + expect(parsed.data.map((e: { id: string }) => e.id)).toEqual(["app-1"]); + expect(parsed.meta.hiddenTerminatedCount).toBe(3); }); }); diff --git a/packages/cli/__tests__/commands/status.test.ts b/packages/cli/__tests__/commands/status.test.ts index 6724b9b5f..e9af2024a 100644 --- a/packages/cli/__tests__/commands/status.test.ts +++ b/packages/cli/__tests__/commands/status.test.ts @@ -14,6 +14,9 @@ import { type Session, type SessionManager, type ActivityState, + createInitialCanonicalLifecycle, + createActivitySignal, + sessionFromMetadata, } from "@aoagents/ao-core"; const { @@ -157,7 +160,13 @@ function parseMetadata(content: string): Record { return meta; } -/** Build Session objects from metadata files in sessionsDir. */ +/** + * Build Session objects from metadata files in sessionsDir. + * + * Routes through the real `sessionFromMetadata()` so lifecycle reconstruction + * runs exactly as in production `sm.list()`. Tests that assert filter behavior + * against on-disk metadata therefore exercise the full path. + */ function buildSessionsFromDir( dir: string, projectId: string, @@ -168,21 +177,11 @@ function buildSessionsFromDir( return files.map((name) => { const content = readFileSync(join(dir, name), "utf-8"); const meta = parseMetadata(content); - return { - id: name, + return sessionFromMetadata(name, meta, { projectId, - status: (meta["status"] as Session["status"]) || "spawning", - activity: activityOverride !== undefined ? activityOverride : null, - branch: meta["branch"] || null, - issueId: meta["issue"] || null, - pr: null, - workspacePath: meta["worktree"] || null, runtimeHandle: { id: name, runtimeName: "tmux", data: {} }, - agentInfo: null, - createdAt: new Date(), - lastActivityAt: new Date(), - metadata: meta, - } satisfies Session; + activity: activityOverride !== undefined ? activityOverride : null, + }); }); } @@ -573,7 +572,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed).toHaveLength(1); expect(parsed[0].prNumber).toBe(10); expect(parsed[0].ciStatus).toBe("passing"); @@ -709,7 +708,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed[0].pendingThreads).toBeNull(); }); @@ -734,7 +733,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed[0].activity).toBe("ready"); }); @@ -755,7 +754,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed[0].activity).toBeNull(); }); @@ -776,7 +775,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed[0].activity).toBeNull(); }); @@ -800,7 +799,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed[0].activity).toBeNull(); }); @@ -821,10 +820,16 @@ describe("status command", () => { }); mockGit.mockResolvedValue("feat/dead"); - await program.parseAsync(["node", "test", "status", "--json"]); + await program.parseAsync([ + "node", + "test", + "status", + "--json", + "--include-terminated", + ]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed[0].activity).toBe("exited"); }); @@ -859,7 +864,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); - const parsed = JSON.parse(consoleSpy.mock.calls.map((c) => c[0]).join("")); + const parsed = JSON.parse(consoleSpy.mock.calls.map((c) => c[0]).join("")).data; expect(parsed[0].name).toBe("app-orchestrator"); expect(parsed[0].pr).toBeNull(); expect(parsed[0].prNumber).toBeNull(); @@ -933,7 +938,7 @@ describe("status command", () => { await program.parseAsync(["node", "test", "status", "--json"]); const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); - const parsed = JSON.parse(jsonCalls); + const parsed = JSON.parse(jsonCalls).data; expect(parsed).toHaveLength(2); expect( parsed.find((entry: { name: string }) => entry.name === "app-orchestrator"), @@ -1162,4 +1167,159 @@ describe("status command", () => { configurable: true, }); }); + + it("hides terminated sessions by default and prints a footer", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); + writeFileSync(join(sessionsDir, "app-3"), "branch=feat/c\nstatus=done\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "status"]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + expect(output).toContain("app-1"); + expect(output).not.toContain("app-2"); + expect(output).not.toContain("app-3"); + expect(output).toContain("2 terminated sessions hidden"); + expect(output).toContain("--include-terminated"); + }); + + it("shows terminated sessions when --include-terminated is passed", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=killed\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync([ + "node", + "test", + "status", + "--include-terminated", + ]); + + const output = consoleSpy.mock.calls.map((c) => String(c[0])).join("\n"); + expect(output).toContain("app-1"); + expect(output).toContain("app-2"); + expect(output).not.toContain("terminated sessions hidden"); + }); + + it("reports hiddenTerminatedCount in JSON output when filtering terminal sessions", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); + writeFileSync(join(sessionsDir, "app-3"), "branch=feat/c\nstatus=done\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "status", "--json"]); + + const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(jsonCalls); + expect(parsed.data).toHaveLength(1); + expect(parsed.data[0].name).toBe("app-1"); + expect(parsed.meta.hiddenTerminatedCount).toBe(2); + }); + + it("returns hiddenTerminatedCount=0 in JSON when --include-terminated is passed", async () => { + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync([ + "node", + "test", + "status", + "--json", + "--include-terminated", + ]); + + const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(jsonCalls); + expect(parsed.data).toHaveLength(2); + expect(parsed.meta.hiddenTerminatedCount).toBe(0); + }); + + it("hides legacy on-disk metadata with status=merged even when pr= URL is absent", async () => { + // Regression test for the reviewer's smoke-test case on PR #1340: a legacy + // metadata file with `status=merged` but no `pr=` URL must still be treated + // as terminal. Routes through the real sessionFromMetadata → lifecycle path. + writeFileSync(join(sessionsDir, "app-1"), "branch=feat/a\nstatus=working\n"); + writeFileSync(join(sessionsDir, "app-2"), "branch=feat/b\nstatus=merged\n"); // no pr= + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "status", "--json"]); + + const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(jsonCalls); + expect(parsed.data.map((e: { name: string }) => e.name)).toEqual(["app-1"]); + expect(parsed.meta.hiddenTerminatedCount).toBe(1); + }); + + it("filters lifecycle-driven terminal sessions (runtime exited, pr merged, session terminated)", async () => { + // Exercises the lifecycle branch of isTerminalSession — legacy status stays + // "working" but canonical lifecycle puts the session in a terminal state. + const makeLifecycleSession = ( + id: string, + mutate: (lc: ReturnType) => void, + ): Session => { + const lifecycle = createInitialCanonicalLifecycle("worker", new Date()); + lifecycle.session.state = "working"; + lifecycle.session.reason = "task_in_progress"; + lifecycle.runtime.state = "alive"; + lifecycle.runtime.reason = "process_running"; + mutate(lifecycle); + return { + id, + projectId: "my-app", + status: "working", + activity: null, + activitySignal: createActivitySignal("unavailable"), + lifecycle, + branch: null, + issueId: null, + pr: null, + workspacePath: null, + runtimeHandle: null, + agentInfo: null, + createdAt: new Date(), + lastActivityAt: new Date(), + metadata: {}, + } satisfies Session; + }; + + mockSessionManager.list.mockResolvedValue([ + makeLifecycleSession("app-1", () => { + // alive — should remain visible + }), + makeLifecycleSession("app-2", (lc) => { + lc.runtime.state = "exited"; + lc.runtime.reason = "process_not_running"; + }), + makeLifecycleSession("app-3", (lc) => { + lc.pr.state = "merged"; + lc.pr.reason = "merged_by_user"; + }), + makeLifecycleSession("app-4", (lc) => { + lc.session.state = "terminated"; + lc.session.reason = "manually_killed"; + }), + ]); + + mockTmux.mockResolvedValue(null); + mockGit.mockResolvedValue(null); + + await program.parseAsync(["node", "test", "status", "--json"]); + + const jsonCalls = consoleSpy.mock.calls.map((c) => c[0]).join(""); + const parsed = JSON.parse(jsonCalls); + expect(parsed.data.map((e: { name: string }) => e.name)).toEqual(["app-1"]); + expect(parsed.meta.hiddenTerminatedCount).toBe(3); + }); }); diff --git a/packages/cli/src/commands/session.ts b/packages/cli/src/commands/session.ts index 132f36e45..89cc88fbe 100644 --- a/packages/cli/src/commands/session.ts +++ b/packages/cli/src/commands/session.ts @@ -3,6 +3,7 @@ import chalk from "chalk"; import type { Command } from "commander"; import { isOrchestratorSession, + isTerminalSession, loadConfig, SessionNotRestorableError, WorkspaceMissingError, @@ -36,8 +37,17 @@ export function registerSession(program: Command): void { .description("List all sessions") .option("-p, --project ", "Filter by project ID") .option("-a, --all", "Include orchestrator sessions") + .option( + "--include-terminated", + "Include terminated sessions (killed/done/merged/terminated/errored/cleanup)", + ) .option("--json", "Output as JSON") - .action(async (opts: { project?: string; all?: boolean; json?: boolean }) => { + .action(async (opts: { + project?: string; + all?: boolean; + includeTerminated?: boolean; + json?: boolean; + }) => { const config = loadConfig(); if (opts.project && !config.projects[opts.project]) { console.error(chalk.red(`Unknown project: ${opts.project}`)); @@ -48,12 +58,21 @@ export function registerSession(program: Command): void { const allSessions = await sm.list(opts.project); // Filter out orchestrator sessions unless --all is passed - const sessions = opts.all + const withoutOrchestrators = opts.all ? allSessions : allSessions.filter( (s) => !isOrchestratorSessionName(config, s.id, s.projectId), ); + // Count terminal sessions that would be hidden by default, then + // drop them unless --include-terminated is passed. + const hiddenTerminatedCount = opts.includeTerminated + ? 0 + : withoutOrchestrators.filter(isTerminalSession).length; + const sessions = opts.includeTerminated + ? withoutOrchestrators + : withoutOrchestrators.filter((s) => !isTerminalSession(s)); + // Group sessions by project const byProject = new Map(); for (const s of sessions) { @@ -149,10 +168,24 @@ export function registerSession(program: Command): void { } if (opts.json) { - console.log(JSON.stringify(jsonOutput, null, 2)); + console.log( + JSON.stringify( + { data: jsonOutput, meta: { hiddenTerminatedCount } }, + null, + 2, + ), + ); return; } + if (hiddenTerminatedCount > 0) { + console.log( + chalk.dim( + ` ${hiddenTerminatedCount} terminated session${hiddenTerminatedCount !== 1 ? "s" : ""} hidden. Use --include-terminated to show.`, + ), + ); + } + console.log(); }); diff --git a/packages/cli/src/commands/status.ts b/packages/cli/src/commands/status.ts index 5666380da..2805ebf5a 100644 --- a/packages/cli/src/commands/status.ts +++ b/packages/cli/src/commands/status.ts @@ -13,6 +13,7 @@ import { type Tracker, type ProjectConfig, isOrchestratorSession, + isTerminalSession, loadConfig, } from "@aoagents/ao-core"; import { git, getTmuxSessions, getTmuxActivity } from "../lib/shell.js"; @@ -51,6 +52,7 @@ interface StatusOptions { json?: boolean; watch?: boolean; interval?: string; + includeTerminated?: boolean; } const DEFAULT_WATCH_INTERVAL_SECONDS = 5; @@ -242,6 +244,10 @@ export function registerStatus(program: Command): void { .option("--json", "Output as JSON") .option("-w, --watch", "Refresh the status view continuously") .option("--interval ", "Refresh interval in seconds (default: 5)") + .option( + "--include-terminated", + "Include terminated sessions (killed/done/merged/terminated/errored/cleanup)", + ) .action(async (opts: StatusOptions) => { if (opts.watch && opts.json) { console.error(chalk.red("--watch cannot be used with --json.")); @@ -281,7 +287,17 @@ export function registerStatus(program: Command): void { // Use session manager to list sessions (metadata-based, not tmux-based) const sm = await getSessionManager(config); const registry = await getPluginRegistry(config); - const sessions = await sm.list(opts.project); + const allSessions = await sm.list(opts.project); + + // Count terminal sessions that would be hidden by default, then drop + // them unless --include-terminated is passed. Recomputed each render + // so --watch reflects transitions to terminal state live. + const hiddenTerminatedCount = opts.includeTerminated + ? 0 + : allSessions.filter(isTerminalSession).length; + const sessions = opts.includeTerminated + ? allSessions + : allSessions.filter((s) => !isTerminalSession(s)); if (!opts.json) { console.log(banner("AGENT ORCHESTRATOR STATUS")); @@ -376,7 +392,13 @@ export function registerStatus(program: Command): void { } if (opts.json) { - console.log(JSON.stringify(jsonOutput, null, 2)); + console.log( + JSON.stringify( + { data: jsonOutput, meta: { hiddenTerminatedCount } }, + null, + 2, + ), + ); } else { console.log( chalk.dim( @@ -387,6 +409,14 @@ export function registerStatus(program: Command): void { ), ); + if (hiddenTerminatedCount > 0) { + console.log( + chalk.dim( + ` ${hiddenTerminatedCount} terminated session${hiddenTerminatedCount !== 1 ? "s" : ""} hidden. Use --include-terminated to show.`, + ), + ); + } + // Check for issues awaiting verification across all projects try { let unverifiedTotal = 0; diff --git a/packages/core/src/__tests__/lifecycle-state.test.ts b/packages/core/src/__tests__/lifecycle-state.test.ts index 6e76fa633..eac7e2c85 100644 --- a/packages/core/src/__tests__/lifecycle-state.test.ts +++ b/packages/core/src/__tests__/lifecycle-state.test.ts @@ -79,6 +79,22 @@ describe("parseCanonicalLifecycle", () => { expect(deriveLegacyStatus(parsed, "merged")).toBe("merged"); }); + it("preserves terminal merged state on legacy metadata with no pr URL", () => { + // Regression: `status=merged` without `pr=` used to rehydrate as + // `pr.state=none` + `session.state=idle`, making isTerminalSession() return + // false and leaking merged sessions into active CLI listings. + const parsed = parseCanonicalLifecycle({ + status: "merged", + createdAt: "2025-01-01T00:00:00.000Z", + }); + + expect(parsed.pr.state).toBe("merged"); + expect(parsed.pr.reason).toBe("merged"); + expect(parsed.pr.number).toBeNull(); + expect(parsed.pr.url).toBeNull(); + expect(deriveLegacyStatus(parsed, "merged")).toBe("merged"); + }); + it("preserves explicit null payload fields instead of rehydrating stale flat metadata", () => { const lifecycle = createInitialCanonicalLifecycle("worker", new Date("2025-01-01T00:00:00Z")); lifecycle.session.state = "working"; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 472e418af..1d5491bf6 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -40,6 +40,7 @@ export { listMetadata, } from "./metadata.js"; export { createInitialCanonicalLifecycle, deriveLegacyStatus } from "./lifecycle-state.js"; +export { sessionFromMetadata } from "./utils/session-from-metadata.js"; // Lifecycle transitions — centralized transition boundary (#137) export { diff --git a/packages/core/src/lifecycle-state.ts b/packages/core/src/lifecycle-state.ts index ee767c94c..993751a09 100644 --- a/packages/core/src/lifecycle-state.ts +++ b/packages/core/src/lifecycle-state.ts @@ -209,6 +209,13 @@ function synthesizePRState(meta: Record, status: SessionStatus): } { const prUrl = meta["pr"] ?? null; if (!prUrl) { + // Legacy metadata can record `status=merged` without `pr=` (the PR URL was + // never written, or was pruned). Preserve the terminal truth — the legacy + // status is authoritative for session terminality — so downstream + // consumers like isTerminalSession don't lose the "merged" signal. + if (status === "merged") { + return { state: "merged", reason: "merged", number: null, url: null }; + } return { state: "none", reason: "not_created", number: null, url: null }; } const parsed = parsePrFromUrl(prUrl);