Skip to content

[WIP] feat: add --all flag to agent preview end (W-22203669)#406

Open
franciscoperezsammartino wants to merge 20 commits intomainfrom
fp/W-22203669/add-all-flag-to-preview-end
Open

[WIP] feat: add --all flag to agent preview end (W-22203669)#406
franciscoperezsammartino wants to merge 20 commits intomainfrom
fp/W-22203669/add-all-flag-to-preview-end

Conversation

@franciscoperezsammartino
Copy link
Copy Markdown
Contributor

Summary

  • Adds --all flag to agent preview end to terminate multiple sessions at once
  • When used alone: ends all sessions across all agents (client-side only, no org needed)
  • When combined with --api-name or --authoring-bundle: ends only sessions for that specific agent
  • Adds --no-prompt / -p flag to skip confirmation prompt (with --all only)
  • Makes --target-org optional; guard throws when --api-name is used without it
  • Adds structured PreviewEndPartialFailure error listing succeeded/failed sessions on mid-loop failure
  • Restores timestamp/sessionType columns on agent preview sessions (W-22203667 regression from shim revert)
  • Regenerates JSON schemas

Test plan

  • 258 unit tests pass (yarn test)
  • agent preview end --all ends all sessions and shows confirmation prompt
  • agent preview end --all --no-prompt skips confirmation
  • agent preview end --all --authoring-bundle <name> ends only sessions for that agent
  • agent preview end --all --api-name <name> --target-org <org> ends only sessions for that agent
  • agent preview end --all --api-name <name> (no org) throws MissingTargetOrgForApiName
  • agent preview sessions shows timestamp and sessionType columns
  • NUT tests pass for single-session end (both authoring-bundle and api-name paths)

🤖 Generated with Claude Code

- Adds --all to end multiple preview sessions at once
- When combined with --api-name or --authoring-bundle, ends only sessions
  for that specific agent; when used alone, ends all sessions in the project
- Adds --no-prompt (-p) to skip the confirmation prompt shown by --all
- Makes --target-org optional (no org needed for client-side session cleanup);
  raises a clear error when --api-name is used without --target-org
- Restores inline previewSessionStore implementation (the @salesforce/agents
  shim was broken against the installed 1.1.1 version); adds getSessionDir
  and removeCacheById helpers needed by the --all code path
- Adds 13 unit tests for the new end command behaviour

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add try/catch in endAll serial loop with structured PreviewEndPartialFailure
  error listing which sessions failed vs succeeded
- Restore timestamp/sessionType columns on agent preview sessions (W-22203667
  regression introduced by the shim revert in the previous commit)
- Add three missing tests: --all+--api-name happy path, missing --target-org guard,
  and mid-loop failure with partial results assertions
- Document --no-prompt only has effect when used with --all

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

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@franciscoperezsammartino franciscoperezsammartino requested a review from a team as a code owner April 28, 2026 17:37
Comment thread messages/agent.preview.end.md Outdated
Comment thread messages/agent.preview.end.md Outdated
Copy link
Copy Markdown
Contributor

@jshackell-sfdc jshackell-sfdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my small suggestions.

franciscoperezsammartino and others added 15 commits April 28, 2026 14:43
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…only

Replace the bloated inline rewrite with the correct base (commit 57fb5f7,
the last working inline implementation before the broken shim). Only the
two helpers needed by --all are added on top: getSessionDir and
removeCacheById. sessions.ts and start.ts are restored to their main state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
agentId was not in the original result type and is not needed by callers.
Kept as an internal SessionTask type for routing within endAll.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use oclif dependsOn instead of a manual guard in run(). authoring-bundle
does not need target-org since it works client-side only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ScriptAgent/ProductionAgent branching was identical in the single-session
path and the endAll loop. Extracted to a module-level function.

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

removeCache only needs getHistoryDir(), so the no-agent path in --all can
pass a plain object instead of requiring a separate removeCacheById helper.
Loosened removeCache/validatePreviewSession signatures to structural types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…force/agents to 1.2.0

The shim was inadvertently replaced with a full inline implementation.
Restoring it to the re-export shim from main. Bumping agents to 1.2.0
which now exports the session store functions the shim references.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>
Co-authored-by: Juliet Shackell <63259011+jshackell-sfdc@users.noreply.github.com>
Replaces manual inline object types with Pick<CommandFlags, ...> so the
method signatures stay in sync with the flag definitions automatically.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
--all alone was only doing client-side cleanup for ProductionAgent sessions,
which was inconsistent. Now --all must be combined with an agent identifier
so every session is properly ended server-side.

Also simplifies endAll: SessionTask reduced to {sessionId}, tracesPath
resolved via agent.getHistoryDir() inside the loop, prompt updated to
name the specific agent rather than a generic count.

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

Moves the --all + agent identifier constraint from a runtime guard to oclif's
atLeastOne flag metadata. Removing 'all' from all three atLeastOne arrays means
oclif itself rejects --all alone with a single clean error message, rather than
a custom SfError with duplicated validation logic in run().

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

Matches the pattern already used in start.ts. exactlyOne is cleaner and
expresses the intent in a single declaration — exactly one of --api-name
or --authoring-bundle must be provided.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread src/commands/agent/preview/end.ts Outdated
: 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' });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this explicit telemetry call because a similar event will be sent as part of the CLI telemetry framework that sends command execution events; both success and error.

Comment thread src/commands/agent/preview/end.ts Outdated
eventName: 'agent_preview_end_all_partial_failure',
failedCount: failed.length,
succeededCount: ended.length,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this specific event and instead set a different exit code for the command when partial success happens. This allows the CLI framework telemetry events to handle it. @WillieRuemmele - can you suggest a good exit code for this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 4 would work, it's PreviewEndFailed, if we want a more fine-grained approach, we could use 6 as a PreviewEndPartialFailure

Comment thread src/commands/agent/preview/end.ts Outdated
await Lifecycle.getInstance().emitTelemetry({
eventName: 'agent_preview_end_all_success',
sessionCount: ended.length,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This telemetry event is not needed. The CLI framework handles it already.

Comment thread src/commands/agent/preview/end.ts Outdated
await callPreviewEnd(agent);
} catch (error) {
const wrapped = SfError.wrap(error);
await Lifecycle.getInstance().emitTelemetry({ eventName: 'agent_preview_end_failed' });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed. The CLI framework telemetry handles this already.

Comment thread test/nuts/z3.agent.preview.nut.ts Outdated
publishedAgent!.DeveloperName
} --target-org ${targetOrg} --json`
).jsonOutput?.result;
).jsonOutput?.result as import('../../src/commands/agent/preview/end.js').EndedSession | undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't need this type import, you can cast with execCmd<AgentPreviewEndResult>

When --all is used without --api-name or --authoring-bundle, reads all
active sessions from the local cache via listCachedSessions, determines
agent type from the cached sessionType, and ends each session properly
(server-side for ProductionAgent, local for ScriptAgent). --target-org
is required with --all since it may be needed for server-side calls.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@franciscoperezsammartino franciscoperezsammartino changed the title feat: add --all flag to agent preview end (W-22203669) [WIP] feat: add --all flag to agent preview end (W-22203669) Apr 30, 2026
- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants