Skip to content

@W-22203675 fix: generate session-meta.json when starting agent preview session#199

Merged
franciscoperezsammartino merged 3 commits into
mainfrom
sem/generate-session-meta-json
May 5, 2026
Merged

@W-22203675 fix: generate session-meta.json when starting agent preview session#199
franciscoperezsammartino merged 3 commits into
mainfrom
sem/generate-session-meta-json

Conversation

@sreckomileta
Copy link
Copy Markdown
Contributor

@W-22203675@

What does this PR do?

What issues does this PR fix or reference?

#, @@

Functionality Before

<insert gif and/or summary>

Functionality After

<insert gif and/or summary>

Testing Setup Notes

@franciscoperezsammartino
Copy link
Copy Markdown
Contributor

Got this from claude:

Code Quality

sessionManager.ts

  • Duplication: The exact same 7-line block is copy-pasted in two places — startSession and what appears to be a
    restart path (~line 252). This is a clear candidate for extraction into a private helper like
    writeSessionCache(agentSource, isLiveMode).
  • Type cast smell: sessionType as 'live' | 'simulated' | 'published' — the cast exists because the ternary produces
    a string. You can get rid of it by typing the variable explicitly:
    const sessionType: 'live' | 'simulated' | 'published' =
    agentSource === AgentSource.SCRIPT ? (isLiveMode ? 'live' : 'simulated') : 'published';
  • Comment style: The inline comments (// Write session-meta.json so SF CLI...) are descriptive of what rather than
    why, which goes against the project conventions observed elsewhere. Consider dropping them since the function name
    createPreviewSessionCache is self-documenting.
  • No error handling: If createPreviewSessionCache throws (e.g., filesystem error, missing project path), it will
    bubble up and abort the session start. A try/catch with a logged warning may be safer — failing to write the cache
    file is probably not fatal enough to crash the whole session start.


// Write session-meta.json so SF CLI and other consumers can discover this session
const sessionType = agentSource === AgentSource.SCRIPT ? (isLiveMode ? 'live' : 'simulated') : 'published';
await createPreviewSessionCache(this.state.agentInstance, {
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.

in endSession, should we delete this session-meta.json as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

When you end a session using the "sf agent preview end" command, it is no longer listed when running "sf agent preview sessions"; however, all the files are retained. I believe that if the command deletes the other files, it should also delete this one.

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.

right, the CLI command does delete this. But if you stop the session from VS code, should that delete the session as well?

@franciscoperezsammartino franciscoperezsammartino merged commit 58098af into main May 5, 2026
9 checks passed
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.

2 participants