Skip to content

Commit 3f1983f

Browse files
Merge branch 'vps2/n2-viewstates-durability' (515cc02) into #2235
Merge base is f3ca33b, which is the incoming commit's own parent and already an ancestor of this head through f2d4336, so the merge is a normal two-parent merge rather than a fold and it needed no conflict decision: git auto-merged both shared paths and merge-tree on the two heads reported no conflicted files. The incoming commit is ContextProxy.ts +12 and ContextProxy.spec.ts +50 only, which is disjoint from everything on this side since f2d4336 (ClineProvider.ts and its spec). Checked rather than trusted, because an auto-merge can be clean and still wrong: - ordering in resetAllState (merged 758-788): their resetting = true opens the method, ahead of my in-flight snapshot (763-765), both cache clears (768-769), the generation-advance loop (772-774), both owner clears (778-779), the delete pass (781-785) and initialize (787); their resetting = false sits after the re-initialize. Nothing on this side of the settlement moved. - their storeSecret refusal (466-473) sits above the cache write, so a refused write never claims ownership, never bumps the write count and never dirties the cache. - survivors: my 81 declarations in ContextProxy.spec.ts all survive (66 it/test plus 15 describe, 2 .each tables), the merged file holds 82 with their one new case present exactly once, "erases a secret whose write begins after the reset has snapshotted the pending writes". - inventory: 3147 paths on each head and 3147 in the merged index, nothing dropped from either side. - both merged blobs are LF-only with a trailing newline. Gates on this tree, in compile-job order: format:check clean; lint 12/12; check-types 11/11; test:code-qa-ci 7 pass 0 fail. The solheim test groups still fail on Windows alone (SIGTERM teardown cases in scripts/solheim-smoke-workflow.test.mjs, POSIX path literal in scripts/solheim-smoke/config.test.ts), both files byte-identical to upstream main and untouched here; solheim-smoke:check-types, test:agent-evals:unit, agent-evals:check-types, lifecycle:model-check, webview-recovery:check-types, webview-recovery:model-check and mcp:integration-check are clean. The core lane reports 194 files, 4260 passed | 9 skipped | 0 failed with no file collecting zero cases - a lane report, not the compile job. src/eslint-suppressions.json is untouched. ContextProxy.spec.ts declares 82 against the 81 this branch carried before the merge; ClineProvider.spec.ts is unchanged at 381 against the 375 floor. Answered rows carried by this merge: the ContextProxy.ts:754 and ContextProxy.spec.ts:188 pair moves to the merged text (the awaited settlement now sits at ContextProxy.ts:765).
2 parents 1d4f28e + 515cc02 commit 3f1983f

2 files changed

Lines changed: 62 additions & 0 deletions

File tree

‎src/core/config/ContextProxy.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,9 @@ export class ContextProxy {
408408
* pairing a reset cannot tell a settled-but-orphaned write from a live one.
409409
*/
410410
private readonly pendingSecretWrites = new Map<string, Set<{ settle: Promise<void> }>>()
411+
// A reset is in progress: the caches are meaningless mid-reset, and a secret write begun
412+
// inside it would commit after the delete pass and survive the erase.
413+
private resetting = false
411414
/** How many writes of each secret were started. Unlike the owners, it is not cleared when a write is over. */
412415
private readonly secretWriteCount = new Map<string, number>()
413416

@@ -461,6 +464,13 @@ export class ContextProxy {
461464
}
462465

463466
storeSecret(key: SecretStateKey, value?: string) {
467+
if (this.resetting) {
468+
// Refused rather than deferred: a deferred secret write would re-store exactly the
469+
// value the user asked to erase, and a silently dropped one would lose the save.
470+
return Promise.reject(
471+
new SettingsWriteError([], [key], new Error("secret writes are refused while state is being reset")),
472+
)
473+
}
464474
// Update cache.
465475
const owner = {}
466476
this.secretWriteOwner.set(key, owner)
@@ -746,6 +756,7 @@ export class ContextProxy {
746756
* @returns A promise that resolves when all reset operations are complete
747757
*/
748758
public async resetAllState() {
759+
this.resetting = true
749760
// A secret write that is in flight at this moment has already committed nothing, but it will
750761
// commit as soon as it settles, and the deletes below cannot un-commit a write that lands
751762
// after them. Wait for the pending writes first, then end the claims, then delete, so the
@@ -774,6 +785,7 @@ export class ContextProxy {
774785
])
775786

776787
await this.initialize()
788+
this.resetting = false
777789
}
778790

779791
private static _instance: ContextProxy | null = null

‎src/core/config/__tests__/ContextProxy.spec.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,56 @@ describe("ContextProxy", () => {
230230
expect(await mockSecrets.get("apiKey")).toBe("newer-secret")
231231
})
232232

233+
it("erases a secret whose write begins after the reset has snapshotted the pending writes", async () => {
234+
const durable = new Map<string, string | undefined>()
235+
let deleteCalls = 0
236+
let settleFirstDelete: () => void = () => {}
237+
let markDeletePassStarted: () => void = () => {}
238+
const deletePassStarted = new Promise<void>((resolve) => {
239+
markDeletePassStarted = resolve
240+
})
241+
mockSecrets.get.mockImplementation(async (key: string) => durable.get(key))
242+
mockSecrets.store.mockImplementation(async (key: string, value: string) => {
243+
durable.set(key, value)
244+
})
245+
// Park the reset inside its own delete pass so a write can begin in the window the in-flight
246+
// snapshot cannot cover. Only the first delete is held open; the rest settle normally, so
247+
// releasing the park lets the reset finish.
248+
mockSecrets.delete.mockImplementation((key: string) => {
249+
deleteCalls++
250+
durable.delete(key)
251+
if (deleteCalls === 1) {
252+
return new Promise<undefined>((resolve) => {
253+
markDeletePassStarted()
254+
settleFirstDelete = () => resolve(undefined)
255+
})
256+
}
257+
return Promise.resolve(undefined)
258+
})
259+
260+
const reset = proxy.resetAllState()
261+
await deletePassStarted
262+
263+
// The reset has taken its in-flight snapshot, cleared both caches and dropped both write
264+
// claims, and is standing inside the delete pass. This write is invisible to all of it.
265+
const late = proxy.storeSecret("apiKey", "late-secret")
266+
settleFirstDelete()
267+
await reset
268+
await late.catch(() => undefined)
269+
270+
// All three reads matter. Durable storage is what a reset is supposed to empty, the cache is
271+
// what this provider instance shows the user, and re-initializing is what a restarted
272+
// provider does - which is how a secret that survived the reset reaches the user after the
273+
// fact, so the middle read alone would understate the finding.
274+
expect(await mockSecrets.get("apiKey")).toBeUndefined()
275+
expect(proxy.getSecret("apiKey")).toBeUndefined()
276+
await proxy.initialize()
277+
expect(proxy.getSecret("apiKey")).toBeUndefined()
278+
// The caller is told: a write the reset cannot honour is refused rather than dropped, so
279+
// the save surfaces as a failure instead of vanishing.
280+
await expect(late).rejects.toThrow()
281+
})
282+
233283
it("should bypass cache for pass-through state keys", async () => {
234284
// Setup mock return value
235285
mockGlobalState.get.mockReturnValue("pass-through-value")

0 commit comments

Comments
 (0)