From db825fc7af5848bf8c0668e2fd5fc293713ca5c4 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 3 Jun 2026 07:19:01 -0700 Subject: [PATCH] fix(sync): don't route the remote-http persistent transcript dir through cleanup (#1802) The ingest finally ran cleanupStagingDir() unconditionally, but in remote-http mode stagingDir is the PERSISTENT transcript dir (~/.gstack/transcripts/) that gstack-brain-sync push must consume. The remote-http branch documents the intent to skip cleanup, but a finally runs on its return. Gate the call on !remoteHttpMode so the ownership guard only ever sees .staging-ingest-* dirs. Pre-gate this dir was deleted outright (broken artifacts handoff); post-#1827 it produced a false 'prevent data loss' warning every sync. Co-Authored-By: Claude Opus 4.8 (1M context) --- bin/gstack-memory-ingest.ts | 10 ++++++- ...regression-1611-gbrain-sync-resume.test.ts | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/bin/gstack-memory-ingest.ts b/bin/gstack-memory-ingest.ts index fd7e53a73..67e885ef2 100644 --- a/bin/gstack-memory-ingest.ts +++ b/bin/gstack-memory-ingest.ts @@ -1778,7 +1778,15 @@ async function ingestPass(args: CliArgs): Promise { ); } } finally { - cleanupStagingDir(stagingDir); + // #1802 D1: in remote-http mode `stagingDir` is the PERSISTENT transcript + // dir (makePersistentTranscriptDir, under ~/.gstack/transcripts/) that + // gstack-brain-sync push must pick up — it is NOT a `.staging-ingest-*` dir + // and must never be deleted here. The remote-http branch above already + // documents this intent ("Skip the ... cleanupStagingDir paths"), but a + // `finally` runs on its `return`, so the gate has to live here. Gating on + // mode (rather than widening the ownership guard) keeps checkOwnedStagingDir + // strict: it only ever sees `.staging-ingest-*` dirs. + if (!remoteHttpMode) cleanupStagingDir(stagingDir); _activeStagingDir = null; } diff --git a/test/regression-1611-gbrain-sync-resume.test.ts b/test/regression-1611-gbrain-sync-resume.test.ts index 481d3284c..1f262a596 100644 --- a/test/regression-1611-gbrain-sync-resume.test.ts +++ b/test/regression-1611-gbrain-sync-resume.test.ts @@ -350,3 +350,30 @@ describe("#1802 checkOwnedStagingDir — ownership matrix", () => { expect(sync).toMatch(/checkOwnedStagingDir\(stagingDir, gstackHome\)/); }); }); + +// ── #1802 D1: remote-http persistent dir must never hit cleanupStagingDir ─── +// In remote-http mode `stagingDir` is the PERSISTENT transcript dir +// (makePersistentTranscriptDir, under ~/.gstack/transcripts/) that +// gstack-brain-sync push consumes. The finally runs on the remote-http `return`, +// so the cleanup call there must be gated on `!remoteHttpMode` — otherwise the +// guard refuses it on every sync (false "prevent data loss" warning) and, pre- +// gate, the dir was deleted outright (broken artifacts handoff). +describe("#1802 D1 — remote-http finally gate (static invariant)", () => { + const ingest = fs.readFileSync( + path.join(ROOT, "bin", "gstack-memory-ingest.ts"), + "utf-8", + ); + + test("finally gates cleanupStagingDir on !remoteHttpMode", () => { + expect(ingest).toMatch(/if \(!remoteHttpMode\) cleanupStagingDir\(stagingDir\)/); + }); + + test("the only finally-scoped cleanup call is the gated one", () => { + // Locate the finally block and assert it does not contain a bare + // `cleanupStagingDir(stagingDir);` that would run regardless of mode. + const finallyAt = ingest.lastIndexOf("} finally {"); + expect(finallyAt).toBeGreaterThan(-1); + const finallySlice = ingest.slice(finallyAt, finallyAt + 800); + expect(finallySlice).not.toMatch(/^\s*cleanupStagingDir\(stagingDir\);/m); + }); +});