Repository navigation
Commit be78b8f
authored
🤖 refactor: the subagent artifact index writes take the workspace FIFO from SessionLocks (#7425)
## Summary
The report, failure, transcript and git patch artifact index writes take
the workspace FIFO from `SessionLocks`. It is PR 14 of phase 4. Lock
keys and order do not change: the Node layer gives today's
`workspaceFileLocks`.
## Order
- PR 13 (#7423), which the plan puts below it, has merged, so the base
is main. PR 18 (the branch-summary marker, landing note 15) stacks on it
in the native stack.
- Phase 5b PR 3 waits for it.
## Implementation
- `subagentFailureArtifacts.ts`, `subagentReportArtifacts.ts`,
`subagentTranscriptArtifacts.ts`, `subagentGitPatchArtifacts.ts`: each
index write takes an optional `locks` (Node default). In
`subagentReportArtifacts.ts`, the helper `workspaceFileLocksOf` holds
the fallback, so the CC of `upsertSubagentReportArtifact` does not grow.
- `workspaceService.ts`: the child-artifact rollup passes
`this.historyService.locks` for the parent's index writes.
Files beyond the plan's list (landing note 14). The plan said that
`taskService.ts`, `gitPatchArtifactService.ts` and
`taskGitPatchEngine.ts` do not change. These changes close the gap of a
production caller that has a service but keeps the Node default:
- `taskService.ts`: the failure and report upserts pass
`this.historyService.locks`, and so does `new
GitPatchArtifactService(config, locks)`.
- `gitPatchArtifactService.ts`: the constructor requires `locks`, so the
type checker proves that no caller is left. Its 4 upserts pass them.
`gitPatchArtifactService.test.ts` passes `nodeSessionLocks` at its 4
call sites. One test now records the locks that the service takes.
- `taskGitPatchEngine.ts`: `TaskGitPatchApplyConfig` picks
`sessionLocks`, and the applied watermark write passes it.
- `common/utils/tools/tools.ts`: the `sessionLocks` comment names the
watermark write. The `task_apply_git_patch` tool gets the locks through
its tool config, which `turnRequestBuilder` already sets.
- `turnRequestBuilder.ts`, `workflows/WorkflowService.ts`,
`cli/workflow.ts`: each workflow patch config sets `sessionLocks` from
its history service. `WorkflowServiceContext` gains `historyService:
Pick<HistoryService, "locks">`. The ORPC context already has it.
`WorkflowService.context.test.ts` adds it to its test context.
The 4 artifact functions keep an optional `locks`: a required one breaks
about 25 test files.
## Validation
On the stack head `c0831c924e` (PR 14 on PR 13's final head
`41aab3e5b4`), every gate passed:
- `make static-check` and `make lint-eslint` pass. `make crap-check`:
PASS, 0 violations, 7 warnings on touched hotspots (total CC of the
changed files 10107 -> 10112: the 4 artifact functions each gain one
`locks ?? nodeSessionLocks` fallback, and `workspaceFileLocksOf` holds
the report one).
- `bun test` of the 4 `subagent*Artifacts.test.ts`,
`historyService.test.ts`, `taskService.agentReports.test.ts`,
`taskService.handoffAndCleanup.test.ts`,
`gitPatchArtifactService.test.ts`, `taskGitPatchEngine.test.ts`,
`workflows/WorkflowService.context.test.ts`,
`workflows/WorkflowTaskServiceAdapter.test.ts` and
`tools/task_apply_git_patch.test.ts`: 589 pass, 0 fail.
- The tripwire (`scripts/perf/loopTestBaseline.ts --runs 3 --base
41aab3e`): 0 trips.
- `make test`: 0 fail in every run.
The head `0ae88d7ef7` is the same 5 commits rebased on main `d1f9d61489`
(after #7423). Its diff against main has the 14 files above. On this
head, `make static-check` passes and the plan tests give 589 pass, 0
fail.
Test: `gitPatchArtifactService.test.ts` gives the service a
`SessionLocks` whose workspace FIFO records each key, and checks that
the parent's index writes take it. Revert proof: with the 4 upserts back
on the Node default, that test fails.
## Mutation check
`make mutate-check` (base `6887934d00`, so PR 13's lines are in scope
too): 167 killed, 128 survived, 4 equivalent, 1 without a test, 682 not
run. No survivor is on a line that this PR changes. I list them in #7326
for PR 17.
- The 4 equivalents are the `locks ?? nodeSessionLocks` fallbacks, each
with a `mutation-equivalent` marker: `locks` is a `SessionLocks` object
or undefined, so `??` and `||` agree.
- The 1 without a test is `cli/workflow.ts`, and the local baseline
reports it as failed. That is a local limit: mutate's worker copies hold
only tracked files, and `cli/workflow.test.ts` runs the built
`dist/cli/index.js`, which they lack. The test passes in the checkout
(10 pass, 0 fail). The PR adds one `sessionLocks` field there, with no
operator to mutate. CI's Quality / Mutation is the check of record.
- Survivors: `GitPatchArtifactService.generate` 29,
`maybeStartGenerationUnlocked` 20,
`taskGitPatchEngine.applyProjectPatch` 31 and `applyPatch` 21,
`upsertSubagentReportArtifact` 7, `upsertSubagentFailureArtifact` 3, and
5 more in the other artifact functions. PR 13's survivors
(`rollbackRefinement` 9, `withTargetMutationLocks` 2,
`acquireRollbackFileLock` 1) repeat and are already listed.
## Risks
- None for behavior: in the app, `historyService.locks` is
`nodeSessionLocks`, which gives the same `workspaceFileLocks` FIFO.
---
_Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking:
`high` • Cost: `$163.55`_
<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high
costs=163.55 -->1 parent b4ea1e4 commit be78b8f
14 files changed
Lines changed: 109 additions & 17 deletions
File tree
- src
- cli
- common/utils/tools
- node/services
- workflows
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
476 | 476 | | |
477 | 477 | | |
478 | 478 | | |
| 479 | + | |
479 | 480 | | |
480 | 481 | | |
481 | 482 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
155 | 155 | | |
156 | 156 | | |
157 | 157 | | |
158 | | - | |
| 158 | + | |
159 | 159 | | |
160 | 160 | | |
161 | 161 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
| 21 | + | |
20 | 22 | | |
21 | 23 | | |
22 | 24 | | |
| |||
97 | 99 | | |
98 | 100 | | |
99 | 101 | | |
100 | | - | |
| 102 | + | |
101 | 103 | | |
102 | 104 | | |
103 | 105 | | |
| |||
152 | 154 | | |
153 | 155 | | |
154 | 156 | | |
155 | | - | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
156 | 169 | | |
157 | 170 | | |
158 | 171 | | |
| |||
178 | 191 | | |
179 | 192 | | |
180 | 193 | | |
| 194 | + | |
| 195 | + | |
181 | 196 | | |
182 | 197 | | |
183 | 198 | | |
184 | 199 | | |
185 | | - | |
| 200 | + | |
186 | 201 | | |
187 | 202 | | |
188 | 203 | | |
| |||
268 | 283 | | |
269 | 284 | | |
270 | 285 | | |
271 | | - | |
| 286 | + | |
272 | 287 | | |
273 | 288 | | |
274 | 289 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| 40 | + | |
40 | 41 | | |
41 | 42 | | |
42 | 43 | | |
| |||
321 | 322 | | |
322 | 323 | | |
323 | 324 | | |
324 | | - | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
325 | 330 | | |
326 | 331 | | |
327 | 332 | | |
| |||
455 | 460 | | |
456 | 461 | | |
457 | 462 | | |
| 463 | + | |
458 | 464 | | |
459 | 465 | | |
460 | 466 | | |
| |||
504 | 510 | | |
505 | 511 | | |
506 | 512 | | |
| 513 | + | |
507 | 514 | | |
508 | 515 | | |
509 | 516 | | |
| |||
538 | 545 | | |
539 | 546 | | |
540 | 547 | | |
| 548 | + | |
541 | 549 | | |
542 | 550 | | |
543 | 551 | | |
| |||
590 | 598 | | |
591 | 599 | | |
592 | 600 | | |
| 601 | + | |
593 | 602 | | |
594 | 603 | | |
595 | 604 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
175 | 176 | | |
176 | 177 | | |
177 | 178 | | |
| 179 | + | |
| 180 | + | |
178 | 181 | | |
179 | | - | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
180 | 185 | | |
181 | 186 | | |
182 | 187 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
348 | 349 | | |
349 | 350 | | |
350 | 351 | | |
| 352 | + | |
| 353 | + | |
351 | 354 | | |
352 | | - | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
353 | 358 | | |
354 | 359 | | |
355 | 360 | | |
| |||
375 | 380 | | |
376 | 381 | | |
377 | 382 | | |
| 383 | + | |
| 384 | + | |
378 | 385 | | |
379 | 386 | | |
380 | 387 | | |
381 | 388 | | |
382 | 389 | | |
383 | 390 | | |
| 391 | + | |
384 | 392 | | |
385 | 393 | | |
386 | 394 | | |
| |||
401 | 409 | | |
402 | 410 | | |
403 | 411 | | |
| 412 | + | |
| 413 | + | |
404 | 414 | | |
405 | 415 | | |
406 | 416 | | |
407 | 417 | | |
408 | 418 | | |
409 | 419 | | |
| 420 | + | |
410 | 421 | | |
411 | 422 | | |
412 | 423 | | |
| |||
| 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 | | |
| |||
262 | 263 | | |
263 | 264 | | |
264 | 265 | | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
265 | 272 | | |
266 | 273 | | |
267 | 274 | | |
268 | 275 | | |
| 276 | + | |
| 277 | + | |
269 | 278 | | |
270 | | - | |
| 279 | + | |
| 280 | + | |
271 | 281 | | |
272 | 282 | | |
273 | 283 | | |
| |||
302 | 312 | | |
303 | 313 | | |
304 | 314 | | |
| 315 | + | |
| 316 | + | |
305 | 317 | | |
306 | 318 | | |
307 | 319 | | |
308 | | - | |
| 320 | + | |
| 321 | + | |
309 | 322 | | |
310 | 323 | | |
311 | 324 | | |
| |||
| 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 | | |
| |||
112 | 113 | | |
113 | 114 | | |
114 | 115 | | |
| 116 | + | |
| 117 | + | |
115 | 118 | | |
116 | | - | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
117 | 122 | | |
118 | 123 | | |
119 | 124 | | |
| |||
137 | 142 | | |
138 | 143 | | |
139 | 144 | | |
| 145 | + | |
| 146 | + | |
140 | 147 | | |
141 | 148 | | |
142 | 149 | | |
143 | 150 | | |
144 | 151 | | |
145 | 152 | | |
| 153 | + | |
146 | 154 | | |
147 | 155 | | |
148 | 156 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
39 | 40 | | |
40 | 41 | | |
41 | 42 | | |
42 | | - | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
43 | 50 | | |
44 | 51 | | |
45 | 52 | | |
| |||
1248 | 1255 | | |
1249 | 1256 | | |
1250 | 1257 | | |
| 1258 | + | |
| 1259 | + | |
1251 | 1260 | | |
1252 | 1261 | | |
1253 | 1262 | | |
| |||
1668 | 1677 | | |
1669 | 1678 | | |
1670 | 1679 | | |
| 1680 | + | |
1671 | 1681 | | |
1672 | 1682 | | |
1673 | 1683 | | |
| |||
1988 | 1998 | | |
1989 | 1999 | | |
1990 | 2000 | | |
| 2001 | + | |
1991 | 2002 | | |
1992 | 2003 | | |
1993 | 2004 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5212 | 5212 | | |
5213 | 5213 | | |
5214 | 5214 | | |
5215 | | - | |
| 5215 | + | |
5216 | 5216 | | |
5217 | 5217 | | |
5218 | 5218 | | |
| |||
19917 | 19917 | | |
19918 | 19918 | | |
19919 | 19919 | | |
| 19920 | + | |
19920 | 19921 | | |
19921 | 19922 | | |
19922 | 19923 | | |
| |||
21400 | 21401 | | |
21401 | 21402 | | |
21402 | 21403 | | |
| 21404 | + | |
21403 | 21405 | | |
21404 | 21406 | | |
21405 | 21407 | | |
| |||
0 commit comments