Note [CRF-17] The created pubsub event is published (line 1459) before hydrateChatContextOnCreate runs. The event payload carries the pre-hydration chat struct, so Context is nil. Watch subscribers see a chat created with no context and have no follow-up event for the hydration. The HTTP handler re-reads after CreateChat returns, so the creator sees the hydrated state. Worth noting for the UI phase where watch events may drive real-time context badges. (Mafuuu, Meruem)

🤖

Acknowledged — deferring to the UI phase as the Note suggests. No watcher consumes Context yet, and the HTTP creator re-reads after CreateChat, so the creator sees the hydrated state. When watch events drive context badges we'll reorder (or emit a follow-up event).

— 🤖 Coder Agents, on behalf of @kylecarbs

Note [CRF-3] RefreshChatContext does not emit a watch event when it clears the dirty marker. HydrateAndMarkChatsDirty publishes ChatWatchEventKindContextDirty when a chat becomes dirty, but no corresponding event signals the transition back to clean. The initiating HTTP client sees the cleared state in the 200 response, but other watchers continue to see dirty until they re-fetch. (Netero)

🤖

Acknowledged — deferring. There is no "context clean" watch event kind or consumer yet; the initiating client sees the cleared state in the 200 response. The UI phase will define the clean-transition event for other watchers.

— 🤖 Coder Agents, on behalf of @kylecarbs

Note [CRF-16] HydrateAgentChatsContext filters only by archived = false, while MarkChatsContextDirtyByAgent also filters by active statuses (waiting, running, paused, pending, requires_action). Terminal chats (completed, errored) receive a pinned hash, causing db2sdk.Chat to emit a non-nil Context sub-struct for finished chats. Harmless today, but the UI may later key context badge visibility off Context != nil. (Chopper)

🤖

Acknowledged — harmless today as noted. HydrateAgentChatsContext lives in the merged foundation migration (#26385); gating it by status to match the dirty query is a reasonable follow-up there rather than widening this PR's scope.

— 🤖 Coder Agents, on behalf of @kylecarbs

P3 [CRF-18] The publishDirty closure captures the RPC-derived ctx and uses it for p.db.GetChatByID calls after the transaction commits. If the agent's gRPC stream dies between commit and callback invocation, context cancellation silently drops every dirty pubsub notification. Database state is correct; watchers never learn about the drift.

The rest of chatd.go uses context.WithoutCancel(ctx) for exactly this class of post-commit fire-and-forget work (8 call sites: lines 2371, 2505, 2635, 2665, 2765, 5110, 5207, 5220). The fix is one line: detach ctx before the closure or inside it. (Hisoka)

🤖

P3 [CRF-19] The publishDirty callback re-reads each dirtied chat from p.db after commit. Between commit and re-read, a concurrent RefreshChatContext can clear context_dirty_since. The callback then publishes a context_dirty event whose Chat.Context.Dirty == false. Event kind says "dirty"; payload says "clean."

The window is narrow (callback runs synchronously after commit), but structurally real. No consumer exists today, but when the UI phase wires watchers to context badges, they need to decide whether to trust the event payload or always re-fetch on context_dirty.

Capturing the chat data needed for the event payload inside the transaction (e.g., via RETURNING * or a SELECT inside the TX) would close both this and CRF-18, since the callback would not need a DB read at all. (Hisoka P3, Chopper Note)

🤖

P3 [CRF-20] After push #2 marks the chat dirty, the test asserts Dirty and DirtySince but not Context.Error. MarkChatsContextDirtyByAgent sets only context_dirty_since; the pinned error should be unchanged (empty, from push #1). Adding require.Empty(t, got.Context.Error, "dirty chat's error reflects the original pin") verifies the design invariant that dirtied chats' pinned error is not overwritten by the dirty fan-out.

The complementary assertion after refresh (require.Equal(t, snapshotError, refreshed.Context.Error)) already exists. The dirty-step negative case is what distinguishes "error unchanged" from "error updated." (Chopper)

🤖

Nit [CRF-21] Two of the three doc sentences restate the signature. Sentence 1 ("returns the agent's most recent pinned context snapshot") restates the function name. Sentence 3 ("It takes a Store so callers can run it...") restates the db database.Store parameter. Only sentence 2 (the ok/nil-error contract) carries information the code does not show.

Trim to: // ok is false (nil error) when no snapshot exists. (Gon)

🤖

Nit [CRF-22] "stamps via the NULL-guarded HydrateAgentChatsContext" leaks the SQL WHERE context_aggregate_hash IS NULL clause into the Go doc. The invariant (concurrent push that already hydrated the chat is not clobbered) is valuable; the SQL mechanism is not.

Trim to: // A concurrent push that already hydrated the chat is not clobbered. (Gon)

🤖

Nit [CRF-23] The fix commit subject (fix(chatd): address deep-review feedback on chat context) names the process, not the fix. If not squash-merged, consider: fix(chatd): close TOCTOU in context refresh, tighten create-time hydration. (Leorio)

🤖