diff --git a/CHANGELOG.md b/CHANGELOG.md index 899947885bb..c238d34eef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Sessions/status: classify ACP spawn-child sessions as `kind: "spawn-child"` instead of `"direct"` in `openclaw sessions` and status output; extract the duplicated session-kind classifier into a shared helper (`src/sessions/classify-session-kind.ts`) so both surfaces stay in sync. Fixes catalog #19. (#79544) - Telegram: delete tool-progress-only draft bubbles before rotating to the real answer, preventing orphaned progress messages in streamed replies. - Codex app-server: keep per-agent `CODEX_HOME` isolation without rewriting `HOME` by default, so Codex-run subprocesses can still find normal user-home config, tokens, and CLI state unless the launch explicitly overrides `HOME`. Thanks @pashpashpash. - ACP: preserve redacted numeric JSON-RPC `RequestError` details in runtime failure text, so backend diagnostics are visible instead of only `Internal error`. Fixes #81126. (#81188) Thanks @vyctorbrzezowski. diff --git a/src/commands/sessions.kind-classification.test.ts b/src/commands/sessions.kind-classification.test.ts new file mode 100644 index 00000000000..b19b0a9a671 --- /dev/null +++ b/src/commands/sessions.kind-classification.test.ts @@ -0,0 +1,214 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { SessionEntry } from "../config/sessions/types.js"; +import { + mockSessionsConfig, + resetMockSessionsConfig, + runSessionsJson, + writeStore, +} from "./sessions.test-helpers.js"; + +/** + * Catalog #19 — `kind` misclassified as `"direct"` for ACP spawn-child sessions. + * + * Bug summary: `classifySessionKey` (defined twice — `src/commands/sessions.ts:136-152` + * and `src/commands/status.summary.runtime.ts:129-145`) classifies a session + * based ONLY on the key shape (`:group:` / `:channel:` substrings) plus + * `entry.chatType`. It ignores `entry.spawnedBy` and `entry.deliveryContext`, + * so ACP spawn-child sessions (e.g., `agent:copilot:acp:` with + * `spawnedBy: "agent:main:telegram:group:..."` and + * `deliveryContext: { channel: "telegram", to: , threadId: }`) + * are misclassified as `kind: "direct"` even though they were spawned from a + * group/topic-bound parent. + * + * Available kinds today: + * "global" | "unknown" | "cron" | "group" | "direct" + * + * The fix shape proposed in the catalog is to add a new `"spawn-child"` kind + * (or, alternatively, fall through to the parent's classification — but the + * catalog calls out `"spawn-child"` as the cleanest minimal fix). + * + * NOTE ON DUPLICATION: the same logic lives in two places — + * - `src/commands/sessions.ts:136-152` (called by `sessionsCommand`, + * the path under test here) + * - `src/commands/status.summary.runtime.ts:129-145` + * The eventual fix MUST update both, or extract a shared helper. + * + * NOTE ON SURFACE: `classifySessionKey` is private to each file (not exported), + * so this test drives the classification through the exposed seam: + * `sessionsCommand --json` and inspects the `kind` field of each session row + * (mirroring `src/commands/sessions.test.ts` and `sessions.acp-runtime-metadata.test.ts`). + */ + +mockSessionsConfig(); + +const { sessionsCommand } = await import("./sessions.js"); + +type SessionRowKind = "global" | "unknown" | "cron" | "group" | "direct" | "spawn-child"; + +type SessionsJsonPayload = { + sessions?: Array<{ + key: string; + kind: SessionRowKind; + }>; +}; + +const ACP_SPAWN_CHILD_KEY = "agent:copilot:acp:7de23a0a-799d-4d63-b1b1-a7de9d4cd840"; +const ACP_DM_KEY = "agent:copilot:acp:86b7b5af-3773-4a56-b244-069d6c5d3db9"; +const TELEGRAM_GROUP_KEY = "agent:main:telegram:group:-1003967207344:topic:1"; + +/** + * SessionEntry shape mirroring the deployed-container record described in + * the catalog (a copilot ACP session spawned by a telegram supergroup parent). + * Only the fields the classifier and the JSON emit path care about are set; + * everything else stays unset / default. + */ +function buildAcpSpawnChildEntry(): SessionEntry { + return { + sessionId: "spawn-child-session-id", + updatedAt: Date.now() - 2 * 60_000, + spawnedBy: TELEGRAM_GROUP_KEY, + deliveryContext: { + channel: "telegram", + to: "-1003967207344", + threadId: 323, + }, + // No chatType — ACP spawn-child entries don't carry one. The classifier + // must infer "this came from a group" from spawnedBy / deliveryContext. + }; +} + +/** + * Plain DM-driven ACP session: same key shape (`agent:copilot:acp:`) + * but no `spawnedBy` and a direct delivery context. Today's classifier + * correctly reports `"direct"` for this; that behavior should be preserved + * after the fix. + */ +function buildAcpDirectEntry(): SessionEntry { + return { + sessionId: "dm-session-id", + updatedAt: Date.now() - 5 * 60_000, + deliveryContext: { + channel: "telegram", + to: "+15555550123", + }, + }; +} + +/** + * Group session with a key that explicitly embeds `:group:` — the + * classifier's existing key-shape branch picks this up correctly today + * and reports `"group"`. + */ +function buildTelegramGroupEntry(): SessionEntry { + return { + sessionId: "group-session-id", + updatedAt: Date.now() - 10 * 60_000, + chatType: "group", + }; +} + +describe("sessionsCommand kind classification (catalog #19)", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2025-12-06T00:00:00Z")); + }); + + afterEach(() => { + resetMockSessionsConfig(); + vi.useRealTimers(); + }); + + it("RED: ACP spawn-child session must NOT be classified as 'direct'", async () => { + // RED today. The classifier ignores `spawnedBy` and `deliveryContext`, + // so an ACP key with no `:group:` substring and no `chatType` falls + // through to `"direct"`. Operators see this session in + // `openclaw sessions --json` as `kind: "direct"` even though it was + // plainly spawned from a group/topic. See `src/commands/sessions.ts:136-152`. + const store = writeStore( + { [ACP_SPAWN_CHILD_KEY]: buildAcpSpawnChildEntry() }, + "sessions-kind-spawn-child-red", + ); + + const payload = await runSessionsJson(sessionsCommand, store); + const row = payload.sessions?.find((entry) => entry.key === ACP_SPAWN_CHILD_KEY); + + expect( + row, + `Expected sessionsCommand --json to include a row for ${ACP_SPAWN_CHILD_KEY}; got none.`, + ).toBeDefined(); + expect( + row?.kind, + `ACP spawn-child session ${ACP_SPAWN_CHILD_KEY} is misclassified: kind="${row?.kind}". ` + + `It carries spawnedBy="${TELEGRAM_GROUP_KEY}" and deliveryContext.channel="telegram", ` + + `which clearly mark it as a non-direct origin. The classifier at ` + + `src/commands/sessions.ts:136-152 ignores these fields and returns "direct".`, + ).not.toBe("direct"); + }); + + it("RED (fix-shape): ACP spawn-child session should resolve to 'spawn-child'", async () => { + // RED today; flips GREEN once the proposed fix lands. + // + // The catalog's recommended fix introduces a new `"spawn-child"` kind + // checked BEFORE the key-shape branch so spawn-child ACP sessions take + // precedence over the fallback `"direct"` classification. + // + // If the fix author chooses a different label (e.g., `"acp-child"`) or + // a different shape (e.g., fall through to the parent's classification + // and report `"group"`), update this assertion to match. The structural + // point is that `entry.spawnedBy` / `entry.deliveryContext` MUST drive + // the classification for ACP children. + const store = writeStore( + { [ACP_SPAWN_CHILD_KEY]: buildAcpSpawnChildEntry() }, + "sessions-kind-spawn-child-fix-shape", + ); + + const payload = await runSessionsJson(sessionsCommand, store); + const row = payload.sessions?.find((entry) => entry.key === ACP_SPAWN_CHILD_KEY); + + expect(row).toBeDefined(); + expect( + row?.kind, + `ACP spawn-child session ${ACP_SPAWN_CHILD_KEY} should classify as "spawn-child" ` + + `(or whichever non-direct label the fix author chooses). Got "${row?.kind}". ` + + `Fix locations: src/commands/sessions.ts:136-152 AND ` + + `src/commands/status.summary.runtime.ts:129-145 (the same logic is duplicated; ` + + `extract to a shared helper or update both).`, + ).toBe("spawn-child"); + }); + + it("GREEN control: non-spawn-child ACP DM session resolves to 'direct'", async () => { + // GREEN today. An ACP-keyed session WITHOUT `spawnedBy` and with a + // direct delivery context (or none) correctly resolves to `"direct"`. + // This control proves the test infrastructure exercises the real + // classification path; if it accidentally regressed to a different + // value, that would indicate the test harness was broken. + const store = writeStore( + { [ACP_DM_KEY]: buildAcpDirectEntry() }, + "sessions-kind-acp-direct-control", + ); + + const payload = await runSessionsJson(sessionsCommand, store); + const row = payload.sessions?.find((entry) => entry.key === ACP_DM_KEY); + + expect(row).toBeDefined(); + expect(row?.kind).toBe("direct"); + }); + + it("GREEN control: telegram group key with chatType='group' resolves to 'group'", async () => { + // GREEN today. The classifier's key-shape branch (`:group:` substring) + // and the `chatType === "group"` branch both fire for this entry, + // yielding `"group"`. This control proves the existing happy-path + // classification still works and is not silently broken by the test + // harness. + const store = writeStore( + { [TELEGRAM_GROUP_KEY]: buildTelegramGroupEntry() }, + "sessions-kind-group-control", + ); + + const payload = await runSessionsJson(sessionsCommand, store); + const row = payload.sessions?.find((entry) => entry.key === TELEGRAM_GROUP_KEY); + + expect(row).toBeDefined(); + expect(row?.kind).toBe("group"); + }); +}); diff --git a/src/commands/sessions.test.ts b/src/commands/sessions.test.ts index 3ce01cc02a3..1cf16f3eeaf 100644 --- a/src/commands/sessions.test.ts +++ b/src/commands/sessions.test.ts @@ -49,7 +49,7 @@ describe("sessionsCommand", () => { const row = logs.find((line) => line.includes("+15555550123")) ?? ""; expect(row).toBe( - "direct +15555550123 45m ago pi:opus OpenAI Codex 2.0k/32k (6%) id:abc123", + "direct +15555550123 45m ago pi:opus OpenAI Codex 2.0k/32k (6%) id:abc123", ); }); @@ -86,7 +86,7 @@ describe("sessionsCommand", () => { const row = logs.find((line) => line.includes("agent:main:main")) ?? ""; expect(row).toBe( - "direct agent:main:main 1m ago claude-opus-4-7 Claude CLI unknown/200k (?%) id:main-session", + "direct agent:main:main 1m ago claude-opus-4-7 Claude CLI unknown/200k (?%) id:main-session", ); }); @@ -121,7 +121,7 @@ describe("sessionsCommand", () => { const row = logs.find((line) => line.includes("agent:main:main")) ?? ""; expect(row).toBe( - "direct agent:main:main 1m ago claude-opus-4-7 Claude CLI unknown/200k (?%) id:main-session", + "direct agent:main:main 1m ago claude-opus-4-7 Claude CLI unknown/200k (?%) id:main-session", ); }); @@ -141,7 +141,7 @@ describe("sessionsCommand", () => { const row = logs.find((line) => line.includes("quietchat:group:demo")) ?? ""; expect(row).toBe( - "group quietchat:group:demo 5m ago pi:opus OpenAI Codex unknown/32k (?%) think:high id:xyz", + "group quietchat:group:demo 5m ago pi:opus OpenAI Codex unknown/32k (?%) think:high id:xyz", ); }); diff --git a/src/commands/sessions.ts b/src/commands/sessions.ts index fb0124d30fd..96e4027a115 100644 --- a/src/commands/sessions.ts +++ b/src/commands/sessions.ts @@ -7,7 +7,8 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { info } from "../globals.js"; import { parseAgentSessionKey } from "../routing/session-key.js"; import { type RuntimeEnv, writeRuntimeJson } from "../runtime.js"; -import { isCronSessionKey, isAcpSessionKey } from "../sessions/session-key-utils.js"; +import { classifySessionKind, type SessionKind } from "../sessions/classify-session-kind.js"; +import { isAcpSessionKey } from "../sessions/session-key-utils.js"; import { createLazyImportLoader } from "../shared/lazy-promise.js"; import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; import { resolveAgentRuntimeLabel } from "../status/agent-runtime-label.js"; @@ -31,7 +32,7 @@ import { type SessionRow = SessionDisplayRow & { agentId: string; - kind: "cron" | "direct" | "group" | "global" | "unknown"; + kind: SessionKind; agentRuntime: ReturnType; runtimeLabel: string; /** @@ -45,7 +46,7 @@ type SessionRow = SessionDisplayRow & { }; const AGENT_PAD = 10; -const KIND_PAD = 6; +const KIND_PAD = 11; // "spawn-child".length — longest kind label const RUNTIME_PAD = 18; const TOKENS_PAD = 20; const DEFAULT_SESSIONS_LIMIT = 100; @@ -172,25 +173,6 @@ async function lookupContextTokensForDisplay(model: string): Promise { const label = kind.padEnd(KIND_PAD); if (!rich) { @@ -318,7 +300,7 @@ export async function sessionsCommand( agentId, acpRuntime, agentRuntime, - kind: classifySessionKey(row.key, store[row.key]), + kind: classifySessionKind(row.key, store[row.key]), runtimeLabel: resolveSessionRuntimeLabel({ cfg, entry, diff --git a/src/commands/status.summary.runtime.ts b/src/commands/status.summary.runtime.ts index be8482f908a..e13aa4a0135 100644 --- a/src/commands/status.summary.runtime.ts +++ b/src/commands/status.summary.runtime.ts @@ -6,7 +6,7 @@ import { normalizeProviderId } from "../agents/provider-id.js"; import { resolveAgentModelPrimaryValue } from "../config/model-input.js"; import type { SessionEntry } from "../config/sessions/types.js"; import type { OpenClawConfig } from "../config/types.js"; -import { isCronSessionKey } from "../sessions/session-key-utils.js"; +import { classifySessionKind } from "../sessions/classify-session-kind.js"; import { normalizeLowercaseStringOrEmpty, normalizeOptionalString, @@ -125,25 +125,6 @@ function resolveConfiguredProviderContextTokens( return undefined; } -function classifySessionKey(key: string, entry?: SessionEntry) { - if (key === "global") { - return "global"; - } - if (key === "unknown") { - return "unknown"; - } - if (isCronSessionKey(key)) { - return "cron"; - } - if (entry?.chatType === "group" || entry?.chatType === "channel") { - return "group"; - } - if (key.includes(":group:") || key.includes(":channel:")) { - return "group"; - } - return "direct"; -} - function resolveSessionModelRef( cfg: OpenClawConfig, entry?: @@ -221,7 +202,7 @@ function resolveContextTokensForModel(params: { export const statusSummaryRuntime = { resolveContextTokensForModel, - classifySessionKey, + classifySessionKey: classifySessionKind, resolveSessionModelRef, resolveSessionRuntimeLabel, resolveConfiguredStatusModelRef, diff --git a/src/commands/status.types.ts b/src/commands/status.types.ts index a00ca467cec..42e05b4e94c 100644 --- a/src/commands/status.types.ts +++ b/src/commands/status.types.ts @@ -1,11 +1,12 @@ import type { ChannelId } from "../channels/plugins/types.public.js"; +import type { SessionKind } from "../sessions/classify-session-kind.js"; import type { TaskAuditSummary } from "../tasks/task-registry.audit.js"; import type { TaskRegistrySummary } from "../tasks/task-registry.types.js"; export type SessionStatus = { agentId?: string; key: string; - kind: "cron" | "direct" | "group" | "global" | "unknown"; + kind: SessionKind; sessionId?: string; updatedAt: number | null; age: number | null; diff --git a/src/sessions/classify-session-kind.ts b/src/sessions/classify-session-kind.ts new file mode 100644 index 00000000000..214fb01d5cb --- /dev/null +++ b/src/sessions/classify-session-kind.ts @@ -0,0 +1,39 @@ +import { isCronSessionKey } from "./session-key-utils.js"; + +export type SessionKind = "cron" | "direct" | "group" | "global" | "spawn-child" | "unknown"; + +/** + * Classify a session key + entry into a display kind. + * + * Evaluation order matters — more-specific signals take priority: + * 1. sentinel keys ("global", "unknown") + * 2. cron key shape + * 3. spawn-child (entry has `spawnedBy`) — checked before key-shape so ACP + * spawn-child sessions with opaque keys are not misclassified as "direct" + * 4. group/channel chatType or key-shape substring + * 5. fallback: "direct" + */ +export function classifySessionKind( + key: string, + entry?: { chatType?: string | null; spawnedBy?: string | null }, +): SessionKind { + if (key === "global") { + return "global"; + } + if (key === "unknown") { + return "unknown"; + } + if (isCronSessionKey(key)) { + return "cron"; + } + if (entry?.spawnedBy) { + return "spawn-child"; + } + if (entry?.chatType === "group" || entry?.chatType === "channel") { + return "group"; + } + if (key.includes(":group:") || key.includes(":channel:")) { + return "group"; + } + return "direct"; +}