From c13bfef8aa1d1f04938da332b1dbeb1b62b52335 Mon Sep 17 00:00:00 2001 From: stack72 Date: Tue, 5 May 2026 00:57:31 +0100 Subject: [PATCH] fix(s3-datastore, gcs-datastore): exclude per-target FileLock files from cache sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend `isInternalCacheFile` to exclude any path with basename `.lock` (in addition to the existing top-level `.datastore.lock`). Without this, the self-healing `discoverIndexFromBucket` path added in #122 captures transient per-target lock files (`data////.lock`, written by the data tier's lock subsystem via S3 PUT/DELETE) into the synthesized index. The lock subsystem then deletes them on release, leaving the remote index referencing missing objects. A fresh reader running `datastore setup` 404s during hydration, fails to persist the `datastore:` block to `.swamp.yaml`, and the next `datastore sync --pull` falls back to the filesystem datastore and reports `Current datastore type: filesystem`. Surfaced by the swamp-uat sync_test.ts cross-repo pull tests (`sync pull retrieves data written by another repo …`) on @swamp/s3-datastore@2026.05.04.4. Reproduced end-to-end in a Linux container with a mock S3 server: writer runs a workflow (acquires the per-target lock, pushes data, releases the lock), reader's setup hits the 404 on the stale `.lock` index entry, `.swamp.yaml` never gets the datastore block, sync --pull surfaces the filesystem-fallback error. With the fix the reader hydrates 19 files cleanly and the pull succeeds. Mirrored in @swamp/gcs-datastore — same `discoverIndexFromBucket` path, same latent bug. Test plan: - 1 new unit test per extension pinning the `.lock` exclusion at any depth, plus negative cases (`data/@m/.locked.yaml`, `data/@m/lock`). - All existing s3 (45) + gcs (43) cache_sync tests still pass. - Linux+mock-S3 end-to-end repro (writer workflow run → reader setup → reader sync --pull) goes from `errors: ["…/.lock - 404 NoSuchKey"]` + filesystem-fallback to `filesPulled: 19` + a successful pull returning the writer's 4 data records. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../datastores/_lib/gcs_cache_sync.ts | 11 +++++++++ .../datastores/_lib/gcs_cache_sync_test.ts | 18 ++++++++++++++ datastore/gcs/manifest.yaml | 2 +- .../datastores/_lib/s3_cache_sync.ts | 19 +++++++++++++-- .../datastores/_lib/s3_cache_sync_test.ts | 24 +++++++++++++++++++ datastore/s3/manifest.yaml | 2 +- 6 files changed, 72 insertions(+), 4 deletions(-) diff --git a/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync.ts b/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync.ts index 264ac743..5b4c9406 100644 --- a/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync.ts +++ b/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync.ts @@ -84,6 +84,16 @@ const SYNC_STATE_FILE = ".datastore-sync-state.json"; * without it, `swamp datastore sync --push` would walk `_catalog.db*` * into `toPush`, SQLite would rewrite the WAL mid-upload, and the * push would fail on `_catalog.db-wal`. + * - basename `.lock` at any depth — per-target FileLock files written + * by the data tier's lock subsystem (e.g. + * `data////.lock`). The lock subsystem creates and + * deletes these directly via GCS Insert/Delete; they must not flow + * through cache sync because (a) the bucket listing in + * `discoverIndexFromBucket` would otherwise capture transient + * `.lock` files into the synthesized index, leaving the index + * referencing objects the lock subsystem deletes on release, and + * (b) a fresh reader hydrating from that stale index would 404 on + * the missing `.lock` and abort `datastore setup`. * * Exported for unit tests; not part of the public extension API. */ @@ -95,6 +105,7 @@ export function isInternalCacheFile(rel: string): boolean { return true; } const base = rel.split("/").pop() ?? ""; + if (base === ".lock") return true; return base === "_catalog.db" || base.startsWith("_catalog.db-"); } diff --git a/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync_test.ts b/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync_test.ts index 3052b332..e3db886c 100644 --- a/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync_test.ts +++ b/datastore/gcs/extensions/datastores/_lib/gcs_cache_sync_test.ts @@ -928,6 +928,24 @@ Deno.test("isInternalCacheFile: excludes the sync-state sidecar", () => { assertEquals(isInternalCacheFile("regular/file.yaml"), false); }); +Deno.test("isInternalCacheFile: excludes per-target FileLock files at any depth", () => { + // Data tier writes per-target locks at `data////.lock`. + // Without this exclusion, `discoverIndexFromBucket` captures transient + // `.lock` files into the synthesized index, the lock subsystem deletes + // them on release, and a fresh reader 404s on the missing object during + // setup hydration. + assertEquals( + isInternalCacheFile( + "data/command/shell/c19f88eb-de4f-4227-ade7-8162aec3d6a6/.lock", + ), + true, + ); + assertEquals(isInternalCacheFile("data/@m/.lock"), true); + assertEquals(isInternalCacheFile(".datastore.lock"), true); + assertEquals(isInternalCacheFile("data/@m/.locked.yaml"), false); + assertEquals(isInternalCacheFile("data/@m/lock"), false); +}); + // -- (2) post-verified pullChanged short-circuits with zero index GETs ---- Deno.test("pullChanged: post-verified second call hits fast path with zero index GETs", async () => { diff --git a/datastore/gcs/manifest.yaml b/datastore/gcs/manifest.yaml index 254fd619..1e7b4cb9 100644 --- a/datastore/gcs/manifest.yaml +++ b/datastore/gcs/manifest.yaml @@ -1,6 +1,6 @@ manifestVersion: 1 name: "@swamp/gcs-datastore" -version: "2026.05.04.4" +version: "2026.05.05.1" description: | Store data in a Google Cloud Storage bucket with local cache synchronization. Provides distributed locking via GCS generation-based preconditions and diff --git a/datastore/s3/extensions/datastores/_lib/s3_cache_sync.ts b/datastore/s3/extensions/datastores/_lib/s3_cache_sync.ts index eee167e3..c1d83d6c 100644 --- a/datastore/s3/extensions/datastores/_lib/s3_cache_sync.ts +++ b/datastore/s3/extensions/datastores/_lib/s3_cache_sync.ts @@ -80,9 +80,23 @@ const SYNC_STATE_FILE = ".datastore-sync-state.json"; * without it, `swamp datastore sync --push` would walk `_catalog.db*` * into `toPush`, SQLite would rewrite the WAL mid-upload, and the * push would fail on `_catalog.db-wal`. + * - basename `.lock` at any depth — per-target FileLock files written + * by the data tier's lock subsystem (e.g. + * `data////.lock`). The lock subsystem creates and + * deletes these directly via S3 PutObject/DeleteObject; they must + * not flow through cache sync because (a) the bucket listing in + * `discoverIndexFromBucket` would otherwise capture transient + * `.lock` files into the synthesized index, leaving the index + * referencing objects the lock subsystem deletes on release, and + * (b) a fresh reader hydrating from that stale index would 404 on + * the missing `.lock` and abort `datastore setup`. Manifests in CI + * as the reader's `datastore sync --pull` reporting "Current + * datastore type: filesystem" because setup fails to persist the + * datastore config to `.swamp.yaml`. * - * Uses basename matching for the catalog pattern so the filter is - * robust to any future change in the data tier subdirectory name. + * Uses basename matching for the catalog and `.lock` patterns so the + * filter is robust to any future change in the data tier subdirectory + * name. * * Exported for unit tests; not part of the public extension API. */ @@ -94,6 +108,7 @@ export function isInternalCacheFile(rel: string): boolean { return true; } const base = rel.split("/").pop() ?? ""; + if (base === ".lock") return true; return base === "_catalog.db" || base.startsWith("_catalog.db-"); } diff --git a/datastore/s3/extensions/datastores/_lib/s3_cache_sync_test.ts b/datastore/s3/extensions/datastores/_lib/s3_cache_sync_test.ts index 56222099..3352f80d 100644 --- a/datastore/s3/extensions/datastores/_lib/s3_cache_sync_test.ts +++ b/datastore/s3/extensions/datastores/_lib/s3_cache_sync_test.ts @@ -1466,6 +1466,30 @@ Deno.test("isInternalCacheFile: excludes the sync-state sidecar", () => { assertEquals(isInternalCacheFile(".datastore-sync-state.json"), true); }); +Deno.test("isInternalCacheFile: excludes per-target FileLock files at any depth", () => { + // The data tier writes per-target locks at `data////.lock` + // via the lock subsystem (PUT/DELETE direct to the bucket). Without this + // exclusion, `discoverIndexFromBucket` captures transient `.lock` files + // from the listing into the synthesized index, the lock subsystem then + // deletes them on release, and a fresh reader hydrating from that stale + // index 404s on the missing object — surfacing as `datastore setup` + // failing to persist `.swamp.yaml` and the next `datastore sync --pull` + // reporting "Current datastore type: filesystem". + assertEquals( + isInternalCacheFile( + "data/command/shell/c19f88eb-de4f-4227-ade7-8162aec3d6a6/.lock", + ), + true, + ); + assertEquals(isInternalCacheFile("data/@m/.lock"), true); + // The top-level distributed lock stays excluded (already covered by the + // exact-match branch, but the basename branch catches it too). + assertEquals(isInternalCacheFile(".datastore.lock"), true); + // Non-`.lock` data files are not affected. + assertEquals(isInternalCacheFile("data/@m/.locked.yaml"), false); + assertEquals(isInternalCacheFile("data/@m/lock"), false); +}); + // -- (2) post-verified pullChanged short-circuits with zero index GETs ---- Deno.test("pullChanged: post-verified second call hits fast path with zero index GETs", async () => { diff --git a/datastore/s3/manifest.yaml b/datastore/s3/manifest.yaml index 39dfcfe8..c8af4c0d 100644 --- a/datastore/s3/manifest.yaml +++ b/datastore/s3/manifest.yaml @@ -1,6 +1,6 @@ manifestVersion: 1 name: "@swamp/s3-datastore" -version: "2026.05.04.4" +version: "2026.05.05.1" description: | Store data in an Amazon S3 bucket with local cache synchronization. Provides distributed locking via S3 conditional writes and bidirectional