mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 20:35:39 -06:00
test(sessions): trim obsolete catalog scaffolding (#123212)
This commit is contained in:
Peter Steinberger
committed by
GitHub
GitHub
parent
44d6e4d30a
commit
37125092d8
@@ -1,4 +1,4 @@
|
||||
// Sessions ACP model display tests cover model metadata rendering for ACP-backed sessions.
|
||||
// Sessions ACP model display tests cover persisted control-plane metadata projection.
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -14,66 +14,21 @@ import {
|
||||
writeStore,
|
||||
} from "./sessions.test-helpers.js";
|
||||
|
||||
/**
|
||||
* Catalog #20 — `model` / `modelProvider` reported as agent-config, not ACP runtime actuals.
|
||||
*
|
||||
* Bug summary: For ACP-keyed sessions (e.g. `agent:copilot:acp:<uuid>`), the
|
||||
* `--json` listing reports the AGENT's configured model
|
||||
* (e.g. `model: "gpt-5.3-codex"`, `modelProvider: "microsoft-foundry"`) — but
|
||||
* those are the values the openclaw-agent-driven flow would have used. When
|
||||
* the same agent runs as an ACP child via `copilot --acp --stdio`, the actual
|
||||
* underlying model selection lives inside copilot CLI and is independent of
|
||||
* the agent's configured model. The listing happily reports the agent default
|
||||
* regardless of whether the session actually ran via ACP.
|
||||
*
|
||||
* `resolveSessionDisplayModelRef` (`src/commands/sessions-display-model.ts:123-148`)
|
||||
* has zero ACP-awareness: it only consults the session entry's persisted
|
||||
* `model` / `modelProvider` / `modelOverride` and the agent's configured
|
||||
* default. It never inspects the session key or the persisted ACP metadata.
|
||||
*
|
||||
* Decided fix shape (catalog #20, mirrors #18): SENTINEL OVERLAY at the call
|
||||
* site, gated on BOTH key shape AND persisted SQLite ACP metadata. Key shape
|
||||
* alone is not sufficient because ACP bridge sessions (translator.ts) also use
|
||||
* ACP-shaped keys without ever writing `SessionAcpMeta` — those sessions run
|
||||
* the normal configured model and must not receive the sentinel.
|
||||
*
|
||||
* When `isAcpSessionKey(row.key)` is true AND SQLite ACP metadata exists, the
|
||||
* JSON-emit path overlays `{ provider: "acpx", model: "<agentId>-acp" }` on
|
||||
* top of the resolver result. The resolver itself stays pure.
|
||||
*
|
||||
* NOTE ON DRIVING SURFACE: `resolveSessionDisplayModelRef` is exported, but
|
||||
* the bug as observed by operators surfaces through `sessions --json`, so we
|
||||
* drive the test end-to-end through `sessionsCommand --json` (mirroring the
|
||||
* #19 test pattern). This proves the bug at the actual emit site that
|
||||
* operators see, not just in the resolver in isolation.
|
||||
*/
|
||||
|
||||
mockSessionsConfig();
|
||||
|
||||
const { sessionsCommand } = await import("./sessions.js");
|
||||
|
||||
type SessionsJsonPayload = {
|
||||
sessions?: Array<{
|
||||
key: string;
|
||||
model?: string | null;
|
||||
modelProvider?: string | null;
|
||||
}>;
|
||||
};
|
||||
|
||||
const ACP_SESSION_KEY = "agent:copilot:acp:86b7b5af-3773-4a56-b244-069d6c5d3db9";
|
||||
const NON_ACP_SESSION_KEY = "agent:copilot:main";
|
||||
|
||||
const AGENT_CONFIGURED_MODEL = "gpt-5.3-codex";
|
||||
const AGENT_CONFIGURED_PROVIDER = "microsoft-foundry";
|
||||
|
||||
let originalStateDir: string | undefined;
|
||||
let tempStateDirs: string[] = [];
|
||||
|
||||
function useTempStateDir(): string {
|
||||
function useTempStateDir(): void {
|
||||
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-acp-sessions-state-"));
|
||||
tempStateDirs.push(stateDir);
|
||||
process.env.OPENCLAW_STATE_DIR = stateDir;
|
||||
return stateDir;
|
||||
}
|
||||
|
||||
function writeAcpRuntimeMeta(sessionKey: string): void {
|
||||
@@ -91,15 +46,6 @@ function writeAcpRuntimeMeta(sessionKey: string): void {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Mock config with a `copilot` agent whose configured model is
|
||||
* `microsoft-foundry/gpt-5.3-codex` (the deployed scenario from the catalog).
|
||||
*
|
||||
* Both the ACP and the non-ACP session entries below leave `model` /
|
||||
* `modelProvider` unset, so `resolveSessionDisplayModelRef` falls through to
|
||||
* the agent's configured default. That is precisely the path under test:
|
||||
* for ACP sessions the agent default is the WRONG answer.
|
||||
*/
|
||||
function mockAgentConfigWithCopilotModel(): void {
|
||||
setMockSessionsConfig(() => ({
|
||||
agents: {
|
||||
@@ -109,43 +55,30 @@ function mockAgentConfigWithCopilotModel(): void {
|
||||
model: { primary: `${AGENT_CONFIGURED_PROVIDER}/${AGENT_CONFIGURED_MODEL}` },
|
||||
},
|
||||
],
|
||||
defaults: {
|
||||
contextTokens: 200_000,
|
||||
},
|
||||
defaults: { contextTokens: 200_000 },
|
||||
},
|
||||
}));
|
||||
}
|
||||
|
||||
/**
|
||||
* ACP bridge session entry: ACP-shaped key but no ACP metadata. The ACP bridge
|
||||
* (translator.ts) uses an in-memory-only session store and never writes
|
||||
* `SessionAcpMeta` to disk. If a bridge client passes an explicit ACP-shaped
|
||||
* key (e.g. `agent:copilot:acp:session-1`) and the Gateway persists the
|
||||
* session, it will have an ACP key without ACP metadata. The overlay must NOT
|
||||
* fire for these sessions — they ran the configured model.
|
||||
*/
|
||||
function buildAcpBridgeSessionEntry(): SessionEntry {
|
||||
return {
|
||||
sessionId: "acp-bridge-session-id",
|
||||
updatedAt: Date.now() - 4 * 60_000,
|
||||
// No `acp` field: this is a bridge session, not a control-plane child session.
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Minimal non-ACP session entry, same shape as the ACP bridge entry. Used as the
|
||||
* GREEN-control case below. The agent default is the correct answer for
|
||||
* non-ACP sessions — those run through the openclaw-agent-driven flow that
|
||||
* actually uses the configured model.
|
||||
*/
|
||||
function buildNonAcpSessionEntry(): SessionEntry {
|
||||
return {
|
||||
sessionId: "non-acp-session-id",
|
||||
updatedAt: Date.now() - 3 * 60_000,
|
||||
};
|
||||
async function readSessionRow(sessionKey: string, store: string) {
|
||||
const payload = await runSessionsJson<{
|
||||
sessions?: Array<{
|
||||
key: string;
|
||||
model?: string | null;
|
||||
modelProvider?: string | null;
|
||||
}>;
|
||||
}>(sessionsCommand, store);
|
||||
return payload.sessions?.find((entry) => entry.key === sessionKey);
|
||||
}
|
||||
|
||||
describe("sessionsCommand model/modelProvider display for ACP sessions (catalog #20)", () => {
|
||||
describe("sessionsCommand ACP model display", () => {
|
||||
beforeEach(() => {
|
||||
originalStateDir = process.env.OPENCLAW_STATE_DIR;
|
||||
mockAgentConfigWithCopilotModel();
|
||||
@@ -165,168 +98,48 @@ describe("sessionsCommand model/modelProvider display for ACP sessions (catalog
|
||||
resetMockSessionsConfig();
|
||||
});
|
||||
|
||||
it("RED: ACP control-plane session must NOT report the agent-configured model", async () => {
|
||||
// RED before fix. The session is a real ACP control-plane session
|
||||
// (key has the `:acp:` segment AND SQLite ACP metadata is present), but
|
||||
// `resolveSessionDisplayModelRef` ignores both and returns the agent
|
||||
// default. Operators relying on `sessions --json` model fields see the
|
||||
// model the openclaw-agent-driven flow would have used, NOT what copilot
|
||||
// actually selected internally when it ran via ACP.
|
||||
//
|
||||
// The discriminator the fix uses: `isAcpSessionKey(row.key)` AND
|
||||
// SQLite ACP metadata (persisted by the ACP control plane manager).
|
||||
it("reports the ACP runtime sentinel for control-plane sessions", async () => {
|
||||
useTempStateDir();
|
||||
writeAcpRuntimeMeta(ACP_SESSION_KEY);
|
||||
const store = await writeStore(
|
||||
{ [ACP_SESSION_KEY]: buildAcpBridgeSessionEntry() },
|
||||
"sessions-acp-model-display-red",
|
||||
"sessions-acp-model-display",
|
||||
{ agentId: "copilot" },
|
||||
);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === ACP_SESSION_KEY);
|
||||
const row = await readSessionRow(ACP_SESSION_KEY, store);
|
||||
|
||||
expect(
|
||||
row,
|
||||
`Expected sessionsCommand --json to include a row for ${ACP_SESSION_KEY}; got none.`,
|
||||
).toBeDefined();
|
||||
expect(
|
||||
row?.model,
|
||||
`ACP session ${ACP_SESSION_KEY} reports model="${row?.model}" — that is the agent-configured ` +
|
||||
`model (${AGENT_CONFIGURED_MODEL}), not what copilot actually used inside ACP. ` +
|
||||
`resolveSessionDisplayModelRef (src/commands/sessions-display-model.ts:123) has zero ` +
|
||||
`ACP-awareness; the call site at src/commands/sessions.ts should consult ` +
|
||||
`isAcpSessionKey(row.key) AND SQLite ACP metadata exists, then overlay an ACP-runtime sentinel.`,
|
||||
).not.toBe(AGENT_CONFIGURED_MODEL);
|
||||
expect(
|
||||
row?.modelProvider,
|
||||
`ACP session ${ACP_SESSION_KEY} reports modelProvider="${row?.modelProvider}" — the ` +
|
||||
`agent-configured provider (${AGENT_CONFIGURED_PROVIDER}), not the ACP runtime. ` +
|
||||
`Same fix site as above; the overlay must gate on persisted ACP metadata.`,
|
||||
).not.toBe(AGENT_CONFIGURED_PROVIDER);
|
||||
expect(row).toMatchObject({ model: "copilot-acp", modelProvider: "acpx" });
|
||||
});
|
||||
|
||||
it("RED (fix-shape): ACP control-plane session should report the ACP runtime sentinel", async () => {
|
||||
// RED before fix; GREEN once the catalog-#20 sentinel-overlay fix lands.
|
||||
//
|
||||
// The catalog's chosen fix shape: when `isAcpSessionKey(row.key)` is true
|
||||
// AND persisted ACP metadata exists, overlay `{ provider: "acpx", model: "<agentId>-acp" }`.
|
||||
// This trades model-name accuracy for "this is ACP control-plane, not the
|
||||
// agent default" clarity. Plumbing the actual copilot-side model selection
|
||||
// into the openclaw record would require capturing ACP `session.model_change`
|
||||
// events (catalog notes this as deferrable).
|
||||
it("reads canonical ACP store keys before querying runtime metadata", async () => {
|
||||
useTempStateDir();
|
||||
writeAcpRuntimeMeta(ACP_SESSION_KEY);
|
||||
const sessionKey = "agent:copilot:acp:binding:discord:default:feedface";
|
||||
const store = await writeStore(
|
||||
{ [ACP_SESSION_KEY]: buildAcpBridgeSessionEntry() },
|
||||
"sessions-acp-model-display-fix-shape",
|
||||
{ agentId: "copilot" },
|
||||
);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === ACP_SESSION_KEY);
|
||||
|
||||
expect(row).toBeDefined();
|
||||
expect(
|
||||
row?.model,
|
||||
`ACP session ${ACP_SESSION_KEY} should resolve model to "copilot-acp" (the catalog-chosen ` +
|
||||
`sentinel). Got "${row?.model}". Fix gates on isAcpSessionKey(row.key) and persisted ACP metadata ` +
|
||||
`and overlays { provider: "acpx", model: "copilot-acp" }. Keeps resolveSessionDisplayModelRef pure.`,
|
||||
).toBe("copilot-acp");
|
||||
expect(
|
||||
row?.modelProvider,
|
||||
`ACP session ${ACP_SESSION_KEY} should resolve modelProvider to "acpx". Got ` +
|
||||
`"${row?.modelProvider}". Same fix as the model assertion above; the overlay sets both ` +
|
||||
`fields together so they remain internally consistent.`,
|
||||
).toBe("acpx");
|
||||
});
|
||||
|
||||
it("reads ACP runtime metadata from SQLite for the display overlay", async () => {
|
||||
useTempStateDir();
|
||||
const store = await writeStore(
|
||||
{ [ACP_SESSION_KEY]: buildAcpBridgeSessionEntry() },
|
||||
"sessions-acp-model-display-sqlite",
|
||||
{ agentId: "copilot" },
|
||||
);
|
||||
writeAcpRuntimeMeta(ACP_SESSION_KEY);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === ACP_SESSION_KEY);
|
||||
|
||||
expect(row).toBeDefined();
|
||||
expect(row?.model).toBe("copilot-acp");
|
||||
expect(row?.modelProvider).toBe("acpx");
|
||||
});
|
||||
|
||||
it("reads canonical ACP store keys before reading SQLite metadata", async () => {
|
||||
useTempStateDir();
|
||||
const canonicalAcpKey = "agent:copilot:acp:binding:discord:default:feedface";
|
||||
const store = await writeStore(
|
||||
{ [canonicalAcpKey]: buildAcpBridgeSessionEntry() },
|
||||
{ [sessionKey]: buildAcpBridgeSessionEntry() },
|
||||
"sessions-acp-model-display-canonical",
|
||||
{ agentId: "copilot" },
|
||||
);
|
||||
writeAcpRuntimeMeta(canonicalAcpKey);
|
||||
writeAcpRuntimeMeta(sessionKey);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === canonicalAcpKey);
|
||||
const row = await readSessionRow(sessionKey, store);
|
||||
|
||||
expect(row).toBeDefined();
|
||||
expect(row?.model).toBe("copilot-acp");
|
||||
expect(row?.modelProvider).toBe("acpx");
|
||||
expect(row).toMatchObject({ model: "copilot-acp", modelProvider: "acpx" });
|
||||
});
|
||||
|
||||
it("GREEN control: ACP bridge session (ACP key, no ACP metadata) reports the configured model", async () => {
|
||||
// ACP bridge sessions (translator.ts) use ACP-shaped keys but never
|
||||
// persist SessionAcpMeta. They run the normal configured model
|
||||
// and must NOT receive the acpx sentinel. This guards against a regression
|
||||
// where key-shape-only detection would misreport bridge sessions.
|
||||
const ACP_BRIDGE_SESSION_KEY = "agent:copilot:acp:bridge-session-1";
|
||||
it("keeps the configured model for ACP-shaped bridge sessions without runtime metadata", async () => {
|
||||
const sessionKey = "agent:copilot:acp:bridge-session-1";
|
||||
const store = await writeStore(
|
||||
{ [ACP_BRIDGE_SESSION_KEY]: buildAcpBridgeSessionEntry() },
|
||||
"sessions-acp-model-display-bridge-control",
|
||||
{ [sessionKey]: buildAcpBridgeSessionEntry() },
|
||||
"sessions-acp-model-display-bridge",
|
||||
{ agentId: "copilot" },
|
||||
);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === ACP_BRIDGE_SESSION_KEY);
|
||||
const row = await readSessionRow(sessionKey, store);
|
||||
|
||||
expect(row).toBeDefined();
|
||||
expect(
|
||||
row?.model,
|
||||
`ACP bridge session ${ACP_BRIDGE_SESSION_KEY} has an ACP-shaped key but no ACP metadata — ` +
|
||||
`it ran the configured model. Got model="${row?.model}"; expected "${AGENT_CONFIGURED_MODEL}". ` +
|
||||
`The overlay must gate on persisted ACP metadata, not key shape alone.`,
|
||||
).toBe(AGENT_CONFIGURED_MODEL);
|
||||
expect(
|
||||
row?.modelProvider,
|
||||
`ACP bridge session ${ACP_BRIDGE_SESSION_KEY} should report the configured provider. ` +
|
||||
`Got "${row?.modelProvider}"; expected "${AGENT_CONFIGURED_PROVIDER}".`,
|
||||
).toBe(AGENT_CONFIGURED_PROVIDER);
|
||||
});
|
||||
|
||||
it("GREEN control: non-ACP session correctly reports the agent-configured model", async () => {
|
||||
// GREEN today. The same agent configuration drives a non-ACP session
|
||||
// (`agent:copilot:main`) — and for that session the agent-configured
|
||||
// model IS the right answer because the openclaw-agent-driven flow
|
||||
// actually runs that model. This control proves:
|
||||
// 1. The test infrastructure is exercising the real resolver path
|
||||
// (not a mock that would silently pass either way).
|
||||
// 2. The configured-model branch of resolveSessionDisplayModelRef
|
||||
// remains correct for non-ACP keys; the proposed sentinel overlay
|
||||
// must NOT break this case (it should only fire when both
|
||||
// isAcpSessionKey(row.key) is true AND ACP metadata is present).
|
||||
const store = await writeStore(
|
||||
{ [NON_ACP_SESSION_KEY]: buildNonAcpSessionEntry() },
|
||||
"sessions-acp-model-display-green-control",
|
||||
{ agentId: "copilot" },
|
||||
);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const row = payload.sessions?.find((entry) => entry.key === NON_ACP_SESSION_KEY);
|
||||
|
||||
expect(row).toBeDefined();
|
||||
expect(row?.model).toBe(AGENT_CONFIGURED_MODEL);
|
||||
expect(row?.modelProvider).toBe(AGENT_CONFIGURED_PROVIDER);
|
||||
expect(row).toMatchObject({
|
||||
model: AGENT_CONFIGURED_MODEL,
|
||||
modelProvider: AGENT_CONFIGURED_PROVIDER,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,93 +1,26 @@
|
||||
// Sessions ACP runtime metadata tests cover agent runtime metadata derived from model and session keys.
|
||||
// Sessions ACP runtime metadata tests cover session-owned runtime overlays.
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolveModelAgentRuntimeMetadata } from "../agents/agent-runtime-metadata.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { parseAgentSessionKey } from "../routing/session-key.js";
|
||||
|
||||
/**
|
||||
* Catalog #18 — `openclaw sessions --json` reports `agentRuntime.id: "openclaw"` for
|
||||
* ACP sessions because the old metadata resolver only consulted agent-config
|
||||
* policies (env / agent / defaults / implicit fallback to "openclaw"). The session
|
||||
* key clearly carries the ACP runtime indicator (the `:acp:` segment), but
|
||||
* `sessions.ts:294` used to ignore it.
|
||||
*
|
||||
* Empirical observation from a deployed openclaw container against a copilot
|
||||
* agent that has no explicit `agentRuntime.id` policy:
|
||||
*
|
||||
* {
|
||||
* "key": "agent:copilot:acp:86b7b5af-3773-4a56-b244-069d6c5d3db9",
|
||||
* "agentId": "copilot",
|
||||
* "agentRuntime": { "id": "openclaw", "source": "implicit" },
|
||||
* "kind": "direct"
|
||||
* }
|
||||
*
|
||||
* That is wrong: this session is plainly ACP, not the native runtime. The runtime field is
|
||||
* supposed to be a faithful classifier of how this session is actually being
|
||||
* run; instead, every ACP session in the JSON output is mislabelled as the native runtime.
|
||||
*
|
||||
* This test mirrors the exact computation `sessionsCommand` performs at
|
||||
* `src/commands/sessions.ts:294` and proves the bug in two parts:
|
||||
*
|
||||
* - RED: ACP-keyed session resolves to `id: "openclaw"`, `source: "implicit"`.
|
||||
* - GREEN control: a non-ACP `agent:main:main` session resolves to the
|
||||
* same implicit-native metadata, which IS correct in that case. The control
|
||||
* proves the assertion infrastructure is not masking the RED case.
|
||||
*
|
||||
* Fix shape (see the third test): when the session key is ACP-style,
|
||||
* agentRuntime.id should report `acpx` (or whatever runtime id is actually
|
||||
* driving the session) so that the JSON faithfully classifies the session.
|
||||
* The fix likely belongs at the caller (sessions.ts:294 and the other
|
||||
* call sites in `src/gateway/server-methods/sessions-*.ts`,
|
||||
* `src/gateway/session-utils.ts`) so it can pass session-key context to
|
||||
* `resolveModelAgentRuntimeMetadata`.
|
||||
*/
|
||||
|
||||
const ACP_SESSION_KEY = "agent:copilot:acp:86b7b5af-3773-4a56-b244-069d6c5d3db9";
|
||||
const NON_ACP_SESSION_KEY = "agent:main:main";
|
||||
|
||||
/**
|
||||
* Build a minimal `OpenClawConfig` that mirrors the deployed scenario:
|
||||
* - a copilot agent exists in the agents.list
|
||||
* - it has NO explicit `agentRuntime.id` policy
|
||||
* - no top-level `agents.defaults.agentRuntime` either
|
||||
*
|
||||
* Result: the old metadata resolver fell through to the implicit "openclaw"
|
||||
* branch — which is the bug under test.
|
||||
*/
|
||||
function buildConfigWithoutAgentRuntimePolicy(): OpenClawConfig {
|
||||
return {
|
||||
agents: {
|
||||
list: [
|
||||
{
|
||||
id: "copilot",
|
||||
// Intentionally no `agentRuntime` field, no `runtime` descriptor.
|
||||
},
|
||||
{
|
||||
id: "main",
|
||||
default: true,
|
||||
},
|
||||
],
|
||||
// No `defaults.agentRuntime` either.
|
||||
list: [{ id: "copilot" }, { id: "main", default: true }],
|
||||
defaults: {},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mirror the per-row computation from `src/commands/sessions.ts:290-298`:
|
||||
* const agentId = parseAgentSessionKey(row.key)?.agentId ?? target.agentId;
|
||||
* const agentRuntime = resolveModelAgentRuntimeMetadata({ cfg, agentId, sessionKey: row.key });
|
||||
*
|
||||
* Returns the same shape that ends up serialized to `--json` output.
|
||||
* After commit 02fe0d8978, the production path goes through resolveModelAgentRuntimeMetadata.
|
||||
*/
|
||||
function computeSessionAgentRuntime(params: {
|
||||
cfg: OpenClawConfig;
|
||||
sessionKey: string;
|
||||
fallbackAgentId: string;
|
||||
/** Mirrors `entry?.acp != null` passed from loaded session rows. */
|
||||
acpRuntime?: boolean;
|
||||
/** Mirrors `entry?.acp?.backend` passed from the session store entry. */
|
||||
acpBackend?: string;
|
||||
}): ReturnType<typeof resolveModelAgentRuntimeMetadata> {
|
||||
const agentId = parseAgentSessionKey(params.sessionKey)?.agentId ?? params.fallbackAgentId;
|
||||
@@ -100,116 +33,33 @@ function computeSessionAgentRuntime(params: {
|
||||
});
|
||||
}
|
||||
|
||||
describe("sessions --json agentRuntime classifier (catalog #18)", () => {
|
||||
it("RED→GREEN: ACP session key is no longer misclassified (overlay applies)", () => {
|
||||
const cfg = buildConfigWithoutAgentRuntimePolicy();
|
||||
describe("session ACP runtime metadata", () => {
|
||||
it("prefers an explicit ACP backend", () => {
|
||||
const agentRuntime = computeSessionAgentRuntime({
|
||||
cfg,
|
||||
sessionKey: ACP_SESSION_KEY,
|
||||
fallbackAgentId: "copilot",
|
||||
acpRuntime: true,
|
||||
});
|
||||
|
||||
// The bug was: the session key plainly contains `:acp:` and yet the
|
||||
// resolved metadata said id="openclaw", source="implicit".
|
||||
// After the fix (applyAcpRuntimeOverlay in resolveModelAgentRuntimeMetadata),
|
||||
// the ACP session key overrides the runtime to id="acpx", source="session-key".
|
||||
expect(
|
||||
agentRuntime.id,
|
||||
`ACP session ${ACP_SESSION_KEY} should no longer be misclassified as "auto" or "openclaw". ` +
|
||||
`Got "${agentRuntime.id}". resolveModelAgentRuntimeMetadata must pass sessionKey to ` +
|
||||
`applyAcpRuntimeOverlay so ACP sessions are classified as "acpx".`,
|
||||
).not.toBe("auto");
|
||||
expect(
|
||||
agentRuntime.source,
|
||||
`ACP session ${ACP_SESSION_KEY} resolved with source="${agentRuntime.source}". ` +
|
||||
`For an ACP-keyed session, the source should not be "implicit" — ` +
|
||||
`the session key itself is an explicit signal that the runtime is ACP.`,
|
||||
).not.toBe("implicit");
|
||||
});
|
||||
|
||||
it("GREEN control: non-ACP session is NOT overridden by ACP overlay", () => {
|
||||
const cfg = buildConfigWithoutAgentRuntimePolicy();
|
||||
const agentRuntime = computeSessionAgentRuntime({
|
||||
cfg,
|
||||
sessionKey: NON_ACP_SESSION_KEY,
|
||||
fallbackAgentId: "main",
|
||||
});
|
||||
|
||||
// For a non-ACP session, the overlay must NOT fire — the result must
|
||||
// not be "acpx" and source must not be "session-key". The control
|
||||
// proves the overlay is gated on the `:acp:` segment in the session key.
|
||||
// (The concrete id — "codex" for the default openai/gpt-5.5 provider —
|
||||
// is determined by resolveAgentHarnessPolicy's Codex-routing rule;
|
||||
// what matters here is the absence of the ACP override.)
|
||||
expect(agentRuntime.id).not.toBe("acpx");
|
||||
expect(agentRuntime.source).not.toBe("session-key");
|
||||
});
|
||||
|
||||
it("FIX-SHAPE expectation: ACP session should resolve to 'acpx'", () => {
|
||||
// What "fixed" should look like once the bug is addressed.
|
||||
// RED today; GREEN once the fix lands.
|
||||
//
|
||||
// Note: the exact id ("acpx" vs another label) is a design choice for
|
||||
// the fix author. What matters is that it is meaningfully different
|
||||
// from "openclaw" and reflects the actual runtime driving the session.
|
||||
// If the fix picks a different label, update this assertion to match —
|
||||
// the structural point (session-key-aware classification) is the
|
||||
// load-bearing part.
|
||||
const cfg = buildConfigWithoutAgentRuntimePolicy();
|
||||
const agentRuntime = computeSessionAgentRuntime({
|
||||
cfg,
|
||||
sessionKey: ACP_SESSION_KEY,
|
||||
fallbackAgentId: "copilot",
|
||||
acpRuntime: true,
|
||||
});
|
||||
|
||||
expect(
|
||||
agentRuntime.id,
|
||||
`ACP session ${ACP_SESSION_KEY} should resolve to runtime id "acpx" (or the canonical ACP runtime label). ` +
|
||||
`Got "${agentRuntime.id}". Fix candidates: ` +
|
||||
`(a) override at the call site in src/commands/sessions.ts:294 once isAcpSessionKey(row.key) is true, or ` +
|
||||
`make resolveModelAgentRuntimeMetadata apply the session-key-aware override centrally.`,
|
||||
).toBe("acpx");
|
||||
});
|
||||
|
||||
it("backend override: ACP session with entry.acp.backend set reports that backend id, NOT 'acpx'", () => {
|
||||
// When the session entry carries an explicit acp.backend (e.g. a registered
|
||||
// non-default backend), the overlay must reflect the actual backend instead
|
||||
// of the generic "acpx" fallback.
|
||||
const cfg = buildConfigWithoutAgentRuntimePolicy();
|
||||
const agentRuntime = computeSessionAgentRuntime({
|
||||
cfg,
|
||||
cfg: buildConfigWithoutAgentRuntimePolicy(),
|
||||
sessionKey: ACP_SESSION_KEY,
|
||||
fallbackAgentId: "copilot",
|
||||
acpRuntime: true,
|
||||
acpBackend: "custom-backend",
|
||||
});
|
||||
|
||||
expect(agentRuntime.id).toBe("custom-backend");
|
||||
expect(agentRuntime.source).toBe("session-key");
|
||||
expect(agentRuntime).toEqual({ id: "custom-backend", source: "session-key" });
|
||||
});
|
||||
|
||||
it("backend fallback: ACP session with entry.acp but no backend falls back to 'acpx'", () => {
|
||||
// When the session entry has ACP metadata but no acp.backend, the overlay
|
||||
// must fall back to the canonical "acpx" id.
|
||||
const cfg = buildConfigWithoutAgentRuntimePolicy();
|
||||
it("falls back to acpx when ACP metadata has no backend", () => {
|
||||
const agentRuntime = computeSessionAgentRuntime({
|
||||
cfg,
|
||||
cfg: buildConfigWithoutAgentRuntimePolicy(),
|
||||
sessionKey: ACP_SESSION_KEY,
|
||||
fallbackAgentId: "copilot",
|
||||
acpRuntime: true,
|
||||
// acpBackend intentionally omitted — mirrors entry with no acp.backend
|
||||
});
|
||||
|
||||
expect(agentRuntime.id).toBe("acpx");
|
||||
expect(agentRuntime.source).toBe("session-key");
|
||||
expect(agentRuntime).toEqual({ id: "acpx", source: "session-key" });
|
||||
});
|
||||
|
||||
it("GREEN control: ACP-shaped bridge session without entry.acp is NOT overridden", () => {
|
||||
const cfg = buildConfigWithoutAgentRuntimePolicy();
|
||||
it("does not overlay ACP-shaped bridge sessions without ACP metadata", () => {
|
||||
const agentRuntime = computeSessionAgentRuntime({
|
||||
cfg,
|
||||
cfg: buildConfigWithoutAgentRuntimePolicy(),
|
||||
sessionKey: ACP_SESSION_KEY,
|
||||
fallbackAgentId: "copilot",
|
||||
acpRuntime: false,
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// Session kind classification tests cover chat, ACP, and agent session metadata classification.
|
||||
// Session kind classification tests cover ACP child session metadata.
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { SessionEntry } from "../config/sessions/types.js";
|
||||
import { normalizeSessionDeliveryState } from "../utils/delivery-context.shared.js";
|
||||
@@ -9,61 +9,13 @@ import {
|
||||
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/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/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:main:acp:7de23a0a-799d-4d63-b1b1-a7de9d4cd840";
|
||||
const ACP_DM_KEY = "agent:main: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",
|
||||
@@ -76,44 +28,10 @@ function buildAcpSpawnChildEntry(): SessionEntry {
|
||||
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,
|
||||
delivery: normalizeSessionDeliveryState({
|
||||
context: {
|
||||
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)", () => {
|
||||
describe("sessionsCommand kind classification", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2025-12-06T00:00:00Z"));
|
||||
@@ -124,97 +42,18 @@ describe("sessionsCommand kind classification (catalog #19)", () => {
|
||||
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`.
|
||||
it("classifies ACP child sessions separately from direct sessions", async () => {
|
||||
const store = await writeStore(
|
||||
{ [ACP_SPAWN_CHILD_KEY]: buildAcpSpawnChildEntry() },
|
||||
"sessions-kind-spawn-child-red",
|
||||
"sessions-kind-spawn-child",
|
||||
);
|
||||
|
||||
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 = await writeStore(
|
||||
{ [ACP_SPAWN_CHILD_KEY]: buildAcpSpawnChildEntry() },
|
||||
"sessions-kind-spawn-child-fix-shape",
|
||||
);
|
||||
|
||||
const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
|
||||
const payload = await runSessionsJson<{
|
||||
sessions?: Array<{ key: string; kind: string }>;
|
||||
}>(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/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 = await 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 = await 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");
|
||||
expect(row?.kind).toBe("spawn-child");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user