Commit 4406585
agentHost: observe dispatched actions through a chat contribution hook (#333238)
* agentHost: report every terminal turn outcome through onTurnEnd
Closes three gaps where a turn ended without telling contributions. A turn that captured a
start checkpoint and then failed to send never scheduled its changeset recompute, because
`captureTurnStartCheckpoint` runs before `sendMessage` but the catch never reported the end.
- Reports a turn end when a send throws. This is the bug: `CheckpointAndChangesetContribution`
handles `kind === 'error'` and never heard about these turns, so their changeset recompute
and unread marking did not run.
- Reports a turn end for a client-dispatched cancellation, which the agent-signal path already
did. This fires exactly once: the reducer clears the active turn first, so a later
agent-emitted cancellation takes the no-active-turn path, which reports only completions.
- Adds a `rejected` reason for requests refused before their turn started, and fires it from
both `startTurn` early returns: a rejected admission and a missing provider. This restores
the per-rejection observability that hoisting the admission gate removed.
- Makes `MarkUnreadContribution` ignore `rejected`. It filtered negatively, so the new reason
would have marked a session unread for a request that never ran, and a rejection on an
archived session would resurface it.
- Leaves `QueueDrainContribution` untouched for `rejected`. A rejected queued turn consumes its
message and the queue stalls, but draining would cascade an archived chat through every
queued message. The fix is to return the message to the queue, which is recorded as such.
- Adds tests for all three paths, including that a rejected turn end does not resurface a read
session. Both new `agentSideEffects` tests were confirmed to fail without the fix.
Rewrites `chatContributions/TODO.md` as a backlog. It had grown into a changelog of finished
extractions and a design-decision record, and two of its claims were already wrong: the
side-chat migration it listed as pending was complete, and `githubReferences` was filed under
outgoing-turn contributions although it only implements `onTurnEnd`. Rationale that had no
other home moved into JSDoc beside the code it explains.
(Commit message generated by Copilot)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* agentHost: observe dispatched actions through a chat contribution hook
Moves session input-needed aggregation, turn-usage persistence, and session flag persistence
out of `AgentSideEffects`, which loses 252 lines. All three observed
`AgentHostStateManager.onDidEmitEnvelope` rather than the client dispatch path, so none of
them could use the existing action hook: most of the actions they react to are dispatched by
the host, not by a client.
- Adds `onDidDispatchAction`, which observes an action from any origin once its outcome is
known, including server-dispatched actions and rejected ones that never reduced. It is a
separate hook and not a widening of the client hook, because `queueDrain` depends on that
hook being client-only and would otherwise see each client action twice.
- Adds `SessionInputNeededContribution` (order 200), which mirrors per-chat blockers into the
owning session's `inputNeeded` list. This is the bulk of the move at roughly 170 lines.
- Gives `PersistedTurnUsageContribution` the write half of turn-usage persistence. It already
owned the restore half, so one contribution now owns both directions, as chat drafts and
chat titles already do.
- Adds `SessionFlagsContribution` (order 700) for read state, archived state, and merged
config values. It preserves an asymmetry that is easy to lose: read and archived changes
skip a rejected action, config values do not.
- Makes `AgentHostToolCallTracker` injectable. Input-needed reports blocked and unblocked tool
calls to it, and it was constructed inside `AgentSideEffects`, so a contribution could not
reach it.
Renames the two action hooks so each says what it observes:
- `onAction` becomes `onDidApplyClientAction`, and `IObservedAction` becomes
`IAppliedClientAction`. Rejected client actions never reach it, so an action it sees always
reduced.
- `onEnvelope` becomes `onDidDispatchAction`, and `IObservedEnvelope` becomes
`IDispatchedAction`. It also delivers rejected actions, so a name claiming they were applied
would be wrong.
Registers the built-in contributions in the tool-call and turn-hang telemetry test graphs.
Both build `AgentSideEffects` without them, so they stopped exercising blocked-call telemetry
once that reporting moved into a contribution. The graphs now mirror production wiring; no
assertion changed.
Updates the contributions skill, which described four hooks and named two that no longer
exist. It now documents all seven, records that the admission hook fails closed and is
synchronous while every other hook isolates failures, and carries the design rationale that
`TODO.md` used to hold. `TODO.md` keeps only open work and caveats, with each fact in one
place.
(Commit message generated by Copilot)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* agentHost: fix cyclic dependency and rejected-action handling from review
- Moves `toolSourceKindFromContributor` and `canRefineContributor` into
`node/shared/toolCallContributor.ts`. Injecting the turn tracker into the tool-call tracker
made the two import each other at runtime, which failed the cyclic dependency check.
- Reports a turn end only when a turn actually ended. `AgentHostTurnTracker.turnCompleted` now
returns whether it had a tracked turn, and the client cancellation and failed-send paths use
that. The reducer no-ops a stale or duplicate cancellation, so reporting one unconditionally
marked a read session unread for a turn that never stopped.
- Reports a turn end when a resumed turn fails to send. That path dispatched `ChatError` and
completed tracking without telling contributions, so it stayed invisible.
- Skips rejected actions in `sessionInputNeeded` and `persistedTurnUsage`. A rejected action
never reduced, so mirroring it can clear a blocker that is still outstanding, and persisting
it can write durable state that was refused.
- Starts the turn through `handleAction` in the client-cancellation test, as `_dispatchActionNow`
does. The shared `startTurn` helper only dispatches to state, so the turn was never tracked
and the test did not reflect production wiring.
- Adds a test that a stale cancellation reports no turn end, and corrects a stale
`_chatContributions.action(...)` reference in the backlog.
(Commit message generated by Copilot)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Dmitriy Vasyura <dmitriv@microsoft.com>1 parent a39698f commit 4406585
25 files changed
Lines changed: 1076 additions & 635 deletions
File tree
- .github/skills/agent-host-chat-contributions
- src/vs/platform/agentHost
- common
- node
- chatContributions
- chatDraft
- markUnread
- persistedTurnUsage
- queueDrain
- sessionFlags
- sessionInputNeeded
- sessionTitle
- test/node
Large diffs are not rendered by default.
Lines changed: 55 additions & 12 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
20 | | - | |
21 | | - | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
| 28 | + | |
27 | 29 | | |
28 | 30 | | |
29 | 31 | | |
| |||
123 | 125 | | |
124 | 126 | | |
125 | 127 | | |
126 | | - | |
| 128 | + | |
127 | 129 | | |
128 | 130 | | |
129 | 131 | | |
130 | 132 | | |
131 | 133 | | |
132 | 134 | | |
133 | 135 | | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
134 | 153 | | |
135 | 154 | | |
136 | 155 | | |
| |||
205 | 224 | | |
206 | 225 | | |
207 | 226 | | |
208 | | - | |
209 | | - | |
210 | | - | |
211 | | - | |
212 | | - | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
213 | 235 | | |
214 | 236 | | |
215 | 237 | | |
216 | 238 | | |
217 | | - | |
218 | | - | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
219 | 257 | | |
220 | 258 | | |
221 | 259 | | |
222 | 260 | | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
223 | 265 | | |
224 | 266 | | |
225 | 267 | | |
| |||
271 | 313 | | |
272 | 314 | | |
273 | 315 | | |
274 | | - | |
| 316 | + | |
| 317 | + | |
275 | 318 | | |
276 | 319 | | |
277 | 320 | | |
| |||
Lines changed: 18 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| |||
146 | 146 | | |
147 | 147 | | |
148 | 148 | | |
149 | | - | |
| 149 | + | |
150 | 150 | | |
151 | 151 | | |
152 | | - | |
| 152 | + | |
153 | 153 | | |
154 | 154 | | |
155 | 155 | | |
156 | | - | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
157 | 171 | | |
158 | 172 | | |
159 | 173 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| 58 | + | |
58 | 59 | | |
59 | 60 | | |
60 | 61 | | |
| |||
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
| 104 | + | |
103 | 105 | | |
104 | 106 | | |
105 | 107 | | |
| |||
Lines changed: 12 additions & 40 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | | - | |
12 | | - | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
13 | 16 | | |
14 | 17 | | |
15 | 18 | | |
| |||
34 | 37 | | |
35 | 38 | | |
36 | 39 | | |
37 | | - | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | 40 | | |
73 | 41 | | |
74 | 42 | | |
| |||
104 | 72 | | |
105 | 73 | | |
106 | 74 | | |
| 75 | + | |
| 76 | + | |
107 | 77 | | |
108 | 78 | | |
| 79 | + | |
| 80 | + | |
109 | 81 | | |
110 | 82 | | |
111 | 83 | | |
112 | 84 | | |
113 | 85 | | |
114 | 86 | | |
115 | 87 | | |
116 | | - | |
117 | | - | |
| 88 | + | |
| 89 | + | |
118 | 90 | | |
119 | 91 | | |
120 | 92 | | |
| |||
132 | 104 | | |
133 | 105 | | |
134 | 106 | | |
135 | | - | |
| 107 | + | |
136 | 108 | | |
137 | 109 | | |
138 | 110 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
40 | | - | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
41 | 56 | | |
42 | 57 | | |
43 | 58 | | |
| |||
66 | 81 | | |
67 | 82 | | |
68 | 83 | | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
69 | 91 | | |
70 | 92 | | |
71 | 93 | | |
| |||
77 | 99 | | |
78 | 100 | | |
79 | 101 | | |
| 102 | + | |
80 | 103 | | |
81 | 104 | | |
82 | 105 | | |
83 | 106 | | |
84 | | - | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
85 | 115 | | |
86 | 116 | | |
87 | 117 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| |||
374 | 374 | | |
375 | 375 | | |
376 | 376 | | |
377 | | - | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
378 | 384 | | |
379 | 385 | | |
380 | 386 | | |
381 | | - | |
| 387 | + | |
382 | 388 | | |
383 | 389 | | |
384 | 390 | | |
| |||
425 | 431 | | |
426 | 432 | | |
427 | 433 | | |
| 434 | + | |
428 | 435 | | |
429 | 436 | | |
430 | 437 | | |
| |||
0 commit comments