fix(cloud-connection): an install-local reseed over an intact baseline answers success with the skipped count

.changeset/21776-reseed-intact-baseline.md

@@ -0,0 +1,11 @@
+---
+"@objectstack/cloud-connection": minor
+---
+
+An install-local reseed over sample rows that are all still in place answers success, with the loader's `skipped` count, instead of a refusal naming a false cause (#21776).
+
+Clause-②: yes (widening)
+
+- **Intact baseline.** `POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data`, run while every seed record the package declares is already present, answers `200 { success: true, data: { manifestId, inserted: 0, updated: 0, skipped: N, errors: 0, withSampleData: true } }`. Before, it answered `422 RESEED_NO_ROWS`, "Reseed wrote no rows. The package declares no seedable records for this runtime.", over a package that declares them. The reseed is idempotent, so a run that finds every row in place has reached its goal. The install's record of sample data is set the same way as when rows land.
+- **`skipped` on every success.** A successful reseed now answers all four of the loader's counts: `inserted`, `updated`, `skipped` and `errors`. Before, `skipped` was not in the response.
+- **Unchanged refusals.** `422 RESEED_NO_ROWS` still answers a run that wrote nothing because records failed, with the error count and the first error, and its `details` are still `{ inserted, updated, errors }`. It also still answers, with the same text, a run in which the loader had no record to process for this runtime: for example, every dataset is scoped to another environment (`Seed.env`). That text now states a true cause. A package with no seed dataset at all still answers `400 RESEED_SKIPPED` (`no-datasets`).

packages/cloud-connection/src/marketplace-install-local-plugin.ts

@@ -1843,6 +1843,14 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
      *   • A purge was undone
      *   • The user wants a clean baseline back after editing demo rows
      *
+     * A success answers the loader's four counts — `inserted`, `updated`,
+     * `skipped`, `errors` — so the caller sees why nothing changed. A run over
+     * an intact baseline (every declared record already present, so all of
+     * them `skipped`) is a success with `inserted: 0`: being idempotent, the
+     * reseed has reached the state it exists to reach. `422 RESEED_NO_ROWS` is
+     * kept for a run that wrote nothing because records failed (`errors`), or
+     * because the loader had no record to process for this runtime.
+     *
      * Multi-tenant: requires an active organization on the session (same
      * rule as install seed path). A walled session with none is refused with
      * ADR-0123 D2 / D4's answer ({@link noActiveOrganizationRefusal}); the
@@ -1888,16 +1896,25 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
 
         const inserted = summary.seeded.inserted ?? 0;
         const updated = summary.seeded.updated ?? 0;
+        const skipped = summary.seeded.skipped ?? 0;
         const errors = summary.seeded.errors ?? 0;
         const wrote = inserted + updated > 0;
+        // The loader reconciles `inserted + updated + skipped + errored`
+        // against the records it processes for this runtime (`SeedLoadResult`
+        // in `@objectstack/spec/data`), so a clean run with nothing written and
+        // `skipped > 0` found every one of them already present: the intact
+        // baseline, which is a success. With `skipped` at 0 as well, it
+        // processed no record at all.
+        const intactBaseline = !wrote && errors === 0 && skipped > 0;
 
         // HONEST RESULT: the loader runs row-by-row and counts write failures
         // (locked DB, missing table, validation reject) into `errors` rather
         // than throwing. Previously this handler returned success — and flipped
         // `withSampleData` to true — even when every row failed, so the UI said
         // "done" while the database stayed empty. Treat a run that landed no
-        // rows as a failure and report why.
-        if (!wrote) {
+        // rows as a failure and report why — unless every row it would have
+        // written is already there.
+        if (!wrote && !intactBaseline) {
             return c.json({
                 success: false,
                 error: {
@@ -1910,7 +1927,8 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
             }, 422);
         }
 
-        // Only mark the install as carrying sample data once rows actually landed.
+        // Only mark the install as carrying sample data once its rows are
+        // there: landed by this run, or found already present by it.
         try {
             entry.withSampleData = true;
             entry.sampleDataPurged = false;
@@ -1923,6 +1941,7 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
                 manifestId,
                 inserted,
                 updated,
+                skipped,
                 errors,
                 withSampleData: true,
             },

packages/cloud-connection/src/marketplace-install-local-reseed.test.ts

@@ -11,6 +11,13 @@
  * stayed empty (the "提示成功但没有数据" bug). These tests pin the corrected
  * behaviour: no rows written => failure + flag stays false; rows written =>
  * success + flag flips.
+ *
+ * …and the converse: a reseed over an INTACT baseline — every declared record
+ * already present, so the loader skips all of them — wrote nothing because
+ * there was nothing to write. It used to answer `422 RESEED_NO_ROWS` "The
+ * package declares no seedable records for this runtime" over a package that
+ * declares 28. It is a success carrying `skipped`, and that refusal text is
+ * kept for the run that really processed no record.
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
@@ -19,8 +26,8 @@ import { join } from 'node:path';
 import { tmpdir } from 'node:os';
 
 // Controls what the (mocked) seed loader reports back. The handler under test
-// only cares about result.summary.total{Inserted,Updated} + result.errors.
-let seedResult: any = { summary: { totalInserted: 0, totalUpdated: 0 }, errors: [] };
+// only cares about result.summary.total{Inserted,Updated,Skipped} + result.errors.
+let seedResult: any = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [] };
 
 vi.mock('@objectstack/runtime', () => ({
     SeedLoaderService: class {
@@ -100,7 +107,7 @@ const MANIFEST = {
 let dir: string;
 beforeEach(() => {
     dir = mkdtempSync(join(tmpdir(), 'mil-reseed-'));
-    seedResult = { summary: { totalInserted: 0, totalUpdated: 0 }, errors: [] };
+    seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [] };
 });
 afterEach(() => { rmSync(dir, { recursive: true, force: true }); vi.restoreAllMocks(); });
 
@@ -118,62 +125,110 @@ async function installAndGetRoutes() {
     return rawApp;
 }
 
+const RESEED = 'POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data';
+/** The ledger's install-wide record of sample data, read from the ledger itself. */
+const recordedWithSampleData = () => new LocalManifestSource(dir).read('app.test.proj').entry?.withSampleData;
+
 describe('reseed honest result', () => {
     it('FAILS (422) when the seed run wrote zero rows but errored', async () => {
         const rawApp = await installAndGetRoutes();
         seedResult = {
             summary: { totalInserted: 0, totalUpdated: 0 },
             errors: [{ message: 'database is locked' }, { message: 'database is locked' }],
         };
-        const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
-            makeC({}, 'app.test.proj'),
-        );
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.proj'));
         expect(res.status).toBe(422);
         expect(res.payload?.success).toBe(false);
         expect(res.payload?.error?.code).toBe('RESEED_NO_ROWS');
         // The real failure reason is surfaced, not swallowed.
         expect(res.payload?.error?.message).toContain('database is locked');
-        expect(res.payload?.error?.details).toMatchObject({ inserted: 0, updated: 0, errors: 2 });
+        expect(res.payload?.error?.details).toEqual({ inserted: 0, updated: 0, errors: 2 });
+        expect(recordedWithSampleData()).toBe(false);
     });
 
-    it('FAILS (422) when the package seeds nothing (0 rows, 0 errors)', async () => {
+    it('FAILS (422) unchanged when records errored beside skipped ones and nothing was written', async () => {
         const rawApp = await installAndGetRoutes();
-        seedResult = { summary: { totalInserted: 0, totalUpdated: 0 }, errors: [] };
-        const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
-            makeC({}, 'app.test.proj'),
-        );
+        seedResult = {
+            summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 1 },
+            errors: [{ message: 'one row rejected' }],
+        };
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.proj'));
         expect(res.status).toBe(422);
+        expect(res.payload?.success).toBe(false);
         expect(res.payload?.error?.code).toBe('RESEED_NO_ROWS');
+        expect(res.payload?.error?.message).toContain('one row rejected');
+        expect(res.payload?.error?.details).toEqual({ inserted: 0, updated: 0, errors: 1 });
+        expect(recordedWithSampleData()).toBe(false);
     });
 
-    it('SUCCEEDS and flips withSampleData when rows actually land', async () => {
+    it('FAILS (422) with its own text when the loader processed no record for this runtime (0 rows, 0 skipped, 0 errors)', async () => {
+        const rawApp = await installAndGetRoutes();
+        seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [] };
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.proj'));
+        expect(res.status).toBe(422);
+        expect(res.payload?.success).toBe(false);
+        expect(res.payload?.error?.code).toBe('RESEED_NO_ROWS');
+        expect(res.payload?.error?.message).toBe(
+            'Reseed wrote no rows. The package declares no seedable records for this runtime.',
+        );
+        expect(res.payload?.error?.details).toEqual({ inserted: 0, updated: 0, errors: 0 });
+        expect(recordedWithSampleData()).toBe(false);
+    });
+
+    it('a package with no seed dataset at all never reaches the loader: 400 RESEED_SKIPPED, unchanged', async () => {
         const rawApp = await installAndGetRoutes();
-        seedResult = { summary: { totalInserted: 2, totalUpdated: 0 }, errors: [] };
-        const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
-            makeC({}, 'app.test.proj'),
+        const { data: _none, ...noData } = MANIFEST;
+        const installRes = await rawApp.routes.get('POST /api/v1/marketplace/install-local')!(
+            makeC({ manifest: { ...noData, id: 'app.test.nodata' } }),
         );
+        expect(installRes.payload?.success).toBe(true);
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.nodata'));
+        expect(res.status).toBe(400);
+        expect(res.payload?.success).toBe(false);
+        expect(res.payload?.error?.code).toBe('RESEED_SKIPPED');
+        expect(res.payload?.error?.message).toContain('no-datasets');
+    });
+
+    it('SUCCEEDS (200) over an intact baseline: every declared record already present, all of them skipped', async () => {
+        const rawApp = await installAndGetRoutes();
+        expect(recordedWithSampleData()).toBe(false);
+        seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 2 }, errors: [] };
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.proj'));
+        expect(res.status).toBe(200);
+        expect(res.payload).toEqual({
+            success: true,
+            data: { manifestId: 'app.test.proj', inserted: 0, updated: 0, skipped: 2, errors: 0, withSampleData: true },
+        });
+        // The rows are there, so the install-wide record says so.
+        expect(recordedWithSampleData()).toBe(true);
+    });
+
+    it('SUCCEEDS and flips withSampleData when rows actually land', async () => {
+        const rawApp = await installAndGetRoutes();
+        seedResult = { summary: { totalInserted: 2, totalUpdated: 0, totalSkipped: 0 }, errors: [] };
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.proj'));
         expect(res.status).toBe(200);
         expect(res.payload?.success).toBe(true);
-        expect(res.payload?.data).toMatchObject({ inserted: 2, updated: 0, withSampleData: true });
+        expect(res.payload?.data).toEqual({
+            manifestId: 'app.test.proj', inserted: 2, updated: 0, skipped: 0, errors: 0, withSampleData: true,
+        });
 
         // The ledger's install-time record flips. Read from the ledger itself:
         // the GET listing no longer serves this record — it answers from the
         // caller's own rows (#21775), and the seed loader here is a stub that
         // writes none.
-        expect(new LocalManifestSource(dir).read('app.test.proj').entry?.withSampleData).toBe(true);
+        expect(recordedWithSampleData()).toBe(true);
     });
 
     it('partial success (some rows + some errors) still reports the error count', async () => {
         const rawApp = await installAndGetRoutes();
         seedResult = {
-            summary: { totalInserted: 1, totalUpdated: 0 },
+            summary: { totalInserted: 1, totalUpdated: 0, totalSkipped: 1 },
             errors: [{ message: 'one row rejected' }],
         };
-        const res = await rawApp.routes.get('POST /api/v1/marketplace/install-local/:manifestId/reseed-sample-data')!(
-            makeC({}, 'app.test.proj'),
-        );
+        const res = await rawApp.routes.get(RESEED)!(makeC({}, 'app.test.proj'));
         expect(res.status).toBe(200);
         expect(res.payload?.success).toBe(true);
-        expect(res.payload?.data?.errors).toBe(1);
+        expect(res.payload?.data).toMatchObject({ inserted: 1, skipped: 1, errors: 1 });
     });
 });