fix(web): resolve tmux sessions when storageKey is wrapped ({hash}-{projectName}) (#1488)
* fix(web): resolve tmux sessions when storageKey is wrapped ({hash}-{projectName})
ao-core names tmux sessions as `{storageKey}-{sessionId}`, where
storageKey can be either a bare 12-hex hash or the legacy wrapped form
`{hash}-{projectName}`. The resolver only handled the bare-hash form, so
the dashboard terminal never attached for any project whose
storageKey includes the project-name segment (the default on 0.3.0).
Fix (closes #1486): resolve the owning storageKey from disk via
`~/.agent-orchestrator/{storageKey}/sessions/{sessionId}`, then ask tmux
for the exact `{storageKey}-{sessionId}` name via `has-session -t =`.
The on-disk record is authoritative, which avoids ambiguous suffix
matches (e.g. looking up `app-1` must not resolve a distinct session
`{hash}-my-app-1`). If multiple storageKeys own the sessionId (rare,
same-sessionId across projects), each candidate is probed so a stale
metadata dir can't shadow the live session of another project. The
tmux-list-sessions fallback still recovers bare-hash sessions when the
on-disk record is absent.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(web): filter readdir to directories only in tmux resolver
readdirSync without withFileTypes includes plain files, so a stray
file matching STORAGE_KEY_PATTERN (e.g. a backup file named
`aabbccddeef0`) would trigger an unnecessary existsSync probe.
Harmless for correctness but an avoidable syscall per attach.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ashish Huddar
GitHub
parent
6d282acf02
commit
eca3001c80
|
|
@ -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.
|
||||
|
|
@ -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 });
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue