mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 19:04:45 +00:00
fix(sessions): classify spawn-child sessions correctly
Classify ACP spawn-child sessions via persisted spawnedBy metadata and share the session kind classifier across sessions/status output. Verified with Azure Crabbox seeded ACP session-store proof, targeted session/status tests, touched-file lint, build, and green PR CI.
This commit is contained in:
@@ -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.
|
||||
|
||||
214
src/commands/sessions.kind-classification.test.ts
Normal file
214
src/commands/sessions.kind-classification.test.ts
Normal file
@@ -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:<uuid>` with
|
||||
* `spawnedBy: "agent:main:telegram:group:..."` and
|
||||
* `deliveryContext: { channel: "telegram", to: <groupId>, threadId: <topic> }`)
|
||||
* 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:<uuid>`)
|
||||
* 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<SessionsJsonPayload>(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<SessionsJsonPayload>(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<SessionsJsonPayload>(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<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === TELEGRAM_GROUP_KEY);
|
||||
|
||||
expect(row).toBeDefined();
|
||||
expect(row?.kind).toBe("group");
|
||||
});
|
||||
});
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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<typeof resolveModelAgentRuntimeMetadata>;
|
||||
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<number | un
|
||||
return lookupContextTokens(model, { allowAsyncLoad: false });
|
||||
}
|
||||
|
||||
function classifySessionKey(key: string, entry?: { chatType?: string | null }): SessionRow["kind"] {
|
||||
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";
|
||||
}
|
||||
|
||||
const formatKindCell = (kind: SessionRow["kind"], rich: boolean) => {
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
39
src/sessions/classify-session-kind.ts
Normal file
39
src/sessions/classify-session-kind.ts
Normal file
@@ -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";
|
||||
}
|
||||
Reference in New Issue
Block a user