Skip to content

Commit 54f36ca

Browse files
suraj-markupclaude
andcommitted
fix(cli): fallback scans the global config so cross-project sessions surface (#1743)
Greptile P1 on PR #1780. `getSessionManager(config)` in the fallback path was built from the current project's config, so `sm.list()` only saw that project's sessions. When `ao stop` ran globally and killed sessions across multiple projects, the synthesized `LastStopState` would have an empty `otherProjects` — defeating the cross-project restore that the pre-existing `readLastStop` path already supports (because `ao stop` writes the cross-project rows at stop time). Mirror the global-config load that the restore step on line ~1002 already does: if the global config exists, prefer it when constructing the fallback session manager. The downstream restore code already loads the global config when `otherProjects` is non-empty, so this just lets the fallback populate that array in the first place. Regression test in start.test.ts asserts both the in-project and a cross-project session get routed to `sm.restore()` after the fallback synthesizes a record from a global-scope `sm.list()`. The two existing fallback tests now pin AO_GLOBAL_CONFIG to a non-existent path so they don't accidentally read the host's real ~/.agent-orchestrator/config.yaml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fb6dc13 commit 54f36ca

2 files changed

Lines changed: 155 additions & 54 deletions

File tree

packages/cli/__tests__/commands/start.test.ts

Lines changed: 142 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1679,65 +1679,143 @@ describe("start command — orchestrator session strategy display", () => {
16791679
// wiped state), `ao start` must still offer to restore recently
16801680
// `manually_killed` sessions by scanning the session manager.
16811681
it("falls back to recently manually-killed sessions when last-stop.json is missing (issue #1743)", async () => {
1682-
mockReadLastStop.mockResolvedValue(null);
1682+
// Force getGlobalConfigPath() to a non-existent path so the fallback's
1683+
// global-config load is a no-op and the test does not read the host's
1684+
// real ~/.agent-orchestrator/config.yaml.
1685+
const origGlobalEnv = process.env["AO_GLOBAL_CONFIG"];
1686+
process.env["AO_GLOBAL_CONFIG"] = join(tmpDir, "no-such-global.yaml");
16831687

1684-
mockConfigRef.current = makeConfig({ "my-app": makeProject() });
1685-
const { findWebDir } = await import("../../src/lib/web-dir.js");
1686-
vi.mocked(findWebDir).mockReturnValue(tmpDir);
1687-
writeFileSync(join(tmpDir, "package.json"), "{}");
1688+
try {
1689+
mockReadLastStop.mockResolvedValue(null);
16881690

1689-
const fakeDashboard = { on: vi.fn(), kill: vi.fn(), emit: vi.fn() };
1690-
mockSpawn.mockReturnValue(fakeDashboard);
1691+
mockConfigRef.current = makeConfig({ "my-app": makeProject() });
1692+
const { findWebDir } = await import("../../src/lib/web-dir.js");
1693+
vi.mocked(findWebDir).mockReturnValue(tmpDir);
1694+
writeFileSync(join(tmpDir, "package.json"), "{}");
16911695

1692-
const recentTerminatedAt = new Date(Date.now() - 60_000).toISOString();
1693-
mockSessionManager.list.mockResolvedValue([
1694-
{
1695-
id: "app-1",
1696-
projectId: "my-app",
1697-
status: "killed",
1698-
activity: "exited",
1699-
metadata: {},
1700-
lastActivityAt: new Date(),
1701-
lifecycle: {
1702-
version: 2,
1703-
session: {
1704-
kind: "worker",
1705-
state: "terminated",
1706-
reason: "manually_killed",
1707-
startedAt: null,
1708-
completedAt: null,
1709-
terminatedAt: recentTerminatedAt,
1710-
lastTransitionAt: recentTerminatedAt,
1696+
const fakeDashboard = { on: vi.fn(), kill: vi.fn(), emit: vi.fn() };
1697+
mockSpawn.mockReturnValue(fakeDashboard);
1698+
1699+
const recentTerminatedAt = new Date(Date.now() - 60_000).toISOString();
1700+
mockSessionManager.list.mockResolvedValue([
1701+
{
1702+
id: "app-1",
1703+
projectId: "my-app",
1704+
status: "killed",
1705+
activity: "exited",
1706+
metadata: {},
1707+
lastActivityAt: new Date(),
1708+
lifecycle: {
1709+
version: 2,
1710+
session: {
1711+
kind: "worker",
1712+
state: "terminated",
1713+
reason: "manually_killed",
1714+
startedAt: null,
1715+
completedAt: null,
1716+
terminatedAt: recentTerminatedAt,
1717+
lastTransitionAt: recentTerminatedAt,
1718+
},
1719+
pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null },
1720+
runtime: { state: "missing", reason: "manual_kill_requested", lastObservedAt: null, handle: null, tmuxName: null },
17111721
},
1712-
pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null },
1713-
runtime: { state: "missing", reason: "manual_kill_requested", lastObservedAt: null, handle: null, tmuxName: null },
17141722
},
1715-
},
1716-
]);
1717-
mockSessionManager.restore.mockResolvedValue(undefined);
1723+
]);
1724+
mockSessionManager.restore.mockResolvedValue(undefined);
17181725

1719-
await program.parseAsync(["node", "test", "start", "--no-orchestrator"]);
1726+
await program.parseAsync(["node", "test", "start", "--no-orchestrator"]);
17201727

1721-
expect(mockSessionManager.restore).toHaveBeenCalledWith("app-1");
1728+
expect(mockSessionManager.restore).toHaveBeenCalledWith("app-1");
1729+
} finally {
1730+
if (origGlobalEnv === undefined) delete process.env["AO_GLOBAL_CONFIG"];
1731+
else process.env["AO_GLOBAL_CONFIG"] = origGlobalEnv;
1732+
}
17221733
});
17231734

17241735
it("does not surface fallback candidates older than the recent window (issue #1743)", async () => {
1725-
mockReadLastStop.mockResolvedValue(null);
1736+
const origGlobalEnv = process.env["AO_GLOBAL_CONFIG"];
1737+
process.env["AO_GLOBAL_CONFIG"] = join(tmpDir, "no-such-global.yaml");
17261738

1727-
mockConfigRef.current = makeConfig({ "my-app": makeProject() });
1728-
const { findWebDir } = await import("../../src/lib/web-dir.js");
1729-
vi.mocked(findWebDir).mockReturnValue(tmpDir);
1730-
writeFileSync(join(tmpDir, "package.json"), "{}");
1739+
try {
1740+
mockReadLastStop.mockResolvedValue(null);
17311741

1732-
const fakeDashboard = { on: vi.fn(), kill: vi.fn(), emit: vi.fn() };
1733-
mockSpawn.mockReturnValue(fakeDashboard);
1742+
mockConfigRef.current = makeConfig({ "my-app": makeProject() });
1743+
const { findWebDir } = await import("../../src/lib/web-dir.js");
1744+
vi.mocked(findWebDir).mockReturnValue(tmpDir);
1745+
writeFileSync(join(tmpDir, "package.json"), "{}");
17341746

1735-
// Terminated 30 minutes ago — beyond the 10-minute fallback window.
1736-
const oldTerminatedAt = new Date(Date.now() - 30 * 60 * 1000).toISOString();
1737-
mockSessionManager.list.mockResolvedValue([
1738-
{
1739-
id: "app-1",
1740-
projectId: "my-app",
1747+
const fakeDashboard = { on: vi.fn(), kill: vi.fn(), emit: vi.fn() };
1748+
mockSpawn.mockReturnValue(fakeDashboard);
1749+
1750+
// Terminated 30 minutes ago — beyond the 10-minute fallback window.
1751+
const oldTerminatedAt = new Date(Date.now() - 30 * 60 * 1000).toISOString();
1752+
mockSessionManager.list.mockResolvedValue([
1753+
{
1754+
id: "app-1",
1755+
projectId: "my-app",
1756+
status: "killed",
1757+
activity: "exited",
1758+
metadata: {},
1759+
lastActivityAt: new Date(),
1760+
lifecycle: {
1761+
version: 2,
1762+
session: {
1763+
kind: "worker",
1764+
state: "terminated",
1765+
reason: "manually_killed",
1766+
startedAt: null,
1767+
completedAt: null,
1768+
terminatedAt: oldTerminatedAt,
1769+
lastTransitionAt: oldTerminatedAt,
1770+
},
1771+
pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null },
1772+
runtime: { state: "missing", reason: "manual_kill_requested", lastObservedAt: null, handle: null, tmuxName: null },
1773+
},
1774+
},
1775+
]);
1776+
1777+
await program.parseAsync(["node", "test", "start", "--no-orchestrator"]);
1778+
1779+
expect(mockSessionManager.restore).not.toHaveBeenCalled();
1780+
expect(mockPromptConfirm).not.toHaveBeenCalled();
1781+
} finally {
1782+
if (origGlobalEnv === undefined) delete process.env["AO_GLOBAL_CONFIG"];
1783+
else process.env["AO_GLOBAL_CONFIG"] = origGlobalEnv;
1784+
}
1785+
});
1786+
1787+
// Regression for Greptile P1 on PR #1780. Before the fix, the fallback's
1788+
// session manager was built from the project-scoped config, so `sm.list()`
1789+
// only saw the current project's sessions and `otherProjects` in the
1790+
// synthesized LastStopState was always empty — defeating the cross-project
1791+
// restore that `readLastStop()` already supports.
1792+
it("fallback uses the global config so cross-project sessions appear in otherProjects (PR #1780)", async () => {
1793+
const origGlobalEnv = process.env["AO_GLOBAL_CONFIG"];
1794+
const globalPath = join(tmpDir, "global-config.yaml");
1795+
// Real, parseable global config so loadConfig(globalPath) succeeds and
1796+
// existsSync(globalPath) returns true. Contents don't have to match the
1797+
// mocked sessions — getSessionManager is mocked to ignore config.
1798+
writeFileSync(
1799+
globalPath,
1800+
"version: 1\nport: 3000\nprojects:\n my-app:\n name: My App\n path: /tmp/my-app\n sessionPrefix: app\n other-app:\n name: Other App\n path: /tmp/other-app\n sessionPrefix: other\n",
1801+
);
1802+
process.env["AO_GLOBAL_CONFIG"] = globalPath;
1803+
1804+
try {
1805+
mockReadLastStop.mockResolvedValue(null);
1806+
mockConfigRef.current = makeConfig({ "my-app": makeProject() });
1807+
const { findWebDir } = await import("../../src/lib/web-dir.js");
1808+
vi.mocked(findWebDir).mockReturnValue(tmpDir);
1809+
writeFileSync(join(tmpDir, "package.json"), "{}");
1810+
1811+
const fakeDashboard = { on: vi.fn(), kill: vi.fn(), emit: vi.fn() };
1812+
mockSpawn.mockReturnValue(fakeDashboard);
1813+
mockPromptConfirm.mockResolvedValue(true);
1814+
1815+
const recent = new Date(Date.now() - 60_000).toISOString();
1816+
const terminated = (id: string, projectId: string) => ({
1817+
id,
1818+
projectId,
17411819
status: "killed",
17421820
activity: "exited",
17431821
metadata: {},
@@ -1750,19 +1828,30 @@ describe("start command — orchestrator session strategy display", () => {
17501828
reason: "manually_killed",
17511829
startedAt: null,
17521830
completedAt: null,
1753-
terminatedAt: oldTerminatedAt,
1754-
lastTransitionAt: oldTerminatedAt,
1831+
terminatedAt: recent,
1832+
lastTransitionAt: recent,
17551833
},
17561834
pr: { state: "none", reason: "not_created", number: null, url: null, lastObservedAt: null },
17571835
runtime: { state: "missing", reason: "manual_kill_requested", lastObservedAt: null, handle: null, tmuxName: null },
17581836
},
1759-
},
1760-
]);
1837+
});
17611838

1762-
await program.parseAsync(["node", "test", "start", "--no-orchestrator"]);
1839+
mockSessionManager.list.mockResolvedValue([
1840+
terminated("app-1", "my-app"),
1841+
terminated("other-1", "other-app"),
1842+
]);
1843+
mockSessionManager.restore.mockResolvedValue(undefined);
17631844

1764-
expect(mockSessionManager.restore).not.toHaveBeenCalled();
1765-
expect(mockPromptConfirm).not.toHaveBeenCalled();
1845+
await program.parseAsync(["node", "test", "start", "--no-orchestrator"]);
1846+
1847+
// Both the in-project session AND the cross-project session must be
1848+
// routed to restore. Pre-fix, only "app-1" would have been seen.
1849+
expect(mockSessionManager.restore).toHaveBeenCalledWith("app-1");
1850+
expect(mockSessionManager.restore).toHaveBeenCalledWith("other-1");
1851+
} finally {
1852+
if (origGlobalEnv === undefined) delete process.env["AO_GLOBAL_CONFIG"];
1853+
else process.env["AO_GLOBAL_CONFIG"] = origGlobalEnv;
1854+
}
17661855
});
17671856

17681857
it("opens the bare dashboard URL when --no-orchestrator skips the orchestrator block", async () => {

packages/cli/src/commands/start.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -980,7 +980,19 @@ async function runStartup(
980980
!!lastStop &&
981981
(lastStop.sessionIds.length > 0 || (lastStop.otherProjects ?? []).length > 0);
982982
if (!lastStopHasContent) {
983-
const fallbackSm = await getSessionManager(config);
983+
// Use the global config so `sm.list()` sees sessions from every
984+
// registered project. The project-scoped `config` only sees the
985+
// current project's sessions, which would silently drop the
986+
// cross-project rows the existing `readLastStop` path preserves
987+
// via `otherProjects`. The restore step on line ~1002 already
988+
// promotes to the global config when otherProjects is non-empty,
989+
// so this just ensures the fallback can populate that array.
990+
let fallbackConfig = config;
991+
const globalPath = getGlobalConfigPath();
992+
if (existsSync(globalPath)) {
993+
fallbackConfig = loadConfig(globalPath);
994+
}
995+
const fallbackSm = await getSessionManager(fallbackConfig);
984996
const fallback = await findRecentlyKilledSessions(fallbackSm, projectId);
985997
if (
986998
fallback &&

0 commit comments

Comments
 (0)