Ver código fonte

fix(include): detach inserted rows so re-applying a patch list mounts the same tree

`applyEntryPatches` pushed a patch's `insert` rows into the entry list by
reference, so a later patch that inserted into a group an earlier patch had
inserted mutated the patch's own object. With bundle rows now inserted into
their group by a separate patch, the Loader's rollback of a rejected update
re-applied the previous include config over those mutated objects and hit
`duplicate loader entry id`; the rollback failed and the rejection reason
named it instead of the row that broke. Inserted rows are now cloned before
they join the list, as the function's detachment contract already stated;
the vendored change is logged and pinned by an application-twice test.
Yichen Jiang 1 mês atrás
pai
commit
8357838bc5

+ 14 - 0
packages/boot/app-boot/tests/external-bundles.spec.ts

@@ -128,6 +128,20 @@ describe('composeExternalLayer', () => {
     expect(patches).toEqual(snapshot)
   })
 
+  it('mounts the same tree when its patches are applied twice, as a rolled-back update re-applies them', () => {
+    const composed = composeExternalLayer(layer('pkg-t', [
+      { insert: [{ id: 'row', name: 'pkg-t' }] },
+      { id: 'tools', insert: [{ id: 'tool', name: 'pkg-t/tool' }] },
+    ]))
+    const snapshot = structuredClone(composed.patches)
+    const base = (): EntryOptions[] => [{ id: 'tools', name: 'cordis:group', group: true, config: [] }]
+    const first = applyEntryPatches(base(), composed.patches, () => {})
+    const second = applyEntryPatches(base(), composed.patches, () => {})
+    expect(second).toEqual(first)
+    expect(composed.patches).toEqual(snapshot)
+    expect((first[1]?.config as EntryOptions[]).map(row => row.id)).toEqual(['row'])
+  })
+
   it('spells the group id without the Loader\'s nested-id separator', () => {
     expect(bundleGroupId('@scope/pkg')).toBe('bundle/@scope/pkg')
     expect(bundleGroupId('x')).not.toContain(':')

+ 1 - 0
vendor/README.md

@@ -50,6 +50,7 @@ Keep this log exhaustive — every divergence from upstream must be listed.
 18. **Entry `disabled` interpolation in `loader/src/config/entry.ts`**: a `disabled: !!js` expression evaluates against the loader context at every mount decision; the raw node stays in the options, so write-back keeps the `!!js` form. `disabled` is the only interpolated metadata field. Covered by `packages/boot/app-boot/tests/user-patches.spec.ts` and `apps/cli/tests/windows-shell.spec.ts`.
 19. **`loader/src/internal.ts` runtime shape detection**: `ModuleLoader.fromInternal()` classifies the internal loader by which module-job API it owns — `getOrCreateModuleJob` for v2, `getModuleJobForImport` for v1 — instead of by Node major version. Upstream tags every major `>= 24` as v2, but the v2 interface arrived in Node 24.12.0, so 24.0–24.11.1 report major 24 while still carrying the v1 loader; consumers then called `resolveSync` with reversed parameters and every call threw. `dsh web` served an empty client graph (`__DSH_BOOT__.entries: []`) and HMR partial reload resolved no entry URL, both behind swallowed or warn-level errors. Arity cannot discriminate the two shapes, because each reports `resolveSync.length === 2`. A loader owning neither API is left unclassified rather than guessed, so consumers take their documented no-internals path. Covered on the `node-compat` Node version matrix, which pins 24.9 for the mistagged range.
 20. **`loader/src/config/entry.ts` typed update errors**: the per-row `failed to <stage> loader entry` wrapper is an exported `EntryUpdateError` carrying its `stage`, so a consumer that classifies a row failure reads the field instead of parsing the message. The message text is unchanged. Covered by `packages/boot/app-boot/tests/contained-group.spec.ts`.
+21. **`include/src/index.ts` detached inserts**: `applyEntryPatches` clones inserted rows before adding them to the list, so a later patch that inserts into or overrides a row an earlier patch inserted mutates the copy and applying one patch list twice yields the same tree — the Loader re-applies the previous include config when it rolls a rejected update back, and an aliased insert row accumulated its children on every application. Covered by `packages/boot/app-boot/tests/external-bundles.spec.ts`.
 
 ## Sync procedure
 

+ 12 - 6
vendor/include/src/index.ts

@@ -47,9 +47,14 @@ function retryableWriteError(error: unknown): boolean {
  * is never mutated and the result is always detached from it (even with no
  * patches): patching or mounting shared entry objects would bake earlier
  * values into the cached parse, so repeated application (config hot-reloads)
- * could never revert a removed or changed patch. Inserted entries are indexed
- * as they are added, so a later patch in the same list can target a row an
- * earlier patch inserted. A patch that matches nothing warns and is skipped.
+ * could never revert a removed or changed patch. Inserted rows are cloned
+ * before they join the list for the same reason: a later patch that inserts
+ * into or overrides a row an earlier patch inserted mutates the copy, so
+ * applying one patch list twice (the Loader rolling a rejected update back to
+ * the previous one) yields the same tree each time. Inserted entries are
+ * indexed as they are added, so a later patch in the same list can target a
+ * row an earlier patch inserted. A patch that matches nothing warns and is
+ * skipped.
  * @param data - the parsed entry list (JSON-safe plain data).
  * @param patches - the patch list to apply, in order.
  * @param warn - sink for skipped-patch diagnostics (printf-style, `%C` = code).
@@ -78,6 +83,7 @@ export function applyEntryPatches(
     const { id, insert, name, ...overrides } = patch
 
     if (insert) {
+      const inserted = structuredClone(insert)
       if (id) {
         const target = entryMap.get(id)
         if (!target) {
@@ -89,16 +95,16 @@ export function applyEntryPatches(
           continue
         }
         if (!Array.isArray(target.config)) target.config = []
-        target.config.push(...insert)
+        target.config.push(...inserted)
       } else {
-        data.push(...insert)
+        data.push(...inserted)
       }
       // Index what this patch added so a LATER patch in the same list can
       // target it. Patch lists compose one layer per source (each bundle
       // layer, then the user's, then `--patch` overlays), and a layer must be
       // able to configure or disable a row an earlier layer inserted; without
       // this, inserted rows were silently unpatchable.
-      buildMap(insert)
+      buildMap(inserted)
       continue
     }