Merge Risk: Low · up to 21cc3

The new tests only strengthen coverage. One known edge case remains open: after a failed per-view state write, a compensating write could persist state the caller was told had failed. It is narrow and rare, so a bounded follow-up is enough.

Failed checks: 2 errors · 2 warnings

Check name	Status	Explanation	Resolution
Security Boundaries		Summary: The prior FAIL still applies at the current head (21cc345). The rollback logic it cites is new in this PR and is unchanged. Changed path: - src/core/webview/ClineProvider.ts:4424-4433 …	Track write ownership or a monotonic revision for every shared-setting writer, including writes that set the same value. Restore a key only when its last writer is still this call. Make the ownership check and the restore atomic with respec…
Persistence Integrity		The checkout is at head 21cc345, the head the standing failure cites. Both persistence defects still exist in the changed code. I did not refute either one. **1. Reset does not coordinate with the v…	1. Reset and the view-state queue: - Make resetState (and any other reset or import clear of viewStates) run inside persistedViewStateWriteQueue, or advance viewStatesResetGeneration there. A queued write that started before the res…
Regression Evidence		The prior FAIL's three claims still hold at the current head. The PR adds view-state behavior in ClineProvider.ts, and the only new provider spec is `src/core/webview/tests/ClineProvider.persist…	Add focused tests in src/core/webview/__tests__/ClineProvider.persistedViewState.spec.ts: - Partial clear: save mode and currentApiConfigName, clear only one field, and assert the other field survives in the entry and in durable stora…
Lifecycle Resource Cleanup		Concrete changed lifecycle path leads to a task that runs after provider disposal. The prior FAIL is not resolved at head 21cc345. Changed path: - `ClineProvider.creat…	Reject task creation after disposal in ClineProvider.createTask: 1. Check this._disposed after each await that precedes addClineToStack (after setValues, getState, and evictCurrentTask). Throw or return if disposed. 2. Recheck `…

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main change as the view-state store and places it in unit 1 of the split. It is concise and related to the changeset.
Description check		The description is detailed, on-topic, and documents implementation scope, tests, CI results, measurements, review findings, and remaining approval requirements. It does not follow the repository temp…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is detailed, on-topic, and documents implementation scope, tests, CI results, measurements, review findings, and remaining approval requirements. It does not follow the repository template fully: it omits the required Closes issue line, checklist, and Discord username, but the core description and test procedure are substantially complete.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Address automated review findings and push fixes.

After fixes are pushed and required CI passes, automated review restarts.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

Codecov Report

❌ Patch coverage is 96.85315% with 9 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

Changed-code mutation testing

Report for PR head 21cc345d3deb.

pnpm install --frozen-lockfile
node scripts/stryker-diff.mjs ci --base 53cb5b5e7c8014d3226e5636bbca8999ed3b8526 --head 917c6cd79e47d591e721368d9514841d7de4d0ea

Who owns the profile reader, and what the reviewer's fallback costs

Answering the ownership question on this PR before any hunk moves. Three measured answers,
then the price of the fallback the reviewer offered.

1. Which unit owns the profile reader under the plan of record

#1992's plan comment (comment 6773802396, body updated 2026-11-10T18:49:42Z) line 129:

src/core/config/ProviderSettingsManager.ts - 19 executable lines on its own, carried by
one unit (U1). At 19 it fits whole too.

Its line 26 file-ownership table row carries the same assignment (extension, 19 lines), and
line 83 already names ProviderSettingsManager.getCurrentProfileName() as the definition
that shape puts in the first unit. So unit 1 owns the file and every executable line in it:
reader and resolver sit in the same unit, and no later unit owns part of that file.

2. Which tree the method actually sits in

It is already inside unit 1's tree, not a later unit's. At unit 1 head
6e3cfda3f75b6d4fdeeb4d534f6c35dab6926308, src/core/config/ProviderSettingsManager.ts:508
declares public async getCurrentProfileName(): Promise<string | undefined>, the read is at
510-513, and the Failed to read the active profile: wrapper at 515. The first production
callers arrive with the next unit: src/core/webview/ClineProvider.ts:3102 and :3116 at
unit 2 head 36498f188369cfb5137a4b393b410ad37aa74b4f, inside the try/catch of the
provider-manager snapshot helpers that compensate a failed profile mutation.

A reader being read by a later unit is the shape the split draws, not a defect in it: the
definition is where the plan puts it and the callers are where the mutation group needs them.

3. The move the ask would imply has nowhere to go, measured on the instrument CI runs

The move does not need pricing line by line, because the destination has no margin at all.
Both figures below are printed by the in-tree instrument blob
7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc - the revision sitting at org main
53cb5b5e7c8014d3226e5636bbca8999ed3b8526 and at every unit head, i.e. the revision CI runs,
with MAX_CHANGED_LINES = 500 declared in it.

• unit 1 c2ed51f8484cbd8781e924fc73a7c69e65247354 from merge base
  18861b2e357203e6cb4d93930aa2ed122b1eecea prints
  Mutation-testing 2 package(s) from merge base 18861b2e3572: types (6 lines), extension (479 lines)
  • 21 lines of margin.
• unit 2 4d0bb6feea5ebd9fc3de6e854684bc8eaede4181 from merge base
  c2ed51f8484cbd8781e924fc73a7c69e65247354 prints
  Mutation-testing 1 package(s) from merge base c2ed51f8484c: extension (500 lines)
  • 0 lines of margin, exactly at the cap.

Any non-empty move of the definition from unit 1 into unit 2 overflows unit 2's cap, and
unit 1's 21 lines of margin do not pay for it. The alternative the reviewer offered - a
focused test instead of a move - is what landed.

4. What landed on this PR, and what it costs

Spec-path-only commit c2ed51f8484cbd8781e924fc73a7c69e65247354 (parent 6e3cfda3f75b),
one file, src/core/config/__tests__/ProviderSettingsManager.spec.ts, 108 added rows and 1
deleted row. Six focused tests:

• getProfile with a missing name rejects with an instance of ProviderSettingsNotFoundError
  carrying Config with name 'absent' not found and error.name set - checked by instance,
  not by message text.
• getProfile with a missing id rejects with the same type carrying
  Config with ID 'missing-id' not found.
• a missed lookup stays out of the wrapped-read branch.
• a failed profile read surfaces as a plain Error named Error, not the typed error, with a
  message that starts Failed to get profile:.
• deleteConfig on a missing name rethrows ProviderSettingsNotFoundError with
  Config 'absent' not found, while the last-remaining-config refusal stays a plain Error
  under Failed to delete config:.
• getCurrentProfileName is covered directly: the value it returns when the store marks a
  profile current, and the Failed to read the active profile: wrapper when the read fails.

Verification on that commit: vitest run core/config/__tests__/ProviderSettingsManager.spec.ts
prints Test Files 1 passed (1) and Tests 68 passed (68); eslint --max-warnings=0 on the
same file exits 0; the commit touches only that file, so no ESLint suppression count moves
(a --prune-suppressions probe proposed no reduction anywhere: 2867 suppressed findings across
265 keys before and after). pnpm --dir src run check-types reports Tasks: 12 successful, 12 total.

Re-running the same instrument over the commit prints the identical price line quoted in 3, so
these 109 added+deleted rows add 0 counted executable lines to any mutation-tested package.

Two ceilings, for two audiences, so the figures are not read against the wrong one: the review
budget is 400 added+deleted rows and this commit costs 109 of them; the gate's ceiling is 500
counted executable lines per mutation-tested package and unit 1 stands at 479 with the tests
included. Unit 2's spec-only commit costs a further 82 added rows the same way, measured at 0
counted lines, and leaves that unit at its measured 500 with zero margin.

@coderabbitai review

For anyone reading this pull later: the gate's blocking count of 0 at head
21cc345 means the extension package was not evaluated, not that its
mutants were killed.

Machine comment 6777017497, rewritten 2026-11-11T09:16:07Z, prints blocking:0 advisories:1 with 0
mutant rows, and its own package table row for the extension says 491 changed executable lines with
Valid 0, Killed 0, Timeout 0, Survived 0, No coverage 0 and Result "Advisory incomplete", under the
advisory finding "extension mutation run exceeded 12 minutes". Its focused-tests line for the extension
states "The mutation run was incomplete; selected test file names were not recorded." The check
finished completed:success (run 130424642052, 08:58:55Z -> 09:15:52Z): a run that evaluated nothing is
green here.

So please do not read the absence of rows for lines 445, 466, 480, 523, 525, 779, 781 or 803 at this
head as evidence that those lines were checked. The previous head 4bf26f1
did evaluate the package and listed 232 blocking rows (run 130409270995, 08:23:25Z -> 08:39:07Z); the
package is 491 of 500 changed executable lines at both heads. Instrument for both: in-tree
scripts/stryker-diff.mjs at blob 7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc against merge base
53cb5b5.

We have put the split-or-narrow-exclusion decision to the maintainers on #1991 with the printed finding
and are not proposing an answer here. Unproved and stated as such: what the extension run would have
reported had it finished inside the 12 minutes.

Prices for the divided-file shape of this pull's provider file, measured locally. Nothing here asks for anything, proposes anything, or volunteers a push.

Footing first, with the mechanism named. Everything below is measured against my own local snapshot of this pull's head: commit 71cfcf3, tree 475953b5cf86aa115464d65ac8b1f5e2051a488b, commit date 2026-11-11T04:15:11+08:00. That date is quoted from a sha-bearing artifact of mine that interpolated it out of git log --date=iso-strict. Earlier prose of mine gave 2026-11-11T23:41:32+08:00 for the same commit, which is a value no read produced, and I repeated it twice before checking; the artifact took its number from git and was right, the prose came out of memory and was wrong. Worth reading the artifact rather than the narrative. Each divided piece below is a commit I constructed and parented on the merge base 18861b2, so a first piece is priced against that merge base and a residual is priced against its own parent. Instrument: scripts/stryker-diff.mjs, blob 7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc, read back out of each priced tree. Caps, named apart from every result below: 500 changed executable lines per package unit, 400 valid mutants per package unit. Every mutant figure is a preflight instrumented count and never a killing count, so the Stryker 10 plus vitest runtime-activation caveat rides on all of them and no score is claimed here.

Where the pull actually is, read at the free door so that no number below is mistaken for a current one. An unauthenticated REST read (a separate 60-per-hour window, no token involved) reports this pull open against main at 53cb5b5 with head branch vps2/1921-u1-view-state-core at 21cc345. I fetched that head over git transport, which costs nothing, and it is not an ancestor of my snapshot nor the reverse: 13 commits on its side, 5 on mine, merge base 18861b2, same merge base I priced against. At that live head the same instrument, run by me in the same way, measures the src package unit at 516 counted executable lines and 130 selector ranges against the 500 and 400 caps with 385 instrumented mutants in 6 source files, the packages/types unit at 6 lines and 2 ranges with 1 instrumented mutant, and a webview-ui unit of 6 lines and 1 range in ApiOptions.tsx where I did not run the instrument. Of its 22 changed paths, 12 are test or spec paths. The provider file alone measures 467 counted executable lines and 106 ranges there, against 455 and 106 at my snapshot. So the whole-unit prices below are prices of my snapshot, and the live head is over the line figure by 16 lines at the same footing.

The whole delta at the priced snapshot, against the two caps. src package unit: 479 counted executable lines, 115 selector ranges, 364 instrumented mutants. packages/types unit: 6 lines, 2 ranges, 1 instrumented mutant. Against the caps that is 21 lines and 36 mutants of headroom in the src unit, which I am calling a margin and not a pass: the caps are per package unit, no number here came from a CI run, and nothing I measured has been checked by the gate that would enforce them. The margin is the reason I priced the split at all: with room left, a division is a choice and not a requirement.

The one seam that holds, with both pieces priced. Piece 1 is my commit 22f5edd0af1cb63c4a954f72963c3981164c3dec (tree 8c4fc3e9cdc3, parented on the merge base): src/core/webview/ClineProvider.ts truncated to its head prefix through line 1202, which is the closing line of loadViewState, with whole hunks truncated at the seam rather than selected line by line, carried alongside the five sibling files this pull touches. That unit is 369 counted executable lines, 78 selector ranges and 287 instrumented mutants against the 500 and 400 caps, and the packages/types unit beside it is 6 lines, 2 ranges and 1 instrumented mutant, 6 files in all. Piece 2 is my commit efd764ba03482766a18c54b0861a38e7b546528a, parented on piece 1 and carrying the head tree: 1 file, 110 counted executable lines, 37 selector ranges and 77 instrumented mutants against the same two caps. Both pieces are inside both caps. The division is a partition and not a duplication, and that is measurable: counted executable lines in the provider file go 345 plus 110 against a whole of 455 with overlap 0 and union 455, totals 375 plus 110 against 485 for the whole delta, selector ranges 80 plus 37 against 115 plus 2 for the whole delta, instrumented mutants 287 plus 77 against 364 for the src unit and 1 plus 0 against 1 for the types unit. Additivity like that is what caught two of my own broken constructions earlier, one whose union came to 457 lines with 71 whole lines in neither piece and two whose files did not parse and so produced no instrumented count at all.

The subtraction that shows no cut-induced compile errors. I ran the typechecker locally at three points: at the merge base, 22 error lines carrying 16 distinct message signatures; at the head snapshot, 34 error lines carrying 19 signatures; at piece 1, 34 error lines; at piece 2, 34 error lines. Taking away every signature that appears at the base or at the head leaves nothing in either piece, which is the only sense in which I can say the seam does not break the code: 0 error signatures that were absent at both the base and the head.

What a reviewer of the 110-line residual would have to believe without evidence in front of them. The residual depends on three module-scope aliases it does not contain. PersistedViewState is declared in the preamble at head lines 163 to 176 and referenced 21 times in this file across nine members, three of those references inside _persistViewLocalStateFromMutation, which sits on the residual side of the seam. PerViewSavedStateKey and ViewLocalStateValues are declared in the same preamble and used only inside saveViewState, also on the residual side. The residual also depends on ContextProxy.ts lines 70 to 72 and importExport.ts lines 105 to 106, which are 3 and 2 counted executable lines whose entire content is a comment stating a machine-local rule that the residual then behaves according to. And it depends on the viewStates schema declared in the other package unit, at global-settings.ts ranges 120-124 and 134, plus viewStateId in ExtensionState, which carries 0 counted executable lines; that last one is true of the undivided pull too, so it is not a cost this seam adds. A reviewer looking at the residual alone is looking at half a machine unless they take those three statements on trust.

The two cuts that are reviewable in size and not in coherence. Seam at line 927, the closing line of prunePersistedViewStates: piece 1 cbab02ea687695d07f4d90999a55f3c67988c9cd at 224 lines, 38 ranges and 166 instrumented mutants, residual c66fce9f99c665e2f5c56572c3ad8a46c5c16fbd at 261 lines, 79 ranges and 198 instrumented mutants. Seam at line 1003, the closing line of rekeyPersistedViewStateEntry, which is the most even line split available on this file: piece 1 0e5a78b4a2401a5a0e2781fa1d045e19ee84667d at 284 lines, 41 ranges and 199 instrumented mutants, residual 150d5be0296407f77a1554e9ef628f6349fc9000 at 201 lines, 76 ranges and 165 instrumented mutants. All four pieces are inside both caps, and all four are a mechanical cut at the last line that fits rather than a division along a seam: each first piece fails to compile on its own with exactly one error, at line 560, where the constructor calls this.loadViewState and loadViewState is on the other side of the seam. Both cut one state machine in half. The seam one member further along, line 1220, the closing line of saveViewState, is coherent apart from a single call in the same direction: piece 1 83fe59d4fe6666a09e65d51bc040e9f025f696ba at 374 lines, 81 ranges and 288 instrumented mutants with residual 8dbbdebae179dd52f5c8a0fff95703130ace161b at 105 lines, 34 ranges and 76 instrumented mutants, and its one cut-induced error is at line 1217, where saveViewState calls this._saveViewLocalStateFromMutation, a 4-line method that would have to travel with it. Telling: the most even split is the incoherent one, and the coherent seam sits 199 lines past it, line 1202 against line 1003. I had written 22 in that sentence before measuring the gap, and the measured gap is 199 file lines.

Limits, in the comment and not in a footnote. First, beside the numbers rather than at the end: at the priced snapshot the src side of this delta carries no test file at all, and the only test path in the whole snapshot delta is packages/types/src/tests/global-settings.test.ts, which the instrument's package map leaves out of the mutation-target rows entirely. Dividing the delta therefore does not divide the killing evidence; it divides the code and leaves the evidence shared, which is the most important thing these prices do not buy. At the live head the delta carries 12 test or spec paths and the package map still counts none of them as mutation targets. Second, every mutant figure here is instrumented and not killed, and instrumenting does not prove a mutant is killable or that a test activates the runtime that runs it, so no score is claimed. Third, lint is unmeasured: eslint cannot load this worktree's src/eslint.config.mjs because the config-eslint package reached through a junction does not export abortSubstringSelector, so the lint verdict is simply absent and not passing. Fourth, cross-package symbols in the typecheck above resolve through a junction into another worktree's node_modules, so the cross-package parts of the compile claim stay limited and the 34 error lines at the head are pre-existing artifacts of that setup, which is exactly why the cut-induced set had to be a subtraction against both controls rather than a count. Fifth, the sibling assignment is mine and not forced: I put all five sibling files with piece 1 in every construction, and the alternative, putting the schema and the machine-local exclusions with the residual, is unpriced, so the file counts of 6 and 1 belong to my assignment and not to the seam.

The pricing cost 0 authenticated calls: no token cores, git transport and local processes only, and the current head above came from the unauthenticated door. The eight commits named here are local objects held only by refs of my own naming under refs/f0-1528/, a contains-check under my fetched tips returns 0 for each of them, and nothing was pushed.

Re-priced at the head this pull actually carries, because the two paragraphs above leave a reader with two facts and no bridge: the live src unit sits 16 lines over the 500 line cap, and every seam price above is labelled to my snapshot. Bridge, in the order I measured it. Same instrument blob 7dfe0006d2d5c7e4d2282f65f68b8ed5f16866bc, verified identical in the live tree before pricing, same merge base 18861b2, live head 21cc345 (tree 39a30c7b2e9d77c1f2ceea5dc7d5db236f370a1c, committer date 2026-11-11T16:57:57+08:00), same engine validated by requiring an all-selected replay to reproduce the live provider blob 7e24d5240b14 byte-exactly, and the same preflight-not-killing caveat; lint is still unmeasured for the same reason given above.

The coherent seam at its live closing line, line 1214, which is where loadViewState ends: my commit 440987f657ea (tree 15882c45a33d, parented on the merge base) is the src package unit at 406 counted executable lines, 93 selector ranges and 308 instrumented mutants against the 500 and 400 caps across 22 paths, the packages/types unit beside it at 6 lines, 2 ranges and 1 instrumented mutant, and the webview-ui path at 6 lines and 1 range where I did not run the instrument. The residual is my commit 0b5e6d97e3c9, parented on that piece and carrying the live head tree: 1 file, 110 counted executable lines, 37 selector ranges and 77 instrumented mutants, and it is the member tail of the class from saveViewState through broadcastResetToAllInstances. Cut-induced compile signatures: 0 in either piece. That is a subtraction and the control set is stated so it can be redone: typecheck at the merge base 22 error lines, at the live head 36, and the 21 distinct signatures those two logs share are what a piece is compared against, so a signature counts only if it is absent at both. The pieces are still a partition: counted executable lines in the provider file 357 plus 110 against a whole of 467 with overlap 0 and union 467, totals 528 against 528 for the whole delta, selector ranges 133 against 133, instrumented mutants 308 plus 77 against 385.

The two mechanical cuts at this head also fit, and they are still reviewable in size and not in coherence: seam at line 936, where prunePersistedViewStates ends, is piece 1 23b4683acc8e at 252 lines, 51 ranges and 183 instrumented mutants with residual 2c3160f3eff5 at 264, 79 and 202, and seam at line 1015, where rekeyPersistedViewStateEntry ends, is piece 1 3bb1e2461532 at 315, 54 and 220 with residual 297cbc0b3900 at 201, 76 and 165 - all eight pieces inside both caps - and in each of those two the constructor at line 560 reaches across the seam, calling this.loadViewState at a method that the other piece carries.

One finding belongs in the same paragraph because it changes what a cut is thought to cost. Under the filing I used, where all 21 non-provider paths travel with the first piece, those two pieces show 7 and 4 cut-induced signatures, and 6 of the 7 and 3 of the 4 of those are not the seam at all: they are the spec this pull adds, which names savePersistedViewState 8 times, setViewStateId 4 and rekeyPersistedViewStateEntry 3, sitting away from members it asserts on. I priced the re-filing instead of arguing it - the provider spec moved to travel with the member tail, which also means the first piece no longer contains it - on four of my own constructions, refs/f0-1528/2416-L2-mid-machine-at-prune-end-specwithtail-p1 at adc3a7793726 with its pair 898574498a88, and refs/f0-1528/2416-L3-after-rekey-tail-specwithtail-p1 at 3f5e3cc18e2c with its pair 5170bb6806c0. Each first piece then shows exactly 1 cut-induced signature, and the one break no filing removes is the same in both: line 560, the constructor calling this.loadViewState across the seam. The spec has no merge-base content because it is new in this delta, so in those two variants the first piece simply omits it.

The residual price did not move between the two heads - 110 lines, 37 ranges and 77 instrumented mutants at both - and the reason is measurable, which matters more than the size: between my priced snapshot 71cfcf3 and the live head 21cc345 the provider file carries 12 hunks whose new-side anchors are 771, 776, 779, 784, 803, 888, 904, 973, 975, 977, 981 and 1001, the highest of them 1001 and every one below the coherent seam, and the residual region is byte-identical at the two heads, snapshot lines 1202 to end against live lines 1215 to end, 5,122 lines each. Everything that changed between the two heads in non-test source sits in the provider prefix, in Task.ts, in observationRegistry.ts and in webview-ui/src/components/settings/ApiOptions.tsx, and none of it in the tail. A reviewer reading the residual is reading a region that those 13 commits left alone, not merely a region that happens to be small.

And the killing-evidence limit in its honest current form, since it moved at this head and moved only halfway: a division here does divide some of the evidence, because the prefix half now has a spec that this very delta adds, while the residual half still leans on test files this pull does not touch - no test path in the delta names a residual-side member. One caveat kept rather than quietly spent: the string setValue appears in that provider spec and in ContextProxy.spec.ts and importExport.spec.ts, and in the two config files it most plausibly names the ContextProxy setter rather than the provider member, and I did not verify which, so I am not counting those occurrences as evidence for the provider member of the same name. The pricing in this paragraph cost nothing on the platform: git transport and local processes only, no authenticated call, and none of the constructions above is pushed - a contains-check under my fetched tips returns 0 for each of them.