[eric] canvas: Close says it dismissed the card, because the executor's closed_at cannot mean that (ENG-421)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U6zrBsUCNzpMBnov3rTVYV

backend/apps/agents/agents.py

@@ -377,6 +377,9 @@ async def close_session(session_id: str):
     p_s = agent_manager.sessions.get(session_id)
     if p_s is not None:
         p_s.ended_by_user = True
+        # Only THIS route means "the user put it away". agent_manager.close_session is also called by
+        # the workflow executor for bookkeeping, so the flag belongs at the door, not in the helper.
+        p_s.dismissed_by_user = True
     try:
         await agent_manager.close_session(session_id)
     except ValueError as e:

backend/apps/agents/core/models.py

@@ -100,6 +100,11 @@ class AgentSession(BaseModel):
     branch: Optional[str] = None
     created_at: datetime = Field(default_factory=datetime.now)
     closed_at: Optional[datetime] = None
+    # Set ONLY by the close route, i.e. a person dismissing the card. `closed_at` cannot carry this:
+    # the workflow executor closes the agent session at the end of every step (so the run sorts into
+    # chat history) including one the user stopped, so it means "this run is over", not "put it away".
+    # One field per meaning, or a card the user stopped to READ disappears on them (ENG-421).
+    dismissed_by_user: bool = False
     # Wall-clock of the first stream event so resumed sessions can show "first response at HH:MM" without rescan.
     first_response_at: Optional[datetime] = None
     # HITL approval log: {tool, behavior, decision_ms} per entry.

backend/tests/test_read_agent_work.py

@@ -130,3 +130,21 @@ def test_the_route_reads_a_session_that_is_only_on_DISK():
     assert "resume_session" in body, "a read tool that 404s on a stored session defeats its purpose"
     assert body.index("agent_manager.get_session") < body.index("resume_session"), \
         "memory first, disk second: resuming every read would be a pointless load"
+
+
+def test_only_the_close_route_marks_a_session_dismissed():
+    """ENG-421, found on the packaged build: `closed_at` cannot mean "the user put this away",
+    because the workflow executor stamps it at the end of every step including a stopped one. Stop
+    and Close were therefore indistinguishable and a card the user stopped to READ vanished ~2s
+    later. The flag belongs at the close DOOR, not in agent_manager.close_session, which the
+    executor also calls."""
+    src = open("backend/apps/agents/agents.py", encoding="utf-8").read()
+    i_close = src.index("async def close_session(")
+    i_stop = src.index("async def stop_agent(")
+    close_body = src[i_close:i_close + 700]
+    stop_body = src[i_stop:i_stop + 700]
+    assert "dismissed_by_user = True" in close_body, "Close must say the user put it away"
+    assert "dismissed_by_user" not in stop_body, "Stop must NOT read as a dismissal"
+    helper = open("backend/apps/agents/manager/SessionControl.py", encoding="utf-8").read()
+    assert "dismissed_by_user" not in helper, \
+        "the executor calls this helper for bookkeeping; a flag here would dismiss stopped runs again"

frontend/src/shared/state/deservesCanvasCard.test.ts

@@ -61,10 +61,12 @@ test('a run that ended on its own still despawns', () => {
 });
 
 test('Close still dismisses it, even though Close also sets ended_by_user', () => {
-  // Both routes stamp ended_by_user; only Close stamps closed_at, which is what separates them.
+  // Both routes stamp ended_by_user, and `closed_at` cannot separate them: the workflow executor
+  // stamps it at the end of EVERY step, including a stopped one. `dismissed_by_user` is the fact
+  // that only the close route writes, so it is what tells "put it away" from "this run is over".
   assert.equal(deservesCanvasCard({
     mode: 'agent', workflow_run_id: 'r1', status: 'stopped',
-    ended_by_user: true, closed_at: '2026-08-28T00:00:00Z',
+    ended_by_user: true, closed_at: '2026-08-28T00:00:00Z', dismissed_by_user: true,
   }), false);
 });
 
@@ -80,3 +82,29 @@ test('a live run is unaffected either way', () => {
 test('a user-launched chat never depends on any of this', () => {
   assert.equal(deservesCanvasCard({ mode: 'agent', status: 'stopped' }), true);
 });
+
+test('a user-stopped workflow run keeps its card even though the executor stamped closed_at', () => {
+  // The bug the packaged drill found, and the reason this asserts the FIELD COMBINATION rather than
+  // trusting the rule's prose: the executor closes the session at the end of every step, including a
+  // stopped one, so `closed_at` is always set here. Gating on it made the fix dead code and the card
+  // vanished ~2s after Stop on the shipped build.
+  assert.equal(
+    deservesCanvasCard({
+      workflow_run_id: 'run-1',
+      status: 'stopped',
+      ended_by_user: true,
+      closed_at: '2026-08-28T19:33:47.873352',
+    }),
+    true,
+  );
+});
+
+test('a workflow run that ended ON ITS OWN still despawns, closed_at or not', () => {
+  // The nightly-litter case the original rule exists for. Losing this is how the fix becomes a leak.
+  for (const closed_at of [null, '2026-08-28T19:33:47.873352']) {
+    assert.equal(
+      deservesCanvasCard({ workflow_run_id: 'run-2', status: 'completed', ended_by_user: false, closed_at }),
+      false,
+    );
+  }
+});

frontend/src/shared/state/isUserLaunchedSession.ts

@@ -10,6 +10,7 @@ export interface SessionOrigin {
   ended_by_user?: boolean;
   // Set by Close, never by Stop. It is what tells the two apart here.
   closed_at?: string | null;
+  dismissed_by_user?: boolean;
 }
 
 /** A run that is still working, so its card is worth having on screen. */
@@ -38,7 +39,17 @@ export function isUserLaunchedSession(session: SessionOrigin): boolean {
  * this could not tell them apart, so pressing Stop on a workflow-backed agent made its card vanish
  * along with the transcript the user stopped it to read (ENG-421). Someone stops a run to LOOK at
  * it; the nightly-litter case it was written for is the one that ends by itself, which still
- * despawns. Close is what dismisses a card, and Close is the only route that stamps `closed_at`.
+ * despawns.
+ *
+ * `closed_at` is NOT the release, and assuming it was made this whole rule dead code on the only
+ * path it exists for. The workflow executor closes the agent session at the end of EVERY step,
+ * including one the user stopped, purely so the run sorts into chat history (`get_history` orders by
+ * `closed_at`, and nulls fall off the first page). So `closed_at` means "this run is over", not
+ * "the user dismissed this card", and gating on it deleted the card ~2s after Stop. Caught on the
+ * packaged build; dev never showed it because the drill there stopped a plain agent, not a
+ * workflow-backed one. Dismissal is the layout's job: closing the card removes it from the
+ * dashboard, which is where a user's "I'm done with this" already lives. The backend now says which
+ * of the two happened: `dismissed_by_user` is stamped by the close route alone.
  */
 export function deservesCanvasCard(
   session: SessionOrigin & { status?: string },
@@ -46,5 +57,5 @@ export function deservesCanvasCard(
   if (isUserLaunchedSession(session)) return true;
   if (!session.workflow_run_id) return false;
   if (LIVE_STATUSES.has(session.status ?? '')) return true;
-  return !!session.ended_by_user && !session.closed_at;
+  return !!session.ended_by_user && !session.dismissed_by_user;
 }