diff --git a/.changeset/fix-web-tmux-resolver-wrapped-storagekey.md b/.changeset/fix-web-tmux-resolver-wrapped-storagekey.md new file mode 100644 index 000000000..d986967c4 --- /dev/null +++ b/.changeset/fix-web-tmux-resolver-wrapped-storagekey.md @@ -0,0 +1,7 @@ +--- +"@aoagents/ao-web": patch +--- + +Fix DirectTerminal "can't find session" when the project uses a wrapped storageKey. `ao-core` names tmux sessions as `{storageKey}-{sessionId}`, where `storageKey` can be either a bare 12-char hash or the legacy wrapped form `{hash}-{projectName}` (e.g. `361287ebbad1-smx-foundation`). The web resolver only handled the bare-hash form, so lookups for sessions like `sf-orchestrator-1` against the tmux name `361287ebbad1-smx-foundation-sf-orchestrator-1` always returned `null` and the terminal never attached (#1486). + +The resolver now looks up the owning storageKey on disk (from the session record at `~/.agent-orchestrator/{storageKey}/sessions/{sessionId}`) and asks tmux for the exact `{storageKey}-{sessionId}` name. The on-disk record is the authoritative disambiguator, so sessions whose IDs happen to be suffixes of other session IDs (e.g. looking up `app-1` while `my-app-1` exists in the same project) cannot be falsely matched. If the on-disk record is missing, the resolver still recovers bare-hash sessions via the tmux session listing as a fallback. diff --git a/packages/web/server/__tests__/tmux-utils.test.ts b/packages/web/server/__tests__/tmux-utils.test.ts index a4d3581a8..58cf92f31 100644 --- a/packages/web/server/__tests__/tmux-utils.test.ts +++ b/packages/web/server/__tests__/tmux-utils.test.ts @@ -8,6 +8,16 @@ import { describe, it, expect, vi } from "vitest"; import { findTmux, resolveTmuxSession, validateSessionId, SESSION_ID_PATTERN } from "../tmux-utils.js"; +// Default fs adapter for resolveTmuxSession tests — empty AO base directory +// so the on-disk storageKey lookup always misses and we exercise the +// tmux-listing fallback. Tests that want to exercise the on-disk lookup +// path provide their own FsAdapter explicitly. +const emptyFs = { + readdir: () => [], + exists: () => false, + homedir: () => "/tmp/ao-test-home-that-does-not-exist", +}; + // ============================================================================= // validateSessionId // ============================================================================= @@ -355,7 +365,7 @@ describe("resolveTmuxSession", () => { it("returns sessionId for exact match", () => { const mockExec = vi.fn().mockReturnValue(""); - const result = resolveTmuxSession("ao-orchestrator", TMUX, mockExec); + const result = resolveTmuxSession("ao-orchestrator", TMUX, mockExec, emptyFs); expect(result).toBe("ao-orchestrator"); }); @@ -364,7 +374,7 @@ describe("resolveTmuxSession", () => { // This is the critical bugbot fix: without =, "ao-1" matches "ao-15" const mockExec = vi.fn().mockReturnValue(""); - resolveTmuxSession("ao-1", TMUX, mockExec); + resolveTmuxSession("ao-1", TMUX, mockExec, emptyFs); // Must pass "=ao-1" not "ao-1" to has-session expect(mockExec).toHaveBeenCalledWith( @@ -378,7 +388,7 @@ describe("resolveTmuxSession", () => { const customTmux = "/usr/bin/tmux"; const mockExec = vi.fn().mockReturnValue(""); - resolveTmuxSession("my-session", customTmux, mockExec); + resolveTmuxSession("my-session", customTmux, mockExec, emptyFs); expect(mockExec).toHaveBeenCalledWith( customTmux, @@ -391,7 +401,7 @@ describe("resolveTmuxSession", () => { // If "ao-15" exists as both exact and hash-prefixed, return exact const mockExec = vi.fn().mockReturnValue(""); - const result = resolveTmuxSession("ao-15", TMUX, mockExec); + const result = resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs); expect(result).toBe("ao-15"); // Should only call has-session, not list-sessions @@ -401,7 +411,7 @@ describe("resolveTmuxSession", () => { it("does not call list-sessions when exact match succeeds", () => { const mockExec = vi.fn().mockReturnValue(""); - resolveTmuxSession("ao-15", TMUX, mockExec); + resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs); expect(mockExec).toHaveBeenCalledTimes(1); expect(mockExec).toHaveBeenCalledWith( @@ -426,7 +436,7 @@ describe("resolveTmuxSession", () => { return "8474d6f29887-ao-15\na1b2c3d4e5f6-ao-16\nao-orchestrator\n"; }); - const result = resolveTmuxSession("ao-15", TMUX, mockExec); + const result = resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs); expect(result).toBe("8474d6f29887-ao-15"); }); @@ -441,7 +451,7 @@ describe("resolveTmuxSession", () => { return "8474d6f29887-ao-15\n8474d6f29887-ao-16\n"; }); - const result = resolveTmuxSession("ao-1", TMUX, mockExec); + const result = resolveTmuxSession("ao-1", TMUX, mockExec, emptyFs); expect(result).toBeNull(); }); @@ -459,7 +469,7 @@ describe("resolveTmuxSession", () => { }); // Should match the one with valid hash prefix, not the ambiguous one - expect(resolveTmuxSession("app-1", TMUX, mockExec)).toBe("8474d6f29887-app-1"); + expect(resolveTmuxSession("app-1", TMUX, mockExec, emptyFs)).toBe("8474d6f29887-app-1"); }); it("rejects session names where hash prefix is not 12-char hex", () => { @@ -479,7 +489,7 @@ describe("resolveTmuxSession", () => { ].join("\n") + "\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("only matches valid 12-char lowercase hex prefix", () => { @@ -491,7 +501,7 @@ describe("resolveTmuxSession", () => { return "abcdef012345-ao-15\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("abcdef012345-ao-15"); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBe("abcdef012345-ao-15"); }); it("matches the correct session among many", () => { @@ -508,7 +518,7 @@ describe("resolveTmuxSession", () => { ].join("\n") + "\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("112233445566-ao-15"); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBe("112233445566-ao-15"); }); it("matches ao-1 to hash-ao-1 (not hash-ao-15)", () => { @@ -524,7 +534,7 @@ describe("resolveTmuxSession", () => { ].join("\n") + "\n"; }); - expect(resolveTmuxSession("ao-1", TMUX, mockExec)).toBe("aabbccddeef0-ao-1"); + expect(resolveTmuxSession("ao-1", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-ao-1"); }); it("matches session with multiple hyphens in name", () => { @@ -536,7 +546,7 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-my-long-session-name\n112233445566-other-session\n"; }); - expect(resolveTmuxSession("my-long-session-name", TMUX, mockExec)) + expect(resolveTmuxSession("my-long-session-name", TMUX, mockExec, emptyFs)) .toBe("aabbccddeef0-my-long-session-name"); }); @@ -549,7 +559,7 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-my_session\n112233445566-other_session\n"; }); - expect(resolveTmuxSession("my_session", TMUX, mockExec)).toBe("aabbccddeef0-my_session"); + expect(resolveTmuxSession("my_session", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-my_session"); }); it("passes list-sessions format flag correctly", () => { @@ -561,7 +571,7 @@ describe("resolveTmuxSession", () => { return "some-session\n"; }); - resolveTmuxSession("ao-99", TMUX, mockExec); + resolveTmuxSession("ao-99", TMUX, mockExec, emptyFs); expect(mockExec).toHaveBeenNthCalledWith(2, TMUX, @@ -580,11 +590,12 @@ describe("resolveTmuxSession", () => { return "ao-15-extended\nao-15-backup\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("does NOT match hash-prefixed session with extra suffix", () => { - // "aabbccddeef0-ao-15-backup" has valid hash but substring(13) is "ao-15-backup" not "ao-15" + // "aabbccddeef0-ao-15-backup" has valid hash but substring(13) is "ao-15-backup" + // which neither equals "ao-15" nor ends with "-ao-15". const mockExec = vi.fn() .mockImplementationOnce(() => { throw new Error("session not found"); @@ -593,7 +604,226 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-ao-15-backup\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); + }); + + it("resolves wrapped-storageKey session via on-disk lookup (issue #1486)", () => { + // Issue #1486: when the project config uses a wrapped storageKey like + // "361287ebbad1-smx-foundation", ao-core names the tmux session + // "361287ebbad1-smx-foundation-sf-orchestrator-1". The resolver must + // find the storageKey on disk (from the session file at + // ~/.agent-orchestrator/361287ebbad1-smx-foundation/sessions/sf-orchestrator-1) + // and then ask tmux whether the full `{storageKey}-{sessionId}` exists. + const fs = { + readdir: () => ["361287ebbad1-smx-foundation", "other-unrelated-dir"], + exists: (p: string) => p.endsWith("/361287ebbad1-smx-foundation/sessions/sf-orchestrator-1"), + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); // exact match fails + }) + .mockImplementationOnce(() => { + return ""; // has-session on full tmux name succeeds (no throw) + }); + + const result = resolveTmuxSession("sf-orchestrator-1", TMUX, mockExec, fs); + + expect(result).toBe("361287ebbad1-smx-foundation-sf-orchestrator-1"); + // Verifies the resolver asks tmux for the exact `{storageKey}-{sessionId}` name. + expect(mockExec).toHaveBeenNthCalledWith( + 2, + TMUX, + ["has-session", "-t", "=361287ebbad1-smx-foundation-sf-orchestrator-1"], + { timeout: 5000 }, + ); + }); + + it("resolves bare-hash session via on-disk lookup", () => { + const fs = { + readdir: () => ["aabbccddeef0"], + exists: (p: string) => p.endsWith("/aabbccddeef0/sessions/ao-15"), + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return ""; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec, fs)).toBe("aabbccddeef0-ao-15"); + }); + + it("does NOT false-match app-1 against bare session my-app-1 (codex review concern)", () => { + // Critical: a bare-hash session `aabbccddeef0-my-app-1` has sessionId + // `my-app-1`. When a user asks for `app-1`, the resolver must NOT return + // it — the trailing `-app-1` is coincidental. The on-disk lookup finds + // no `app-1` session, and the tmux-listing fallback only accepts exact + // remainder matches, so this correctly returns null. + const fs = { + readdir: () => ["aabbccddeef0"], + // Only my-app-1 exists on disk, app-1 does not. + exists: (p: string) => p.endsWith("/aabbccddeef0/sessions/my-app-1"), + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); // exact match on "app-1" fails + }) + .mockImplementationOnce(() => { + // list-sessions fallback sees my-app-1 + return "aabbccddeef0-my-app-1\n"; + }); + + expect(resolveTmuxSession("app-1", TMUX, mockExec, fs)).toBeNull(); + }); + + it("does NOT false-match when wrapped session belongs to a different project", () => { + // Storage key `aabbccddeef0-other-project` owns sessionId `app-1`. + // A user looking up `app-1` for a DIFFERENT project (with storageKey + // we don't have on disk) must not attach to the wrong project. + // With on-disk lookup, we find the right storageKey unambiguously. + const fs = { + readdir: () => ["aabbccddeef0-other-project", "112233445566-my-project"], + exists: (p: string) => + p.endsWith("/112233445566-my-project/sessions/app-1"), + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return ""; // has-session on the correct name succeeds + }); + + const result = resolveTmuxSession("app-1", TMUX, mockExec, fs); + + expect(result).toBe("112233445566-my-project-app-1"); + }); + + it("falls back to tmux list when on-disk session record is missing (bare hash)", () => { + // If the on-disk sessions/{sessionId} record is missing (e.g. filesystem + // scrubbed), we still recover bare-hash sessions via the list-sessions + // fallback. Wrapped-storageKey sessions cannot be safely recovered + // without the on-disk record — that's the intended safety trade-off. + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-ao-15"); + }); + + it("does NOT match sessions without a valid hex hash prefix via fallback", () => { + // Safety: non-hex prefixes must never be treated as ao sessions. + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "nonhexprefix-smx-foundation-sf-orchestrator-1\n"; + }); + + expect(resolveTmuxSession("sf-orchestrator-1", TMUX, mockExec, emptyFs)).toBeNull(); + }); + + it("probes later candidates when earlier storageKey has no live tmux session", () => { + // Two projects both have sessionId `app-1` on disk. The first one + // (alphabetically) has a stale metadata dir but no live tmux session. + // The resolver must continue to the next candidate and find the live one. + const fs = { + readdir: () => [ + "aaaaaaaaaaaa-stale-project", + "bbbbbbbbbbbb-live-project", + ], + exists: (p: string) => + p.endsWith("/aaaaaaaaaaaa-stale-project/sessions/app-1") || + p.endsWith("/bbbbbbbbbbbb-live-project/sessions/app-1"), + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); // exact match fails + }) + .mockImplementationOnce(() => { + throw new Error("session not found"); // stale candidate has no tmux session + }) + .mockImplementationOnce(() => { + return ""; // live candidate has tmux session + }); + + const result = resolveTmuxSession("app-1", TMUX, mockExec, fs); + + expect(result).toBe("bbbbbbbbbbbb-live-project-app-1"); + expect(mockExec).toHaveBeenNthCalledWith( + 2, + TMUX, + ["has-session", "-t", "=aaaaaaaaaaaa-stale-project-app-1"], + { timeout: 5000 }, + ); + expect(mockExec).toHaveBeenNthCalledWith( + 3, + TMUX, + ["has-session", "-t", "=bbbbbbbbbbbb-live-project-app-1"], + { timeout: 5000 }, + ); + }); + + it("accepts wrapped storageKeys with spaces/unicode in the project name", () => { + // Legacy storageKeys use basename(projectPath), which has no character + // restrictions on-disk. Regexes that reject spaces or unicode would + // strand these projects the same way the bare-hash-only regex did. + const fs = { + readdir: () => ["aabbccddeef0-My App (v2)"], + exists: (p: string) => p.endsWith("/aabbccddeef0-My App (v2)/sessions/ao-15"), + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return ""; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec, fs)) + .toBe("aabbccddeef0-My App (v2)-ao-15"); + }); + + it("ignores AO base directories that don't match the storageKey pattern", () => { + // Files like `.DS_Store`, `portfolio`, `{hash}-observability` exist in + // the AO base. Only `{12-hex}` and `{12-hex}-{projectName}` are valid + // storageKeys. Extraneous entries must not be probed for sessions. + const probed: string[] = []; + const fs = { + readdir: () => [".DS_Store", "portfolio", "aabbccddeef0-observability", "aabbccddeef0"], + exists: (p: string) => { + probed.push(p); + return p.endsWith("/aabbccddeef0/sessions/ao-15"); + }, + homedir: () => "/home/user", + }; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return ""; + }); + + const result = resolveTmuxSession("ao-15", TMUX, mockExec, fs); + + expect(result).toBe("aabbccddeef0-ao-15"); + // observability dir is a valid storageKey-pattern match ({hash}-{name}) + // so it will be probed, but `.DS_Store` and `portfolio` must not be. + expect(probed.some((p) => p.includes("/.DS_Store/"))).toBe(false); + expect(probed.some((p) => p.includes("/portfolio/"))).toBe(false); }); it("returns first match when multiple hash-prefixed sessions exist for same ID", () => { @@ -607,7 +837,7 @@ describe("resolveTmuxSession", () => { }); // find() returns the first match - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("aabbccddeef0-ao-15"); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-ao-15"); }); }); @@ -625,7 +855,7 @@ describe("resolveTmuxSession", () => { return "some-other-session\nanother-session\n"; }); - expect(resolveTmuxSession("ao-99", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-99", TMUX, mockExec, emptyFs)).toBeNull(); }); it("returns null when tmux is not running", () => { @@ -633,7 +863,7 @@ describe("resolveTmuxSession", () => { throw new Error("no server running on /tmp/tmux-501/default"); }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("returns null when list-sessions returns empty string", () => { @@ -645,7 +875,7 @@ describe("resolveTmuxSession", () => { return ""; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("returns null when list-sessions returns only newlines", () => { @@ -657,7 +887,7 @@ describe("resolveTmuxSession", () => { return "\n\n\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("returns null when list-sessions throws (no sessions exist)", () => { @@ -669,7 +899,7 @@ describe("resolveTmuxSession", () => { throw new Error("no sessions"); // list-sessions fails }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("returns null when has-session times out and list-sessions is empty", () => { @@ -681,7 +911,7 @@ describe("resolveTmuxSession", () => { return "\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); }); @@ -699,7 +929,7 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-abcdef123456\n"; }); - expect(resolveTmuxSession("abcdef123456", TMUX, mockExec)).toBe("aabbccddeef0-abcdef123456"); + expect(resolveTmuxSession("abcdef123456", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-abcdef123456"); }); it("handles single-char session ID", () => { @@ -711,7 +941,7 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-a\n112233445566-b\n"; }); - expect(resolveTmuxSession("a", TMUX, mockExec)).toBe("aabbccddeef0-a"); + expect(resolveTmuxSession("a", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-a"); }); it("does not match session without valid hash prefix", () => { @@ -723,7 +953,7 @@ describe("resolveTmuxSession", () => { return "xao-15\nnotahash-ao-15\n"; }); - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("handles Windows-style line endings in list-sessions output", () => { @@ -738,7 +968,7 @@ describe("resolveTmuxSession", () => { // \r will remain in the session name after split("\n") // substring(13) of "aabbccddeef0-ao-15\r" is "ao-15\r" which !== "ao-15" // This documents the current behavior — tmux shouldn't produce \r\n on unix - expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + expect(resolveTmuxSession("ao-15", TMUX, mockExec, emptyFs)).toBeNull(); }); it("handles very long session list", () => { @@ -754,7 +984,7 @@ describe("resolveTmuxSession", () => { .mockImplementationOnce(() => sessions); const hex50 = (50).toString(16).padStart(12, "0"); - expect(resolveTmuxSession("session-50", TMUX, mockExec)).toBe(`${hex50}-session-50`); + expect(resolveTmuxSession("session-50", TMUX, mockExec, emptyFs)).toBe(`${hex50}-session-50`); }); it("handles session list where target is last entry", () => { @@ -766,7 +996,7 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-ao-1\n112233445566-ao-2\nffeeddccbbaa-ao-3\na0b1c2d3e4f5-ao-target\n"; }); - expect(resolveTmuxSession("ao-target", TMUX, mockExec)).toBe("a0b1c2d3e4f5-ao-target"); + expect(resolveTmuxSession("ao-target", TMUX, mockExec, emptyFs)).toBe("a0b1c2d3e4f5-ao-target"); }); it("handles session list where target is first entry", () => { @@ -778,7 +1008,7 @@ describe("resolveTmuxSession", () => { return "aabbccddeef0-ao-target\n112233445566-ao-2\nffeeddccbbaa-ao-3\n"; }); - expect(resolveTmuxSession("ao-target", TMUX, mockExec)).toBe("aabbccddeef0-ao-target"); + expect(resolveTmuxSession("ao-target", TMUX, mockExec, emptyFs)).toBe("aabbccddeef0-ao-target"); }); it("works with different tmux paths", () => { @@ -786,7 +1016,7 @@ describe("resolveTmuxSession", () => { for (const tmuxPath of paths) { const mockExec = vi.fn().mockReturnValue(""); - resolveTmuxSession("ao-15", tmuxPath, mockExec); + resolveTmuxSession("ao-15", tmuxPath, mockExec, emptyFs); expect(mockExec).toHaveBeenCalledWith(tmuxPath, ["has-session", "-t", "=ao-15"], { timeout: 5000 }); } }); diff --git a/packages/web/server/tmux-utils.ts b/packages/web/server/tmux-utils.ts index 6a476fbe1..f22ed514e 100644 --- a/packages/web/server/tmux-utils.ts +++ b/packages/web/server/tmux-utils.ts @@ -6,6 +6,9 @@ */ import { execFileSync } from "node:child_process"; +import { readdirSync, existsSync } from "node:fs"; +import { homedir } from "node:os"; +import { join } from "node:path"; /** Session ID validation regex — alphanumeric, hyphens, underscores only */ export const SESSION_ID_PATTERN = /^[a-zA-Z0-9_-]+$/; @@ -13,6 +16,70 @@ export const SESSION_ID_PATTERN = /^[a-zA-Z0-9_-]+$/; /** Hash prefix pattern — 12-char lowercase hex, as generated by generateConfigHash */ const HASH_PREFIX_PATTERN = /^[a-f0-9]{12}-/; +/** + * StorageKey pattern — either bare 12-hex hash or `{hash}-{projectName}` + * wrapped form. The wrapped suffix comes from `basename(projectPath)` on + * disk so it can contain any character a filesystem path component allows + * (spaces, unicode, etc.); we only require that something non-empty + * follows the `-` separator. Security: storageKey is passed to + * `execFileSync` which bypasses the shell, so arbitrary characters are + * safe in the argv. + */ +const STORAGE_KEY_PATTERN = /^[a-f0-9]{12}(-.+)?$/; + +/** Filesystem accessors injected for testability. */ +interface FsAdapter { + readdir: (path: string) => string[]; + exists: (path: string) => boolean; + homedir: () => string; +} + +const defaultFs: FsAdapter = { + // Only return subdirectory names. `readdirSync` without withFileTypes + // includes plain files, so a stray file like `aabbccddeef0` would pass + // STORAGE_KEY_PATTERN and trigger an unnecessary existsSync probe. + readdir: (p) => + readdirSync(p, { withFileTypes: true }) + .filter((e) => e.isDirectory()) + .map((e) => e.name), + exists: (p) => existsSync(p), + homedir, +}; + +/** + * Find every storageKey that owns a given sessionId by scanning the AO + * base directory for projects whose `sessions/{sessionId}` file exists. + * + * This is the authoritative disambiguation step: tmux names alone are + * ambiguous when storageKey can take either the bare-hash or wrapped + * `{hash}-{projectName}` form. The on-disk session record is unique per + * storageKey so it tells us exactly which tmux name to expect. + * + * Returns all candidates (not just the first). Multiple projects can + * share a sessionId like `app-1`, and the caller must probe each until + * it finds a live tmux session — otherwise a stale metadata dir from + * one project could shadow the live session of another. + */ +function findStorageKeysForSession(sessionId: string, fs: FsAdapter): string[] { + const aoBase = join(fs.homedir(), ".agent-orchestrator"); + let entries: string[]; + try { + entries = fs.readdir(aoBase); + } catch { + return []; + } + + const matches: string[] = []; + for (const entry of entries) { + if (!STORAGE_KEY_PATTERN.test(entry)) continue; + const sessionFile = join(aoBase, entry, "sessions", sessionId); + if (fs.exists(sessionFile)) { + matches.push(entry); + } + } + return matches; +} + /** * Validate a session ID format. * Prevents path traversal, shell injection, and other attacks. @@ -52,23 +119,33 @@ export function findTmux( /** * Resolve a user-facing session ID to its actual tmux session name. * - * The hash-based architecture prefixes tmux session names with a config hash - * (e.g., "8474d6f29887-ao-15" for user-facing "ao-15"). This function: + * ao-core names tmux sessions as `{storageKey}-{sessionId}`, where + * storageKey is either `{12-hex}` (bare hash) or `{12-hex}-{projectName}` + * (legacy wrapped format). This function: * - * 1. Tries exact match first using tmux's `=` prefix syntax to prevent + * 1. Tries exact match using tmux's `=` prefix syntax to prevent * prefix matching (where "ao-1" would incorrectly match "ao-15"). - * 2. Falls back to listing all sessions and finding one with a 12-char hex - * prefix followed by the exact session ID (e.g., `{12-hex}-{sessionId}`). + * 2. Looks up the storageKey owning this sessionId on disk (under + * `~/.agent-orchestrator/{storageKey}/sessions/{sessionId}`) and asks + * tmux whether the exact `{storageKey}-{sessionId}` session exists. + * The on-disk check is authoritative — it avoids ambiguous suffix + * matches where a bare session like `{hash}-my-app-1` could be + * mistaken for a lookup of `app-1`. + * 3. Falls back to listing sessions and matching a hash-prefixed name + * whose remainder equals the sessionId (bare-hash only), so behavior + * stays correct even if the on-disk session record is absent. * * @param sessionId - User-facing session ID (e.g., "ao-15") * @param tmuxPath - Full path to tmux binary * @param execFn - Injectable execFileSync for testing. Defaults to child_process.execFileSync. + * @param fs - Injectable filesystem adapter for testing. * @returns The actual tmux session name, or null if not found */ export function resolveTmuxSession( sessionId: string, tmuxPath: string, execFn: typeof execFileSync = execFileSync, + fs: FsAdapter = defaultFs, ): string | null { // Try exact match first using = prefix for exact matching (e.g., "ao-orchestrator") // Without =, tmux uses prefix matching: "ao-1" would match "ao-15" @@ -79,17 +156,42 @@ export function resolveTmuxSession( // Not an exact match } - // Search for hash-prefixed tmux session (e.g., "8474d6f29887-ao-15" for "ao-15") - // Validate the 12-char hex prefix to avoid ambiguous suffix matches where - // "hash-my-app-1" could falsely match a lookup for "app-1". + // Authoritative path: find candidate storageKeys on disk, then verify + // each exact tmux session name with has-session. This is unambiguous + // even when the storageKey is wrapped (`{hash}-{projectName}`). Walk + // every candidate so a stale metadata dir in one project can't shadow + // the live session of another project with the same sessionId. + // + // Pre-existing limitation (not introduced by this fix): when two + // different projects have both (a) the same user-facing sessionId and + // (b) live tmux sessions, we return the first live match. The terminal + // open protocol carries only the session ID — no project context — so + // there's nothing to disambiguate with here. The previous bare-hash + // code had the same behavior via `Array.find(...)`. A proper fix would + // thread projectId through the mux protocol from the browser URL. + for (const storageKey of findStorageKeysForSession(sessionId, fs)) { + const tmuxName = `${storageKey}-${sessionId}`; + try { + execFn(tmuxPath, ["has-session", "-t", `=${tmuxName}`], { timeout: 5000 }); + return tmuxName; + } catch { + // Session dir exists but tmux session doesn't — try next candidate + } + } + + // Fallback: list sessions and match the bare-hash form only. We + // intentionally do NOT match by trailing suffix here — that would cause + // `app-1` to falsely resolve a distinct session `{hash}-my-app-1`. If a + // wrapped-storageKey session isn't findable on disk above, it's safer + // to return null than to guess. try { const output = execFn(tmuxPath, ["list-sessions", "-F", "#{session_name}"], { timeout: 5000, encoding: "utf8", }) as string; const sessions = output.split("\n").filter(Boolean); - const match = sessions.find((s) => - HASH_PREFIX_PATTERN.test(s) && s.substring(13) === sessionId, + const match = sessions.find( + (s) => HASH_PREFIX_PATTERN.test(s) && s.substring(13) === sessionId, ); if (match) { return match;