diff --git a/docs/guide.md b/docs/guide.md index 09e8e71..ffc3c3c 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -455,10 +455,14 @@ back to its project directory), inside one operator-chosen chat. ``` - Each session **claims one topic** on start (and when that session runs `/telegram topics on` or `/telegram topics `). - Under herdr it is named after the pane's **agent name** — the identity you assigned - with `herdr agent start ` or `agent rename`, which is one-to-one with the - session. Otherwise it falls back to `basename(cwd)`, which is all there is outside - herdr and is ambiguous when several panes share one parent directory. A session + The title is the strongest identity available, in this order: the pane's **herdr agent + name** (what you assigned with `herdr agent start ` or `agent rename`, one-to-one + with the session), then the pane's **herdr space label**, then `basename(cwd)`. + The space is consulted because the agent lookup is best-effort — it reads herdr over a + socket and returns nothing on failure — and because `topics tidy on` re-derives the + title on every restart rather than once. On a host where several panes share one parent + directory, that combination is what used to title every topic after the shared + directory. Outside herdr, `basename(cwd)` is all there is. A session restarted in the same directory **re-adopts** its existing topic instead of creating a duplicate; a second live session in the same directory gets a `-` topic. Sessions already running when topics are first diff --git a/src/index.ts b/src/index.ts index b95b011..25bd7b8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -59,6 +59,7 @@ import { loadRegistry, purgeRouteDir, releaseThread, + sessionTopicTitle, watchRoute, } from "./topics"; import { BlockedPings, askQuestionSummary } from "./blocked"; @@ -925,11 +926,9 @@ export default function telegramExtension(pi: ExtensionAPI): void { let name = basename(cwd); try { await captureOwnSpace(ctx); - // Prefer the name herdr knows this pane by. It is assigned deliberately and - // is one-to-one with the session, whereas `basename(cwd)` is whatever the - // parent directory happens to be called — every pane under one tree then - // claims a topic with the same useless title. - name = ownAgentName ?? name; + // Identity beats the directory name; see `sessionTopicTitle` for why the + // herdr space is consulted before falling back to `basename(cwd)`. + name = sessionTopicTitle(ownAgentName, ownSpace?.label, cwd); const r = loadRegistry(warn); const sessionId = ctx?.sessionManager.getSessionId(); const sessionFile = ctx?.sessionManager.getSessionFile(); diff --git a/src/topics.test.ts b/src/topics.test.ts index 0e08978..c17c521 100644 --- a/src/topics.test.ts +++ b/src/topics.test.ts @@ -16,6 +16,7 @@ import { loadRegistry, purgeRouteDir, releaseThread, + sessionTopicTitle, staleThreads, watchRoute, writeRouted, @@ -43,6 +44,42 @@ const topicMsg = (over: Partial = {}): TgMessage => ({ ...over, }); +describe("sessionTopicTitle", () => { + test("prefers the herdr agent name over everything else", () => { + expect(sessionTopicTitle("veltrosecurity", "veltrosecurity", "/root/.omp/conductor")).toBe("veltrosecurity"); + }); + + test("falls back to the herdr space when the agent lookup came back empty", () => { + // The lookup reads herdr over a socket and swallows its own failure, so + // "no agent name" is a routine outcome, not a broken host. + expect(sessionTopicTitle(undefined, "veltrosecurity", "/root/.omp/conductor")).toBe("veltrosecurity"); + }); + + test("falls back to the cwd basename outside herdr", () => { + expect(sessionTopicTitle(undefined, undefined, "/srv/checkouts/api")).toBe("api"); + }); + + test("treats blank identities as absent rather than titling a topic with nothing", () => { + // Telegram rejects an empty topic name, and a space with no custom name + // must not consume the fallback chain on the way past. + expect(sessionTopicTitle("", "", "/srv/checkouts/api")).toBe("api"); + expect(sessionTopicTitle(" ", "veltro", "/srv/checkouts/api")).toBe("veltro"); + expect(sessionTopicTitle(undefined, " spaced ", "/srv/checkouts/api")).toBe("spaced"); + }); + + test("two panes under one directory tree get their own titles, not the shared one", () => { + // The regression this rule exists for: both panes live under + // ~/.omp/conductor, so basename alone titles both of them "conductor" and + // a project's pages land in the other project's topic. + const shared = "/root/.omp/conductor"; + expect(sessionTopicTitle(undefined, "veltrosecurity", shared)).toBe("veltrosecurity"); + expect(sessionTopicTitle(undefined, "conductor", shared)).toBe("conductor"); + // Without a space either, both collapse to the same useless title — which + // is exactly the state that shipped before this fallback existed. + expect(sessionTopicTitle(undefined, undefined, shared)).toBe("conductor"); + }); +}); + describe("decideRoute", () => { const reg = (threads: Record): ThreadRegistry => ({ version: 1, chatId: "100", threads }); const alive = (): boolean => true; diff --git a/src/topics.ts b/src/topics.ts index c00f163..40f86de 100644 --- a/src/topics.ts +++ b/src/topics.ts @@ -1,9 +1,10 @@ // Per-session forum-topic routing. In topics mode each omp session claims one -// Telegram forum topic (named after its project dir) in an operator-designated -// chat; inbound topic messages are routed to the owning session — even across -// processes — via JSON payload files spooled under the shared state dir and a -// per-topic watcher. No network here: this module is pure filesystem + policy, -// so it is fully unit-testable. Telegram I/O stays in api.ts / outbound.ts. +// Telegram forum topic (named by `sessionTopicTitle` below) in an operator- +// designated chat; inbound topic messages are routed to the owning session — +// even across processes — via JSON payload files spooled under the shared state +// dir and a per-topic watcher. No network here: this module is pure filesystem +// + policy, so it is fully unit-testable. Telegram I/O stays in api.ts / +// outbound.ts. import { randomBytes } from "node:crypto"; import { @@ -19,7 +20,7 @@ import { watch, writeFileSync, } from "node:fs"; -import { join } from "node:path"; +import { basename, join } from "node:path"; import { ensureStateDir, statePath } from "./access"; import type { Logger, TgMessage } from "./api"; @@ -51,6 +52,35 @@ export const ROUTED_TTL_MS = 600_000; /** Spool key for untopiced private messages routed to the pinned DM owner. */ export const DM_ROUTE_KEY = "dm" as const; +/** + * The title a newly created session topic gets, strongest identity first. + * + * 1. **herdr agent name** — operator-assigned and one-to-one with the session, + * which is exactly what a per-session topic represents. + * 2. **herdr space label** — equally one-to-one with the pane, and captured by + * a *different* call than the agent name, so it still answers when that + * lookup comes back empty. + * 3. **`basename(cwd)`** — the last resort, and the reason the first two exist: + * every pane under one directory tree claims the same useless title. + * + * The middle rung is not belt-and-braces. The agent lookup reads herdr over a + * socket and swallows its own failure, and `tidy` closes a topic on exit so the + * title is re-derived on every restart rather than once. On a fleet whose panes + * share a parent directory — `~/.omp/conductor/…` for two projects, say — a + * single missed lookup is enough to retitle a live project's topic after the + * shared directory, which is how two projects end up both called "conductor". + * + * Blank is treated as absent throughout: Telegram rejects an empty topic name, + * and a space with no custom name must not consume the fallback chain. + */ +export function sessionTopicTitle( + agentName: string | undefined, + spaceLabel: string | undefined, + cwd: string, +): string { + return agentName?.trim() || spaceLabel?.trim() || basename(cwd); +} + /** * Load threads.json. ENOENT / read error → fresh empty registry. Corrupt JSON → * move aside to threads.json.corrupt-, warn, return fresh. Mirrors loadAccess.