fix: address PR review feedback and improve --all end-all-agents path

- Remove all explicit Lifecycle.emitTelemetry() calls (CLI framework handles it)
- Change PreviewEndPartialFailure exit code from 4 to 6
- Use sessionType as discriminator for agent type (published vs local)
- Re-set sessionId after ProductionAgent.endSession() clears it before removeCache
- Clean local cache on server error for --all paths to prevent stale buildup
- Show per-agent breakdown with local/published label in confirmation prompt
- Use displayName in prompt breakdown, falling back to agentId

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: franciscoperezsammartino

messages/agent.preview.end.md

@@ -8,7 +8,7 @@ You must have previously started a programmatic agent preview session with the "
 
 The original "agent preview start" command outputs a session ID which you then use with the --session-id flag of this command to end the session. You don't have to specify the --session-id flag if an agent has only one active preview session. You must also use either the --authoring-bundle or --api-name flag to specify the API name of the authoring bundle or the published agent, respectively. To find either API name, navigate to your package directory in your DX project. The API name of an authoring bundle is the same as its directory name under the "aiAuthoringBundles" metadata directory. Similarly, the published agent's API name is the same as its directory name under the "Bots" metadata directory.
 
-Use the --all flag together with either --api-name or --authoring-bundle to end all active preview sessions for a specific agent at once. Alternatively, use --all with only --target-org to end all active preview sessions for every agent found in the local session cache.
+Use the --all flag to end all active preview sessions at once. You can combine --all with --api-name or --authoring-bundle to end only sessions for a specific agent, or use --all on its own to end every session across all agents in the project.
 
 # flags.session-id.summary
 
@@ -62,17 +62,13 @@ No active preview sessions found.
 
 Ended %s preview session(s).
 
-# output.skippingSessionMissingDisplayName
-
-Skipping session '%s' for agent '%s': no agent name found in cache.
-
 # prompt.confirmAll
 
 About to end %s preview session(s) for agent '%s'. Continue?
 
 # prompt.confirmAllAgents
 
-About to end %s preview session(s) across all agents. Continue?
+About to end %s preview session(s) across %s agent(s). Continue?
 
 # examples
 

src/commands/agent/preview/end.ts

@@ -15,7 +15,7 @@
  */
 
 import { Flags, SfCommand, toHelpSection, prompts } from '@salesforce/sf-plugins-core';
-import { Messages, SfError, Lifecycle, EnvironmentVariable } from '@salesforce/core';
+import { Messages, SfError, EnvironmentVariable } from '@salesforce/core';
 import { Agent, ProductionAgent, ScriptAgent } from '@salesforce/agents';
 import type { Connection } from '@salesforce/core';
 import type { Interfaces } from '@oclif/core';
@@ -64,7 +64,7 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
       'Neither --api-name nor --authoring-bundle was provided (required when --all is not set).',
     'NotFound (2)': 'Agent not found, or no preview session exists for this agent.',
     'PreviewEndFailed (4)': 'Failed to end the preview session.',
-    'PreviewEndPartialFailure (4)': 'With --all, one or more sessions failed to end while others succeeded.',
+    'PreviewEndPartialFailure (6)': 'With --all, one or more sessions failed to end while others succeeded.',
     'SessionAmbiguous (5)': 'Multiple preview sessions found; specify --session-id to choose one.',
   });
 
@@ -123,11 +123,9 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
     if (sessionId === undefined) {
       const cached = await getCachedSessionIds(this.project!, agent);
       if (cached.length === 0) {
-        await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_no_session' });
         throw new SfError(messages.getMessage('error.noSession'), 'PreviewSessionNotFound', [], 2);
       }
       if (cached.length > 1) {
-        await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_multiple_sessions' });
         throw new SfError(
           messages.getMessage('error.multipleSessions', [cached.join(', ')]),
           'PreviewSessionAmbiguous',
@@ -144,7 +142,6 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
       await validatePreviewSession(agent);
     } catch (error) {
       const wrapped = SfError.wrap(error);
-      await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_session_invalid' });
       throw new SfError(
         messages.getMessage('error.sessionInvalid', [sessionId]),
         'PreviewSessionInvalid',
@@ -161,7 +158,6 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
       await callPreviewEnd(agent);
     } catch (error) {
       const wrapped = SfError.wrap(error);
-      await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_failed' });
       throw new SfError(
         messages.getMessage('error.endFailed', [wrapped.message]),
         'PreviewEndFailed',
@@ -171,7 +167,6 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
       );
     }
 
-    await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_success' });
     const result: EndedSession = { sessionId, tracesPath };
     this.log(messages.getMessage('output.tracesPath', [tracesPath]));
     return result;
@@ -195,7 +190,6 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
         : await Agent.init({ connection: conn as Connection, project: this.project!, apiNameOrId: flags['api-name']! });
     } catch (error) {
       const wrapped = SfError.wrap(error);
-      await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_agent_not_found' });
       throw new SfError(messages.getMessage('error.agentNotFound', [agentIdentifier]), 'AgentNotFound', [], 2, wrapped);
     }
   }
@@ -241,30 +235,39 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
       }
     }
 
-    const { ended, failed } = await endSessionsForAgent(agent, sessionsToEnd);
+    const { ended, failed } = await endSessionsForAgent(agent, sessionsToEnd, true);
     return this.finishEndAll(ended, failed);
   }
 
   /**
    * Path 2: --all alone (no agent identifier).
    * Reads all agents from the local cache via listCachedSessions and ends every session.
-   * Missing displayName: warns and skips the entry (no sessionId context for failed list).
-   * Missing sessionType: defaults to ScriptAgent (safest — local-only cleanup).
+   * sessionType 'published' → ProductionAgent (server-side DELETE). 'simulated'/'live' → ScriptAgent (local only).
+   * session-meta.json is always present for entries returned by listCachedSessions, so sessionType is always defined.
    */
   private async endAllAgents(conn: Connection, noPrompt: boolean | undefined): Promise<{ ended: EndedSession[] }> {
     const entries: CachedPreviewSessionEntry[] = await listCachedSessions(this.project!);
 
-    // Only count sessions for entries that have a displayName; entries without one will be skipped.
-    const totalSessions = entries.reduce((sum, e) => (e.displayName ? sum + e.sessions.length : sum), 0);
+    const totalSessions = entries.reduce((sum, e) => sum + e.sessions.length, 0);
 
     if (totalSessions === 0) {
       this.log(messages.getMessage('output.noSessionsFound'));
       return { ended: [] };
     }
 
     if (!noPrompt) {
+      const agentBreakdown = entries
+        .map((e) => {
+          const label = e.displayName ?? e.agentId;
+          const type = e.sessions[0]?.sessionType === 'published' ? 'published' : 'local';
+          return `  - ${label} (${type}): ${e.sessions.length} session(s)`;
+        })
+        .join('\n');
       const confirmed = await prompts.confirm({
-        message: messages.getMessage('prompt.confirmAllAgents', [totalSessions]),
+        message: `${messages.getMessage('prompt.confirmAllAgents', [
+          totalSessions,
+          entries.length,
+        ])}\n${agentBreakdown}`,
       });
       if (!confirmed) {
         return { ended: [] };
@@ -275,29 +278,17 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
     const failed: Array<{ task: SessionTask; error: string }> = [];
 
     for (const entry of entries) {
-      const { agentId, displayName, sessions } = entry;
-
-      if (!displayName) {
-        // Cannot init an agent without a name. Log a warning per session and skip — we have no
-        // sessionId context to surface in the failed list, so we skip rather than fail.
-        for (const s of sessions) {
-          this.warn(messages.getMessage('output.skippingSessionMissingDisplayName', [s.sessionId, agentId]));
-        }
-        continue;
-      }
+      const { agentId, sessions } = entry;
 
       let agent: ScriptAgent | ProductionAgent;
       try {
-        // Determine agent type from any session's sessionType (all sessions for an agent share the same type).
-        // Default to ScriptAgent when sessionType is missing — local-only cleanup is safest.
-        const sessionType = sessions[0]?.sessionType ?? 'simulated';
-        const isProduction = sessionType === 'published';
+        const isProduction = sessions[0]?.sessionType === 'published';
         if (isProduction) {
           // eslint-disable-next-line no-await-in-loop
-          agent = await Agent.init({ connection: conn, project: this.project!, apiNameOrId: displayName });
+          agent = await Agent.init({ connection: conn, project: this.project!, apiNameOrId: agentId });
         } else {
           // eslint-disable-next-line no-await-in-loop
-          agent = await Agent.init({ connection: conn, project: this.project!, aabName: displayName });
+          agent = await Agent.init({ connection: conn, project: this.project!, aabName: agentId });
         }
       } catch (error) {
         // If we can't init the agent, mark all its sessions as failed.
@@ -311,7 +302,8 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
       // eslint-disable-next-line no-await-in-loop
       const { ended: agentEnded, failed: agentFailed } = await endSessionsForAgent(
         agent,
-        sessions.map((s) => ({ sessionId: s.sessionId }))
+        sessions.map((s) => ({ sessionId: s.sessionId })),
+        true
       );
       ended.push(...agentEnded);
       failed.push(...agentFailed);
@@ -324,28 +316,19 @@ export default class AgentPreviewEnd extends SfCommand<AgentPreviewEndResult> {
    * Called by endAllForAgent when we have a single agent and already aggregated ended/failed.
    * Throws a partial-failure error if needed; logs success otherwise.
    */
-  private async finishEndAll(
+  private finishEndAll(
     ended: EndedSession[],
     failed: Array<{ task: SessionTask; error: string }>
-  ): Promise<{ ended: EndedSession[] }> {
+  ): { ended: EndedSession[] } {
     if (failed.length > 0) {
       const failedList = failed.map((f) => `${f.task.sessionId}: ${f.error}`).join(', ');
       const endedIds = ended.map((e) => e.sessionId).join(', ');
       const msg = `Failed to end ${failed.length} session(s): [${failedList}]. Successfully ended ${
         ended.length
       } session(s)${ended.length > 0 ? `: [${endedIds}]` : ''}.`;
-      await Lifecycle.getInstance().emitTelemetry({
-        eventName: 'agent_preview_end_all_partial_failure',
-        failedCount: failed.length,
-        succeededCount: ended.length,
-      });
-      throw new SfError(msg, 'PreviewEndPartialFailure', [], 4);
+      throw new SfError(msg, 'PreviewEndPartialFailure', [], 6);
     }
 
-    await Lifecycle.getInstance().emitTelemetry({
-      eventName: 'agent_preview_end_all_success',
-      sessionCount: ended.length,
-    });
     this.log(messages.getMessage('output.endedAll', [ended.length]));
     return { ended };
   }
@@ -357,10 +340,13 @@ type CommandFlags = Interfaces.InferredFlags<typeof AgentPreviewEnd.flags>;
  * Ends a list of sessions on the given agent object serially.
  * Returns { ended, failed } so callers can aggregate results.
  * Does NOT throw on partial failure — callers decide what to do.
+ * When cleanOnServerError is true, a server-side failure still removes the local cache entry
+ * so stale sessions don't accumulate (used by --all paths).
  */
 async function endSessionsForAgent(
   agent: ScriptAgent | ProductionAgent,
-  sessionsToEnd: SessionTask[]
+  sessionsToEnd: SessionTask[],
+  cleanOnServerError = false
 ): Promise<{ ended: EndedSession[]; failed: Array<{ task: SessionTask; error: string }> }> {
   const ended: EndedSession[] = [];
   const failed: Array<{ task: SessionTask; error: string }> = [];
@@ -376,10 +362,20 @@ async function endSessionsForAgent(
       // ScriptAgent flushes traces to disk; ProductionAgent issues the server-side end request.
       // eslint-disable-next-line no-await-in-loop
       await callPreviewEnd(agent);
+      // ProductionAgent.endSession() clears this.sessionId after the server call; re-set it so
+      // removeCache can call getHistoryDir() without throwing "No sessionId set on agent".
+      agent.setSessionId(sessionId);
       // eslint-disable-next-line no-await-in-loop
       await removeCache(agent);
       ended.push({ sessionId, tracesPath });
     } catch (error) {
+      if (cleanOnServerError) {
+        // Server doesn't know about this session (e.g. already expired). Remove local cache so
+        // it doesn't show up in future listings.
+        agent.setSessionId(sessionId);
+        // eslint-disable-next-line no-await-in-loop
+        await removeCache(agent).catch(() => {});
+      }
       failed.push({ task, error: SfError.wrap(error).message });
     }
   }

test/commands/agent/preview/end.test.ts

@@ -283,8 +283,8 @@ describe('agent preview end', () => {
         expect(err.message).to.include('network timeout');
         // Also mentions the ones that succeeded
         expect(err.message).to.include('Successfully ended 2 session(s)');
-        // Only 2 successful removes happened (session-1 and session-3)
-        expect(removeCacheStub.callCount).to.equal(2);
+        // 3 removes: session-1 (success), session-2 (catch cleanup), session-3 (success)
+        expect(removeCacheStub.callCount).to.equal(3);
         expect(err.name).to.equal('PreviewEndPartialFailure');
       }
     });
@@ -308,7 +308,8 @@ describe('agent preview end', () => {
         expect(err.message).to.include('session-2');
         expect(err.message).to.include('stale session');
         expect(err.message).to.include('Successfully ended 1 session(s)');
-        expect(removeCacheStub.callCount).to.equal(1);
+        // 2 removes: session-1 (success) + session-2 (catch cleanup)
+        expect(removeCacheStub.callCount).to.equal(2);
         expect(agentPreviewEndStub.callCount).to.equal(1);
         expect(err.name).to.equal('PreviewEndPartialFailure');
       }
@@ -363,16 +364,14 @@ describe('agent preview end', () => {
     it('ends sessions for all agents from listCachedSessions when only --target-org is provided', async () => {
       listCachedSessionsStub.resolves([
         {
-          agentId: 'agent-1-id',
-          displayName: 'My_Script_Agent',
+          agentId: 'My_Script_Agent',
           sessions: [
             { sessionId: 'sess-1', sessionType: 'simulated' },
             { sessionId: 'sess-2', sessionType: 'live' },
           ],
         },
         {
-          agentId: 'agent-2-id',
-          displayName: 'My_Published_Agent',
+          agentId: '0Xxg8000000NBNlCAO',
           sessions: [{ sessionId: 'sess-3', sessionType: 'published' }],
         },
       ]);
@@ -404,70 +403,39 @@ describe('agent preview end', () => {
       }
     });
 
-    it('skips agents with missing displayName and logs a warning', async () => {
+    it('uses aabName (ScriptAgent) for live sessionType', async () => {
       listCachedSessionsStub.resolves([
         {
-          agentId: 'agent-no-name',
-          displayName: undefined,
-          sessions: [{ sessionId: 'orphan-sess-1', sessionType: 'simulated' }],
-        },
-        {
-          agentId: 'agent-with-name',
-          displayName: 'My_Script_Agent',
-          sessions: [{ sessionId: 'known-sess-1', sessionType: 'simulated' }],
+          agentId: 'Local_Info_Agent',
+          sessions: [{ sessionId: 'aab-sess-1', sessionType: 'live' }],
         },
       ]);
 
-      const warnStub = $$.SANDBOX.stub(AgentPreviewEnd.prototype, 'warn');
-
       const result = await AgentPreviewEnd.run(['--all', '--target-org', 'test@org.com', '--no-prompt']);
 
-      // Only the named agent's session should have been ended
       expect((result as { ended: unknown[] }).ended).to.have.length(1);
-      expect(initStub.callCount).to.equal(1);
-      expect(removeCacheStub.callCount).to.equal(1);
-      // A warning should have been emitted for the session with the missing displayName
-      expect(warnStub.calledOnce).to.be.true;
+      expect(initStub.calledOnce).to.be.true;
+      expect(initStub.firstCall.args[0]).to.have.property('aabName', 'Local_Info_Agent');
+      expect(removeCacheStub.calledOnce).to.be.true;
     });
 
-    it('uses aabName init path for simulated/live sessions and apiNameOrId for published', async () => {
+    it('uses aabName for simulated/live sessions and apiNameOrId for published', async () => {
       listCachedSessionsStub.resolves([
         {
-          agentId: 'script-agent-id',
-          displayName: 'My_Script_Agent',
+          agentId: 'My_Script_Agent',
           sessions: [{ sessionId: 'sess-sim', sessionType: 'simulated' }],
         },
         {
-          agentId: 'prod-agent-id',
-          displayName: 'My_Published_Agent',
+          agentId: 'Weather_Agent',
           sessions: [{ sessionId: 'sess-pub', sessionType: 'published' }],
         },
       ]);
 
       await AgentPreviewEnd.run(['--all', '--target-org', 'test@org.com', '--no-prompt']);
 
       expect(initStub.callCount).to.equal(2);
-      // First call should use aabName for simulated agent
       expect(initStub.firstCall.args[0]).to.have.property('aabName', 'My_Script_Agent');
-      // Second call should use apiNameOrId for published agent
-      expect(initStub.secondCall.args[0]).to.have.property('apiNameOrId', 'My_Published_Agent');
-    });
-
-    it('defaults to aabName (ScriptAgent) when sessionType is undefined', async () => {
-      listCachedSessionsStub.resolves([
-        {
-          agentId: 'agent-no-type',
-          displayName: 'My_Unknown_Agent',
-          sessions: [{ sessionId: 'sess-no-type', sessionType: undefined }],
-        },
-      ]);
-
-      await AgentPreviewEnd.run(['--all', '--target-org', 'test@org.com', '--no-prompt']);
-
-      expect(initStub.calledOnce).to.be.true;
-      // sessionType undefined → defaults to 'simulated' → should use aabName path
-      expect(initStub.firstCall.args[0]).to.have.property('aabName', 'My_Unknown_Agent');
-      expect(initStub.firstCall.args[0]).to.not.have.property('apiNameOrId');
+      expect(initStub.secondCall.args[0]).to.have.property('apiNameOrId', 'Weather_Agent');
     });
 
     it('throws PreviewEndPartialFailure when one agent succeeds and another throws (no agent identifier)', async () => {
@@ -477,13 +445,11 @@ describe('agent preview end', () => {
 
       listCachedSessionsStub.resolves([
         {
-          agentId: 'agent-a-id',
-          displayName: 'Agent_A',
+          agentId: 'Agent_A',
           sessions: [{ sessionId: 'sess-a-1', sessionType: 'simulated' }],
         },
         {
-          agentId: 'agent-b-id',
-          displayName: 'Agent_B',
+          agentId: 'Agent_B',
           sessions: [{ sessionId: 'sess-b-1', sessionType: 'simulated' }],
         },
       ]);

test/nuts/z3.agent.preview.nut.ts

@@ -154,7 +154,7 @@ describe('agent preview', function () {
         `agent preview end --session-id ${sessionId} --api-name ${
           publishedAgent!.DeveloperName
         } --target-org ${targetOrg} --json`
-      ).jsonOutput?.result as import('../../src/commands/agent/preview/end.js').EndedSession | undefined;
+      ).jsonOutput?.result as { sessionId?: string; tracesPath?: string } | undefined;
       expect(endResult?.sessionId).to.equal(sessionId);
       expect(endResult?.tracesPath).to.be.a('string');
     });