fix: a failed target resolution names the rung that declined (#72) + release 0.12.4

docs/guide.md

@@ -42,7 +42,7 @@ Install the published extension:
 
 ```bash
 omp plugin install omp-telegram
-omp plugin list        # → omp-telegram@0.12.3
+omp plugin list        # → omp-telegram@0.12.4
 ```
 
 There is no build step and no runtime dependency install. The extension uses

package.json

@@ -1,6 +1,6 @@
 {
   "name": "omp-telegram",
-  "version": "0.12.3",
+  "version": "0.12.4",
   "description": "Standalone Telegram bridge, session topics, and herdr control plane for omp",
   "type": "module",
   "license": "MIT",

src/index.ts

@@ -922,15 +922,41 @@ export default function telegramExtension(pi: ExtensionAPI): void {
     }
   }
 
-  function resolveToolTarget(
+  /**
+   * The target, or why every rung declined (#72).
+   *
+   * `declines` exists because the refusal callers used to get —
+   * "no active telegram chat" — is true and unactionable: it cannot separate
+   * "topics are off" from "a live claim exists but its session identity is not
+   * mine", and those want opposite responses. Measured on conductor#882: that
+   * ambiguity cost two 300-second arming attempts and a five-hour delivery hold
+   * while a matching claim sat in the registry the whole time.
+   *
+   * Counts, never ids. `resolveProjectTopicId`'s rule holds here too: a log line
+   * is a place ids leak from.
+   */
+  interface TargetResolution {
+    target?: ResolvedToolTarget;
+    declines: readonly string[];
+  }
+
+  function resolveTarget(
     ctx: ExtensionContext | undefined,
     currentAccess: Access = loadAccess(warn),
-  ): ResolvedToolTarget | undefined {
+  ): TargetResolution {
+    const declines: string[] = [];
     const active = outbound.lastTarget();
-    if (active) return { ...active, source: "active inbound" };
+    if (active) return { target: { ...active, source: "active inbound" }, declines: [] };
+    declines.push("nothing inbound this turn");
+
     if (ownTopic && currentAccess.topicsChat) {
-      return { chatId: currentAccess.topicsChat, threadId: ownTopic.threadId, source: "session topic" };
+      return {
+        target: { chatId: currentAccess.topicsChat, threadId: ownTopic.threadId, source: "session topic" },
+        declines: [],
+      };
     }
+    declines.push(currentAccess.topicsChat ? "this session owns no topic" : "topics are off");
+
     const current = ctx ?? lastCtx;
     const identity = {
       sessionId: current?.sessionManager?.getSessionId(),
@@ -940,18 +966,37 @@ export default function telegramExtension(pi: ExtensionAPI): void {
     if (currentAccess.topicsChat) {
       const registry = loadRegistry(warn);
       if (registry.chatId === currentAccess.topicsChat) {
-        for (const [threadId, entry] of Object.entries(registry.threads)) {
+        const entries = Object.entries(registry.threads);
+        for (const [threadId, entry] of entries) {
           const belongs = hasIdentity ? sameSession(entry, identity) : entry.pid === process.pid;
           if (belongs) {
-            return { chatId: currentAccess.topicsChat, threadId: Number(threadId), source: "topic registry" };
+            return {
+              target: { chatId: currentAccess.topicsChat, threadId: Number(threadId), source: "topic registry" },
+              declines: [],
+            };
           }
         }
+        // The case #882 could not diagnose, and the reason this whole list
+        // exists: claims are present, so the registry is fine and the bridge has
+        // run — the mismatch is identity, which points at a resumed session or a
+        // plugin too old to compare session files.
+        declines.push(
+          entries.length === 0
+            ? "topic registry carries no claims"
+            : `topic registry has ${entries.length} claim(s), none matching this session's ` +
+              `${hasIdentity ? "identity" : "pid (no session identity available)"}`,
+        );
+      } else {
+        declines.push("topic registry names a different chat");
       }
     }
+
     const dmOwner = loadDmOwner(warn);
     const dmChat = pairedOwnerId(currentAccess);
     const ownsDm = dmOwner && (hasIdentity ? sameSession(dmOwner, identity) : dmOwner.pid === process.pid);
-    return dmOwner && dmChat && ownsDm ? { chatId: dmChat, source: "DM owner" } : undefined;
+    if (dmOwner && dmChat && ownsDm) return { target: { chatId: dmChat, source: "DM owner" }, declines: [] };
+    declines.push(dmOwner === undefined ? "no DM owner pinned" : "DM owner is another session");
+    return { declines };
   }
 
   function logTargetFallback(tool: string, resolved: ResolvedToolTarget | undefined): void {
@@ -960,6 +1005,11 @@ export default function telegramExtension(pi: ExtensionAPI): void {
     }
   }
 
+  /** One line per failed call, naming every rung and why it declined (#72). */
+  function targetFailureDetail(tool: string, declines: readonly string[]): string {
+    return `[telegram] ${tool} found no target: ${declines.join("; ")}`;
+  }
+
   /**
    * Claim this session's forum topic once. Exact saved-session identity wins.
    * A missing remote topic is forgotten and replaced; otherwise create a topic.
@@ -2004,12 +2054,16 @@ export default function telegramExtension(pi: ExtensionAPI): void {
           chatId = p.chat_id;
           threadId = p.thread_id != null && p.thread_id !== "" ? Number(p.thread_id) : undefined;
         } else {
-          const resolved = resolveToolTarget(ctx, currentAccess);
-          logTargetFallback("telegram_send", resolved);
-          chatId = resolved?.chatId;
-          threadId = resolved?.threadId;
+          const resolution = resolveTarget(ctx, currentAccess);
+          logTargetFallback("telegram_send", resolution.target);
+          chatId = resolution.target?.chatId;
+          threadId = resolution.target?.threadId;
+          if (chatId === undefined) {
+            const detail = targetFailureDetail("telegram_send", resolution.declines);
+            log.warn(detail);
+            return errorResult(`no active telegram chat — pass chat_id. ${detail}`);
+          }
         }
-        if (!chatId) return errorResult("no active telegram chat — pass chat_id");
         assertAllowedChat(chatId, currentAccess);
         const replyTo = p.reply_to != null && p.reply_to !== "" ? Number(p.reply_to) : undefined;
         const ids: number[] = [];
@@ -2058,15 +2112,23 @@ export default function telegramExtension(pi: ExtensionAPI): void {
       const questions: PromptQuestion[] = p.questions.map((q) => ({ ...q, options: q.options ?? [] }));
       const canTerminal = ctx?.hasUI === true && typeof ctx.ui?.askDialog === "function";
       let resolved = activePromptTarget ? { ...activePromptTarget } : undefined;
+      let declines: readonly string[] = [];
       if (!resolved && token.length > 0) {
         const currentAccess = loadAccess(warn);
-        const fallback = resolveToolTarget(ctx, currentAccess);
-        logTargetFallback("telegram_ask", fallback);
-        resolved = buildPromptTarget(fallback, currentAccess);
+        const resolution = resolveTarget(ctx, currentAccess);
+        logTargetFallback("telegram_ask", resolution.target);
+        declines = resolution.declines;
+        resolved = buildPromptTarget(resolution.target, currentAccess);
       }
       const target = resolved;
       if (!target && !canTerminal) {
-        return errorResult("telegram_ask has no surface available — no Telegram target and no interactive terminal.");
+        // Name the rungs here too: "no surface available" reads as a config
+        // problem, and the common cause is a target that nearly resolved (#72).
+        const detail = declines.length === 0 ? "" : ` ${targetFailureDetail("telegram_ask", declines)}`;
+        if (detail.length > 0) log.warn(detail.trim());
+        return errorResult(
+          `telegram_ask has no surface available — no Telegram target and no interactive terminal.${detail}`,
+        );
       }
       const posted: AskSurfaceState = { terminal: false, telegram: false };
       const surfaceErrors: AskSurfaceErrors = {};
@@ -2211,10 +2273,16 @@ export default function telegramExtension(pi: ExtensionAPI): void {
       const p = params as ReactParams;
       try {
         const currentAccess = loadAccess(warn);
-        const resolved = p.chat_id ? undefined : resolveToolTarget(ctx, currentAccess);
+        const resolution = p.chat_id ? undefined : resolveTarget(ctx, currentAccess);
+        const resolved = resolution?.target;
         logTargetFallback("telegram_react", resolved);
         const chatId = p.chat_id ?? resolved?.chatId;
-        if (!chatId) return errorResult("no active telegram chat — pass chat_id");
+        if (!chatId) {
+          const detail =
+            resolution === undefined ? "" : ` ${targetFailureDetail("telegram_react", resolution.declines)}`;
+          if (detail.length > 0) log.warn(detail.trim());
+          return errorResult(`no active telegram chat — pass chat_id.${detail}`);
+        }
         assertAllowedChat(chatId, currentAccess);
         await outbound.react(chatId, Number(p.message_id), p.emoji);
         return { content: [{ type: "text", text: "reacted" }] };

src/index.wiring.test.ts

@@ -5,7 +5,7 @@ import { tmpdir } from "node:os";
 import { basename, join } from "node:path";
 import { type Access, defaultAccess, loadAccess } from "./access";
 import telegramExtension from "./index";
-import { claimDmOwner, loadDmOwner, loadRegistry } from "./topics";
+import { claimDmOwner, loadDmOwner, loadRegistry, saveRegistry } from "./topics";
 
 type EventHandler = (event: unknown, ctx: unknown) => unknown;
 type CommandHandler = (args: string, ctx: unknown) => unknown;
@@ -402,15 +402,81 @@ describe("extension wiring", () => {
       { sessionManager: { getSessionId: () => "session-1", getSessionFile: () => "/tmp/session-1.jsonl" } },
     );
     expect(result.isError).toBe(true);
-    expect(result.content[0].text).toBe("no active telegram chat — pass chat_id");
+    // The refusal still leads with the actionable instruction, and now names the
+    // rungs (#72). This case is "a DM owner exists but is somebody else" —
+    // distinct wording from "none pinned", which is the whole point: the two
+    // want opposite responses and used to read identically.
+    expect(result.content[0].text).toContain("no active telegram chat — pass chat_id");
+    expect(result.content[0].text).toContain("DM owner is another session");
+    expect(result.content[0].text).not.toContain("no DM owner pinned");
   });
 
   test("telegram_send still refuses when no bridge session has claimed a target", async () => {
     writeAccess({ enabled: true, allowFrom: ["42"] });
     const h = harness(["ask"]);
     const result = await h.tools.get("telegram_send")!.execute("t", { text: "done" }, undefined, undefined, {});
     expect(result.isError).toBe(true);
-    expect(result.content[0].text).toBe("no active telegram chat — pass chat_id");
+    expect(result.content[0].text).toContain("no active telegram chat — pass chat_id");
+    expect(result.content[0].text).toContain("no DM owner pinned");
+  });
+
+  test("a live claim that is not this session's names the identity mismatch, and leaks no ids (#72)", async () => {
+    // The case conductor#882 could not diagnose. Claims are present, so the
+    // registry is fine and the bridge has run — the mismatch is identity, which
+    // points at a resumed session or a plugin too old to compare session files.
+    // Before this, it read exactly like "no claims at all".
+    writeAccess({ enabled: true, allowFrom: ["42"], topicsChat: "42" });
+    saveRegistry({
+      version: 1,
+      chatId: "42",
+      threads: {
+        "8801": { pid: 999_999, cwd: "/foreign", name: "foreign", claimedAt: 1, sessionId: "foreign-a", sessionFile: "/tmp/foreign-a.jsonl" },
+        "8802": { pid: 999_998, cwd: "/other", name: "other", claimedAt: 2, sessionId: "foreign-b", sessionFile: "/tmp/foreign-b.jsonl" },
+      },
+    });
+    const h = harness(["ask"]);
+    const result = await h.tools.get("telegram_send")!.execute("t", { text: "done" }, undefined, undefined, {
+      sessionManager: { getSessionId: () => "mine", getSessionFile: () => "/tmp/mine.jsonl" },
+    });
+    expect(result.isError).toBe(true);
+    const text = result.content[0].text as string;
+    expect(text).toContain("no active telegram chat — pass chat_id");
+    // Distinguishable from an empty registry, and it says how many exist.
+    expect(text).toContain("topic registry has 2 claim(s), none matching this session's identity");
+    expect(text).not.toContain("carries no claims");
+    // Every rung, one line.
+    expect(text).toContain("nothing inbound this turn");
+    expect(text).toContain("this session owns no topic");
+    expect(text.split("\n")).toHaveLength(1);
+    // Counts, never ids: the precedent is resolveProjectTopicId's rule that a
+    // log line is a place ids leak from.
+    for (const id of ["8801", "8802", "999999", "999998", "/tmp/foreign-a.jsonl", "/tmp/mine.jsonl"]) {
+      expect(text).not.toContain(id);
+    }
+  });
+
+  test("an empty registry says so, rather than blaming identity (#72)", async () => {
+    writeAccess({ enabled: true, allowFrom: ["42"], topicsChat: "42" });
+    saveRegistry({ version: 1, chatId: "42", threads: {} });
+    const h = harness(["ask"]);
+    const result = await h.tools.get("telegram_send")!.execute("t", { text: "done" }, undefined, undefined, {
+      sessionManager: { getSessionId: () => "mine", getSessionFile: () => "/tmp/mine.jsonl" },
+    });
+    expect(result.content[0].text).toContain("topic registry carries no claims");
+    expect(result.content[0].text).not.toContain("none matching");
+  });
+
+  test("a registry for another chat is named as such, not as an identity problem (#72)", async () => {
+    writeAccess({ enabled: true, allowFrom: ["42"], topicsChat: "42" });
+    saveRegistry({
+      version: 1,
+      chatId: "999",
+      threads: { "8801": { pid: process.pid, cwd: "/here", name: "here", claimedAt: 1 } },
+    });
+    const h = harness(["ask"]);
+    const result = await h.tools.get("telegram_send")!.execute("t", { text: "done" }, undefined, undefined, {});
+    expect(result.content[0].text).toContain("topic registry names a different chat");
+    expect(result.content[0].text).not.toContain("999");
   });
 
   test("before_agent_start leaves ask untouched for a plain terminal turn with notify off", async () => {