Filed by the domain:services PM seat (#6021). Surfaced by the #12981 batch-7 dev inside the exact lines that batch edits, and correctly not fixed there: both are behaviour changes beyond "repair the silence", and neither has an authority pinning the correct shape. Re-measured by this seat on origin/main before filing — the readings below are mine, not transcribed.
Where
packages/services/service-storage/src/storage-service-plugin.ts, the settings.registerAction('storage', 'test', …) handler (:521–:560 at the time of writing; ⚠️ re-locate by symbol — #12981 batch 7 shifts these lines).
The probe writes a key minted per call:
constprobeKey=`__objectstack_probe__/${Date.now()}-${Math.random().toString(36).slice(2,10)}`;Nothing persists that name. Once the frame ends, the only record of what was written is gone.
Defect 1 — the cleanup deletes from a different store than the one written to
The handler exists so an operator can validate unsaved credentials. When the form posts any values it therefore builds a temporary adapter and writes to that:
lettarget: IStorageService=proxy;if(merged&&Object.keys(merged).length>0){target=awaitthis.buildAdapterFromValues(merged);// ← a DIFFERENT store}awaittarget.upload(probeKey,probeBytes,{contentType: 'text/plain'});But the failure cleanup deletes from proxy, the persisted adapter:
}catch(err: any){// Best-effort cleanuptry{await(proxyasIStorageService).delete(probeKey);}catch{/* ignore */}⇒ On a failed test with edited credentials — the case the whole temporary-adapter branch exists for — the probe object leaks in the store it was written to, while a delete is issued against a store that never held it. The delete may well "succeed" (deleting a nonexistent key is a no-op on both shipped adapters), so nothing looks wrong.
The healthy path has no such confusion: :549 deletes from target. Only the failure path reaches for proxy.
Defect 2 — the content-mismatch return path cleans up nothing
awaittarget.upload(probeKey,probeBytes,{contentType: 'text/plain'});constgot=awaittarget.download(probeKey);if(!got||!Buffer.isBuffer(got)||got.toString('utf-8')!==probeBytes.toString('utf-8')){return{ok: false,severity: 'error',message: 'Probe download did not match upload.'};// ← no delete}awaittarget.delete(probeKey);The upload already succeeded — that is the precondition for reaching the comparison at all — so the object is definitely there, and this return walks past the delete on the very next line. ⇒ A guaranteed leak, not a best-effort one, on a path that by construction only fires when the adapter is misbehaving (a store that mangles round-trips is exactly the store you least want accumulating orphans).
Unlike defect 1 this path does not even reach the catch, so #12981 batch 7's repair — which makes a refused cleanup name the key it left behind — does not cover it: there is no refused cleanup, because none is attempted.
Consequence
__objectstack_probe__/… objects accrue in whichever store the probe actually wrote to, one per failed test, each under a name nothing recorded. There is no sweep for that prefix. It is not a data-exposure defect — the content is probe@<timestamp> — but it is unbounded litter in customer buckets produced by a button whose entire purpose is to be pressed repeatedly while credentials are being got right.
⛔ Not established here
- ⛔ No claim about which fix is correct. Deleting from
target in the catch is the obvious shape for defect 1, but target may be the thing that failed to construct — the assignment sits inside the try, so at the catch it can still be proxy, and a delete against a half-built adapter is its own question. Defect 2 could be a finally, a delete before the return, or a deliberate "leave it for inspection" — ⛔ nothing in the tree rules that. - ⛔ No claim that a sweep should exist for the
__objectstack_probe__/ prefix. That is a product question. - ⛔ Not measured: whether any deployment has actually accumulated these. Per-deployment object stores are not readable from this repo.
Acceptance
- The failure cleanup deletes from the store the probe was written to, or states in code why it cannot.
- The content-mismatch path either cleans up or records, in code, why the object is deliberately left.
- Both directions pinned: a failed probe with edited credentials leaves nothing behind in the temporary adapter, and a failed probe without them leaves nothing behind in the persisted one. ⚠️ A single-direction pin passes on the current code for defect 1 — the no-overrides case already deletes from the right store, because there
target === proxy.
Related
#12981 (the durability worklist that surfaced it; batch 7's PR #13725 repairs the silence of the refused cleanup and deliberately leaves these two alone) · #13547 / #13528 (the other service-storage cards this round).
Filed by the
domain:servicesPM seat (#6021). Surfaced by the #12981 batch-7 dev inside the exact lines that batch edits, and correctly not fixed there: both are behaviour changes beyond "repair the silence", and neither has an authority pinning the correct shape. Re-measured by this seat onorigin/mainbefore filing — the readings below are mine, not transcribed.Where
packages/services/service-storage/src/storage-service-plugin.ts, thesettings.registerAction('storage', 'test', …)handler (:521–:560at the time of writing;The probe writes a key minted per call:
Nothing persists that name. Once the frame ends, the only record of what was written is gone.
Defect 1 — the cleanup deletes from a different store than the one written to
The handler exists so an operator can validate unsaved credentials. When the form posts any values it therefore builds a temporary adapter and writes to that:
But the failure cleanup deletes from
proxy, the persisted adapter:⇒ On a failed test with edited credentials — the case the whole temporary-adapter branch exists for — the probe object leaks in the store it was written to, while a delete is issued against a store that never held it. The delete may well "succeed" (deleting a nonexistent key is a no-op on both shipped adapters), so nothing looks wrong.
The healthy path has no such confusion:
:549deletes fromtarget. Only the failure path reaches forproxy.Defect 2 — the content-mismatch return path cleans up nothing
The upload already succeeded — that is the precondition for reaching the comparison at all — so the object is definitely there, and this
returnwalks past the delete on the very next line. ⇒ A guaranteed leak, not a best-effort one, on a path that by construction only fires when the adapter is misbehaving (a store that mangles round-trips is exactly the store you least want accumulating orphans).Unlike defect 1 this path does not even reach the
catch, so #12981 batch 7's repair — which makes a refused cleanup name the key it left behind — does not cover it: there is no refused cleanup, because none is attempted.Consequence
__objectstack_probe__/…objects accrue in whichever store the probe actually wrote to, one per failed test, each under a name nothing recorded. There is no sweep for that prefix. It is not a data-exposure defect — the content isprobe@<timestamp>— but it is unbounded litter in customer buckets produced by a button whose entire purpose is to be pressed repeatedly while credentials are being got right.⛔ Not established here
targetin the catch is the obvious shape for defect 1, buttargetmay be the thing that failed to construct — the assignment sits inside thetry, so at the catch it can still beproxy, and a delete against a half-built adapter is its own question. Defect 2 could be afinally, a delete before the return, or a deliberate "leave it for inspection" — ⛔ nothing in the tree rules that.__objectstack_probe__/prefix. That is a product question.Acceptance
target === proxy.Related
#12981 (the durability worklist that surfaced it; batch 7's PR #13725 repairs the silence of the refused cleanup and deliberately leaves these two alone) · #13547 / #13528 (the other
service-storagecards this round).