fix: target specific chat in MarkStale instead of broadcasting to all workspace chats by kylecarbs · Pull Request #23883 · coder/coder

kylecarbs · 2026-03-31T23:18:47Z

Problem

Subagent chats were receiving git context (branch, remote origin, PR
status) from their parent or sibling chats' git operations. When a git
operation triggers external auth, the workspace agent sends chat_id
identifying which chat initiated it — but this was broken at two levels:

Agent side: CODER_CHAT_ID was never injected into process
environments. chatd sets Coder-Chat-Id HTTP headers and the
agent extracts them for process isolation, but never propagated
CODER_CHAT_ID to cmd.Env. So gitaskpass always sent an empty
chat_id.
Server side: workspaceAgentsExternalAuth ignored the chat_id
query param. MarkStale broadcast git context to all chats on
the workspace via filterChatsByWorkspaceID.

Fix

Inject CODER_CHAT_ID into cmd.Env in agentproc when the chat
ID is known, so gitaskpass can read and forward it.
Read chat_id from query params in workspaceAgentsExternalAuth
and thread it through chatGitRef.
Refactor MarkStale to accept a MarkStaleParams struct. When
ChatID is provided, target only that specific chat. When empty
(legacy agents, non-chat git operations), fall back to the existing
workspace-wide broadcast.
Extract markStaleSingle helper to deduplicate the upsert+publish
logic.

Investigation notes

Data flow before fix

chatd → sets Coder-Chat-Id header on agent conn
agent → extracts chatID, stores on process struct
agent → does NOT set CODER_CHAT_ID in cmd.Env  ← gap 1
gitaskpass → reads CODER_CHAT_ID (always empty), sends chat_id=""
server handler → ignores chat_id query param     ← gap 2
MarkStale → broadcasts to ALL workspace chats

Data flow after fix

chatd → sets Coder-Chat-Id header on agent conn
agent → extracts chatID, stores on process struct
agent → sets CODER_CHAT_ID in cmd.Env
gitaskpass → reads CODER_CHAT_ID, sends chat_id=<uuid>
server handler → reads chat_id, passes to MarkStale
MarkStale → targets only that specific chat

… workspace chats When a git operation triggers external auth, the agent sends a chat_id identifying which chat initiated the operation. Two gaps caused every chat on the workspace to receive the git context: 1. CODER_CHAT_ID was never injected into process environments, so gitaskpass always sent an empty chat_id. 2. The server handler ignored the chat_id query param and MarkStale broadcast to all workspace chats via filterChatsByWorkspaceID. This commit: - Injects CODER_CHAT_ID into cmd.Env in agentproc so gitaskpass can read and forward it. - Reads chat_id from query params in workspaceAgentsExternalAuth and threads it through chatGitRef. - Refactors MarkStale to accept a MarkStaleParams struct. When ChatID is provided, targets only that chat; otherwise falls back to the existing workspace-wide broadcast.

johnstcn · 2026-04-01T11:54:13Z

🤖

Clean fix — the data flow is now complete end-to-end and the MarkStaleParams refactor is a strict improvement. Two findings from a deep review, both validated with passing tests:

P2 Missing test for invalid ChatID fallback path (worker.go:300-310, worker_test.go)

When uuid.Parse(p.ChatID) fails, the code falls back to workspace-wide broadcast with a warning log. This fallback path is not tested. The existing TestWorker_MarkStale_WithChatID only covers the happy path (valid UUID → markStaleSingle). A test passing ChatID: "not-a-uuid" that asserts GetChats IS called (broadcast activates) and UpsertChatDiffStatusReference fires for workspace-filtered chats would close this gap.

Validated: wrote TestWorker_MarkStale_WithInvalidChatID — passes, confirming the fallback works correctly but has no coverage today. If someone later changes the error handling (e.g. returns early instead of falling through), there'd be no test to catch the regression.

P3 Targeted path skips workspace ownership check (worker.go:306-308)

When ChatID is valid, markStaleSingle is called directly — WorkspaceID is never consulted. The broadcast path uses filterChatsByWorkspaceID for ownership validation; the targeted path does not.

Validated: wrote TestWorker_MarkStale_WithChatID_IgnoresWorkspaceID — passes a targetChat with a completely unrelated WorkspaceID. GetChats is never called, and the chat is targeted anyway.

Mitigated by the data flow (CODER_CHAT_ID comes from chatd via the agent), so this is P3. Worth documenting or addressing in a follow-up — the asymmetry between the two paths is surprising.

johnstcn · 2026-04-01T12:04:52Z

+	Origin      string
+	// ChatID, when non-empty, targets a single chat instead
+	// of broadcasting to every chat on the workspace.
+	ChatID string


meat cian says: why not make this a uuid.UUID

…cast

kylecarbs · 2026-04-01T12:10:38Z

Thanks for the thorough review!

P2 — invalid ChatID fallback: Added TestWorker_MarkStale_WithInvalidChatID in 1b03384. It passes ChatID: "not-a-uuid", asserts GetChats IS called (broadcast activates), and verifies the workspace-filtered chat gets the upsert+publish.

P3 — workspace ownership check on targeted path: Agree this is low risk given CODER_CHAT_ID comes from chatd via the agent (trusted data flow), but the asymmetry is worth noting. Happy to add a comment documenting it or address in a follow-up — let me know your preference.

johnstcn · 2026-04-01T12:12:17Z

P3 — workspace ownership check on targeted path: Agree this is low risk given CODER_CHAT_ID comes from chatd via the agent (trusted data flow), but the asymmetry is worth noting. Happy to add a comment documenting it or address in a follow-up — let me know your preference.

@kylecarbs a comment is probs fine here

… path

kylecarbs · 2026-04-01T12:18:11Z

Added a comment on the targeted path in 9111576 documenting the workspace ownership skip and why it's safe.

johnstcn

Approved in advance, pending comment regarding making ChatID in MarkStaleParams a uuid.UUID

Parse chat_id at the handler boundary instead of inside MarkStale. Removes the string-parsing fallback path — invalid values are silently ignored at parse time.

kylecarbs · 2026-04-01T12:52:31Z

Changed MarkStaleParams.ChatID from string to uuid.UUID in bdbc7b1. Parsing now happens at the handler boundary (workspaceAgentsExternalAuth) — invalid values are silently ignored at parse time, and the invalid-string fallback path in MarkStale is gone. The old TestWorker_MarkStale_WithInvalidChatID test is replaced by TestWorker_MarkStale_NilChatID_Broadcasts which tests the zero-value broadcast path.

github-actions Bot assigned kylecarbs Mar 31, 2026

kylecarbs requested a review from johnstcn April 1, 2026 11:15

johnstcn reviewed Apr 1, 2026

View reviewed changes

test(coderd/x/gitsync): add test for invalid ChatID fallback to broad…

1b03384

…cast

docs(coderd/x/gitsync): document workspace ownership skip on targeted…

9111576

… path

kylecarbs requested a review from johnstcn April 1, 2026 12:31

johnstcn approved these changes Apr 1, 2026

View reviewed changes

refactor: make MarkStaleParams.ChatID a uuid.UUID

bdbc7b1

Parse chat_id at the handler boundary instead of inside MarkStale. Removes the string-parsing fallback path — invalid values are silently ignored at parse time.

kylecarbs enabled auto-merge (squash) April 1, 2026 13:00

kylecarbs merged commit 19e44f4 into main Apr 1, 2026
25 of 26 checks passed

kylecarbs deleted the fix/subagent-git-context-targeting branch April 1, 2026 13:05

github-actions Bot locked and limited conversation to collaborators Apr 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: target specific chat in MarkStale instead of broadcasting to all workspace chats#23883

fix: target specific chat in MarkStale instead of broadcasting to all workspace chats#23883
kylecarbs merged 4 commits into
mainfrom
fix/subagent-git-context-targeting

kylecarbs commented Mar 31, 2026

Uh oh!

johnstcn commented Apr 1, 2026

Uh oh!

johnstcn Apr 1, 2026

Uh oh!

kylecarbs commented Apr 1, 2026

Uh oh!

johnstcn commented Apr 1, 2026

Uh oh!

kylecarbs commented Apr 1, 2026

Uh oh!

johnstcn left a comment

Uh oh!

kylecarbs commented Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kylecarbs commented Mar 31, 2026

Problem

Fix

Data flow before fix

Data flow after fix

Uh oh!

johnstcn commented Apr 1, 2026

Uh oh!

johnstcn Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

kylecarbs commented Apr 1, 2026

Uh oh!

johnstcn commented Apr 1, 2026

Uh oh!

kylecarbs commented Apr 1, 2026

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

kylecarbs commented Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants