Skip to content

fix(staged): emit session running event from backend and remove redundant frontend registration - #645

Merged
matt2e merged 5 commits into
mainfrom
session-from-diff
Apr 22, 2026
Merged

fix(staged): emit session running event from backend and remove redundant frontend registration#645
matt2e merged 5 commits into
mainfrom
session-from-diff

Conversation

@matt2e

@matt2e matt2e commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Emit a session-status-changed "running" event from start_branch_session in the Rust backend so the frontend state stores (project list spinner, unread badges, etc.) are updated regardless of which UI surface starts the session
  • Remove the now-redundant registerRunningSession call from BranchCardSessionManager on the frontend, since the backend event handles it

Test plan

  • Start a branch session from different UI surfaces and verify the project list spinner and unread badges update correctly
  • Verify no duplicate session registration occurs
  • Confirm existing session flows (commit, note, review) still work end-to-end

🤖 Generated with Claude Code

matt2e and others added 2 commits April 22, 2026 12:04
…session tracking

Sessions started from the diff viewer were not tracked in the project
list/sidebar because start_branch_session did not emit a "running"
session-status-changed event with projectId/branchId. The
BranchCardSessionManager worked around this by manually registering
sessions after the call, but DiffCommitSessionLauncher did not,
causing missing spinners and unread badges.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…startBranchSession

The global sessionStatusListener now handles session registration for
non-auto-review sessions via the "running" event emitted by
start_branch_session (added in b9d627b). The manual call in
startSession() was a workaround that is no longer needed.

The registerRunningSession method and its auto-review adoption call
sites remain, since the global listener intentionally skips auto-review
sessions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt2e
matt2e requested review from baxen and wesbillman as code owners April 22, 2026 03:11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15ac851277

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +752 to +756
let _ = app_handle.emit(
"session-status-changed",
session_runner::SessionStatusEvent {
session_id: session.id.clone(),
status: "running".to_string(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Emit running status only after session startup succeeds

This emits session-status-changed with status: "running" before several fallible steps in start_branch_session (artifact creation, git::get_head_sha, and session_runner::start_session all use ? and can still return an error). In those failure paths, the frontend now receives a running event but no matching terminal event, so projectStateStore/sessionRegistry keep a stale running session (spinner/unread state can remain incorrect) even though the command failed and UI shows an error. Previously BranchCard only registered running sessions after a successful command result, so this regression is introduced by the new early emit.

Useful? React with 👍 / 👎.

matt2e and others added 3 commits April 22, 2026 13:34
…sion

Replace inline SessionStatusEvent construction with the existing
session_runner::emit_session_running() helper, which builds the same
event and also logs a warning on emit failure (vs silently discarding
with `let _ =`).

Resolves DRY review comment from 7d10b24f.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deleting a pending commit, note, or review with an active session left
the sidebar showing "running" forever. The DB row was deleted before the
session runner finished, so transition_from_running updated 0 rows, the
terminal event was never emitted, and projectStateStore was never
cleaned up.

Frontend (primary fix): delete handlers now call cleanupSessionState()
after cancelSession to immediately remove the session from
projectStateStore and sessionRegistry.

Backend (safety net): session runner now emits the terminal status event
unconditionally, even when transition_from_running returns false. The
frontend's handleSessionEnd already handles unknown sessions gracefully.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…il after delete

Resolve review comments from e629974e on 549beee:

1. Extract shared cleanupSession helper to sessionRegistry
   The cleanup logic (removeRunningSession + unregister) was duplicated
   between BranchCard.svelte's local cleanupSessionState function and
   sessionStatusListener.ts's handleSessionEnd. Now both call
   sessionRegistry.cleanupSession(), which is the symmetric counterpart
   to register() — keeping the cleanup contract in one place.

2. Move cleanup calls after delete operations succeed
   Previously, cleanupSessionState ran before deleteNote/deleteReview/
   deletePendingCommit. If the delete threw, the session state was
   already cleaned from frontend stores while the entity still existed
   in the DB. Now cleanup runs after the delete succeeds, with the
   backend's unconditional terminal event as the safety net for the
   interim period between cancelSession and delete completion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt2e
matt2e merged commit f761b9b into main Apr 22, 2026
5 checks passed
@matt2e
matt2e deleted the session-from-diff branch April 22, 2026 05:28
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.

1 participant