fix(sessions): gate ACP model overlay on entry.acp metadata, not key shape alone; add bridge-session regression case and CHANGELOG

efpiva · Galin Iliev · commit 8d33eccdabbf · 2026-05-13T21:16:27.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -105,6 +105,7 @@ Docs: https://docs.openclaw.ai
 - Plugins/runtime: attribute deprecated runtime config load/write warnings to the plugin id and source that triggered them so logs and plugin doctor runs are actionable. Refs #81394. (#81425) Thanks @BKF-Gitty.
 - Memory/LanceDB: make auto-capture recognize short CJK memory phrases and configurable literal triggers, so Chinese, Japanese, and Korean users can capture memories without regex or LLM intent detection. Fixes #75680. Thanks @vyctorbrzezowski and @guokewuming.
 - Plugins doctor: report stale plugin config warnings and avoid claiming full plugin health when config warnings remain. (#81515) Thanks @BKF-Gitty.
+- Sessions: display `model: "<agentId>-acp"` / `modelProvider: "acpx"` (ACP-runtime sentinel) for ACP control-plane sessions in `openclaw sessions` output, instead of the agent's configured model which was misleading. Catalog finding 20. (#79543)
 
 ### Changes
 
diff --git a/src/commands/sessions.acp-model-display.test.ts b/src/commands/sessions.acp-model-display.test.ts
@@ -23,13 +23,17 @@ import {
  * `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.
+ * 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. When `isAcpSessionKey(row.key)` is true, the JSON-emit path overlays
- * `{ provider: "acpx", model: "copilot-acp" }` on top of the resolver result.
- * The resolver itself stays pure (a config-policy resolver); the call site
- * applies the runtime-aware overlay.
+ * site, gated on BOTH key shape AND persisted `entry.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 `entry.acp != null`, 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
@@ -82,19 +86,47 @@ function mockAgentConfigWithCopilotModel(): void {
 }
 
 /**
- * Minimal ACP session entry: just a session id and an updatedAt timestamp.
+ * ACP control-plane session entry: includes `entry.acp` as persisted by
+ * `src/acp/control-plane/manager.core.ts:365` during acpx child init. The
+ * presence of `entry.acp` is the discriminator the overlay uses to distinguish
+ * real ACP child-runtime sessions from ACP bridge sessions.
+ *
  * No `model` / `modelProvider` set on the entry — the listing falls through
  * to the agent's configured default, which is the buggy path for ACP keys.
  */
 function buildAcpSessionEntry(): SessionEntry {
   return {
     sessionId: "acp-session-id",
     updatedAt: Date.now() - 2 * 60_000,
+    acp: {
+      backend: "copilot",
+      agent: "copilot",
+      runtimeSessionName: "acp-runtime-session-1",
+      mode: "persistent",
+      state: "idle",
+      lastActivityAt: Date.now() - 2 * 60_000,
+    },
   };
 }
 
 /**
- * Minimal non-ACP session entry, same shape as the ACP entry. Used as the
+ * ACP bridge session entry: ACP-shaped key but no `entry.acp`. 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 `entry.acp`. 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.
@@ -118,18 +150,16 @@ describe("sessionsCommand model/modelProvider display for ACP sessions (catalog
     vi.useRealTimers();
   });
 
-  it("RED: ACP session must NOT report the agent-configured model", async () => {
-    // RED today. The session is plainly ACP (key has the `:acp:` segment),
-    // but `resolveSessionDisplayModelRef` (`src/commands/sessions-display-model.ts:123`)
-    // ignores the key 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.
+  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 entry.acp 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 should consult is `isAcpSessionKey(row.key)`
-    // (re-exported from `src/routing/session-key.ts:9`, defined at
-    // `src/sessions/session-key-utils.ts:86`). Mirrors catalog #18's overlay
-    // pattern.
+    // The discriminator the fix uses: `isAcpSessionKey(row.key)` AND
+    // `entry.acp != null` (persisted by the ACP control plane manager).
     const store = writeStore(
       { [ACP_SESSION_KEY]: buildAcpSessionEntry() },
       "sessions-acp-model-display-red",
@@ -147,33 +177,26 @@ describe("sessionsCommand model/modelProvider display for ACP sessions (catalog
       `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:335 should consult ` +
-        `isAcpSessionKey(row.key) and overlay an ACP-runtime sentinel before serialization.`,
+        `ACP-awareness; the call site at src/commands/sessions.ts should consult ` +
+        `isAcpSessionKey(row.key) AND entry.acp != null, 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: src/commands/sessions-display-model.ts:123 has no key awareness; ` +
-        `apply the overlay at the call site using isAcpSessionKey.`,
+        `Same fix site as above; the overlay must gate on entry.acp presence.`,
     ).not.toBe(AGENT_CONFIGURED_PROVIDER);
   });
 
-  it("RED (fix-shape): ACP session should report the ACP runtime sentinel", async () => {
-    // RED today; flips GREEN once the catalog-#20 sentinel-overlay fix lands.
+  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 is option (a): when `isAcpSessionKey(row.key)`
-    // is true, overlay `{ provider: "acpx", model: "copilot-acp" }` (or a
-    // similar sentinel). This trades model-name accuracy for "this is ACP,
-    // 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).
-    //
-    // If the fix author chooses different sentinel names ("acp-runtime" vs
-    // "acpx", "copilot-acp" vs "<acp-runtime>"), update both expectations to
-    // match. The structural point is that for ACP-keyed sessions the values
-    // MUST be different from the agent default and clearly mark the ACP
-    // origin.
+    // The catalog's chosen fix shape: when `isAcpSessionKey(row.key)` is true
+    // AND `entry.acp != null`, 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).
     const store = writeStore(
       { [ACP_SESSION_KEY]: buildAcpSessionEntry() },
       "sessions-acp-model-display-fix-shape",
@@ -186,9 +209,8 @@ describe("sessionsCommand model/modelProvider display for ACP sessions (catalog
     expect(
       row?.model,
       `ACP session ${ACP_SESSION_KEY} should resolve model to "copilot-acp" (the catalog-chosen ` +
-        `sentinel). Got "${row?.model}". Fix lands at the call site in src/commands/sessions.ts:335 ` +
-        `which already has row.key in scope — gate on isAcpSessionKey(row.key) and overlay ` +
-        `{ provider: "acpx", model: "copilot-acp" }. Keeps resolveSessionDisplayModelRef pure.`,
+        `sentinel). Got "${row?.model}". Fix gates on isAcpSessionKey(row.key) AND entry.acp != null ` +
+        `and overlays { provider: "acpx", model: "copilot-acp" }. Keeps resolveSessionDisplayModelRef pure.`,
     ).toBe("copilot-acp");
     expect(
       row?.modelProvider,
@@ -198,6 +220,34 @@ describe("sessionsCommand model/modelProvider display for ACP sessions (catalog
     ).toBe("acpx");
   });
 
+  it("GREEN control: ACP bridge session (ACP key, no entry.acp) reports the configured model", async () => {
+    // ACP bridge sessions (translator.ts) use ACP-shaped keys but never
+    // persist SessionAcpMeta to disk. 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";
+    const store = writeStore(
+      { [ACP_BRIDGE_SESSION_KEY]: buildAcpBridgeSessionEntry() },
+      "sessions-acp-model-display-bridge-control",
+    );
+
+    const payload = await runSessionsJson<SessionsJsonPayload>(sessionsCommand, store);
+    const row = payload.sessions?.find((entry) => entry.key === ACP_BRIDGE_SESSION_KEY);
+
+    expect(row).toBeDefined();
+    expect(
+      row?.model,
+      `ACP bridge session ${ACP_BRIDGE_SESSION_KEY} has an ACP-shaped key but no entry.acp — ` +
+        `it ran the configured model. Got model="${row?.model}"; expected "${AGENT_CONFIGURED_MODEL}". ` +
+        `The overlay must gate on entry.acp != null, 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
@@ -207,8 +257,8 @@ describe("sessionsCommand model/modelProvider display for ACP sessions (catalog
     //      (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
-    //      isAcpSessionKey(row.key) is true).
+    //      must NOT break this case (it should only fire when both
+    //      isAcpSessionKey(row.key) is true AND entry.acp is present).
     const store = writeStore(
       { [NON_ACP_SESSION_KEY]: buildNonAcpSessionEntry() },
       "sessions-acp-model-display-green-control",
diff --git a/src/commands/sessions.ts b/src/commands/sessions.ts
@@ -34,6 +34,14 @@ type SessionRow = SessionDisplayRow & {
   kind: "cron" | "direct" | "group" | "global" | "unknown";
   agentRuntime: ReturnType<typeof resolveModelAgentRuntimeMetadata>;
   runtimeLabel: string;
+  /**
+   * True only when the session entry has persisted ACP runtime metadata
+   * (`entry.acp` is present). Key-shape alone is not sufficient because ACP
+   * bridge sessions (translator.ts) may use ACP-shaped keys without ever
+   * writing `SessionAcpMeta` — those use the normal configured model and must
+   * not be overlaid with the acpx sentinel.
+   */
+  acpRuntime: boolean;
 };
 
 const AGENT_PAD = 10;
@@ -49,12 +57,18 @@ const formatKTokens = (value: number) => `${(value / 1000).toFixed(value >= 10_0
 /**
  * Inline ACP model overlay — catalog #20.
  *
- * When a session ran via ACP (e.g. key = `agent:copilot:acp:<uuid>`), the
- * agent's configured model is irrelevant: the actual model is selected inside
- * the ACP child process. We overlay a sentinel `{ provider: "acpx",
+ * When a session ran via the ACP control plane (e.g. key =
+ * `agent:copilot:acp:<uuid>` AND `entry.acp` is present), the agent's
+ * configured model is irrelevant: the actual model is selected inside the ACP
+ * child process. We overlay a sentinel `{ provider: "acpx",
  * model: "<agentId>-acp" }` so the listing clearly signals "ACP runtime" and
  * does not mislead operators into thinking the configured model ran.
  *
+ * Key-shape alone is not sufficient: ACP bridge sessions (translator.ts) also
+ * use ACP-shaped keys but never persist `SessionAcpMeta` — they run the
+ * normal configured model and must not receive the sentinel. The `acpRuntime`
+ * flag is set at row-construction time from `entry.acp != null`.
+ *
  * The resolver (`resolveSessionDisplayModelRef`) stays pure; this overlay
  * applies only at the emit sites in this file.
  *
@@ -64,8 +78,9 @@ const formatKTokens = (value: number) => `${(value / 1000).toFixed(value >= 10_0
 function applyAcpModelOverlayIfNeeded(
   modelRef: { provider: string; model: string },
   sessionKey: string,
+  acpRuntime: boolean,
 ): { provider: string; model: string } {
-  if (!isAcpSessionKey(sessionKey)) {
+  if (!acpRuntime || !isAcpSessionKey(sessionKey)) {
     return modelRef;
   }
   const agentId = parseAgentSessionKey(sessionKey)?.agentId ?? "acp";
@@ -286,9 +301,11 @@ export async function sessionsCommand(
       .map(([key, entry]) => {
         const row = toSessionDisplayRow(key, entry);
         const agentId = parseAgentSessionKey(row.key)?.agentId ?? target.agentId;
+        const acpRuntime = entry?.acp != null;
         const modelRef = applyAcpModelOverlayIfNeeded(
           resolveSessionDisplayModelRef(cfg, row),
           row.key,
+          acpRuntime,
         );
         const agentRuntime = resolveModelAgentRuntimeMetadata({
           cfg,
@@ -299,6 +316,7 @@ export async function sessionsCommand(
         });
         return Object.assign({}, row, {
           agentId,
+          acpRuntime,
           agentRuntime,
           kind: classifySessionKey(row.key, store[row.key]),
           runtimeLabel: resolveSessionRuntimeLabel({
@@ -340,6 +358,7 @@ export async function sessionsCommand(
           const modelRef = applyAcpModelOverlayIfNeeded(
             resolveSessionDisplayModelRef(cfg, r),
             r.key,
+            row.acpRuntime,
           );
           return {
             ...r,
@@ -402,6 +421,7 @@ export async function sessionsCommand(
     const model = applyAcpModelOverlayIfNeeded(
       resolveSessionDisplayModelRef(cfg, row),
       row.key,
+      row.acpRuntime,
     ).model;
     const contextTokens =
       row.contextTokens ??
