Fix extends resolution behavior

Author: noxify

.changeset/fix-extends-resolution.md

@@ -0,0 +1,23 @@
+---
+"@noxify/gitlab-ci-builder": patch
+---
+
+Fixed extends resolution behavior with `resolveTemplatesOnly: true`
+
+Previously, when `resolveTemplatesOnly: true` was set (the default), ALL extends were removed after merging, including normal jobs and remote references that should have been preserved.
+
+**Old behavior (incorrect):**
+
+- Templates (`.prefix`) were merged ✅
+- Normal jobs (without `.`) were merged ❌ (should stay in extends)
+- Remote jobs were merged ❌ (should stay in extends)
+- Unknown/external jobs were merged ❌ (should stay in extends)
+
+**New behavior (correct):**
+
+- Templates (`.prefix`) are merged ✅
+- Normal jobs (without `.`) remain in extends ✅
+- Remote jobs remain in extends ✅
+- Unknown/external jobs remain in extends ✅
+
+This fix enables proper GitLab CI template composition patterns, particularly for shallow jobs that use `remote: true` to reference jobs from other configurations without merging them.

package.json

@@ -51,7 +51,7 @@
     "js-yaml": "4.1.1",
     "tinyglobby": "0.2.15",
     "typescript": "5.9.3",
-    "zod": "4.1.12"
+    "zod": "4.1.13"
   },
   "devDependencies": {
     "@changesets/cli": "2.29.7",
@@ -67,10 +67,10 @@
     "eslint-plugin-package-json": "0.85.0",
     "json-schema-to-typescript": "15.0.4",
     "jsonc-eslint-parser": "2.4.1",
-    "memfs": "^4.51.0",
+    "memfs": "4.51.0",
     "prettier": "3.6.2",
     "tsdown": "0.16.6",
-    "tsx": "^4.20.6",
+    "tsx": "4.20.6",
     "typescript-eslint": "8.47.0",
     "vitest": "4.0.13"
   },

pnpm-lock.yaml

@@ -79,20 +79,66 @@ export function resolveExtends(
     // Start with empty definition
     let mergedDef: JobDefinitionNormalized = {}
 
+    // Collect all extends that should be kept (not merged)
+    const keptExtends: string[] = []
+
+    // Recursively collect non-template extends from merged templates
+    const collectNonTemplateExtends = (extendName: string, visited = new Set<string>()): void => {
+      if (visited.has(extendName)) return
+      visited.add(extendName)
+
+      const targetName = graph.has(extendName) ? extendName : `.${extendName}`
+      const targetNode = graph.get(targetName)
+
+      if (!targetNode?.extends) return
+
+      for (const nestedExtend of targetNode.extends) {
+        const nestedTargetName = graph.has(nestedExtend) ? nestedExtend : `.${nestedExtend}`
+        const nestedNode = graph.get(nestedTargetName)
+
+        if (!nestedNode) {
+          // Unknown extend, keep it
+          if (!keptExtends.includes(nestedExtend)) {
+            keptExtends.push(nestedExtend)
+          }
+        } else if (nestedNode.isRemote) {
+          // Remote extend, keep it
+          if (!keptExtends.includes(nestedExtend)) {
+            keptExtends.push(nestedExtend)
+          }
+        } else if (!nestedTargetName.startsWith(".")) {
+          // Normal job (not template), keep it
+          if (!keptExtends.includes(nestedExtend)) {
+            keptExtends.push(nestedExtend)
+          }
+        } else {
+          // It's a template, recurse into it
+          collectNonTemplateExtends(nestedExtend, visited)
+        }
+      }
+    }
+
     // Merge extends chain (ALWAYS resolve for merging, regardless of mergeExtends)
     if (node.extends.length > 0) {
       for (const extendName of node.extends) {
         // Try with and without dot prefix
         const targetName = graph.has(extendName) ? extendName : `.${extendName}`
         const targetNode = graph.get(targetName)
 
-        if (!targetNode) continue
+        if (!targetNode) {
+          // Unknown target, keep in extends
+          keptExtends.push(extendName)
+          continue
+        }
 
         // Check if we should merge this extend
         const shouldMerge = resolveTemplatesOnly ? targetName.startsWith(".") : true
 
-        // Skip remote extends
-        if (targetNode.isRemote) continue
+        // Skip remote extends from merging
+        if (targetNode.isRemote) {
+          keptExtends.push(extendName)
+          continue
+        }
 
         if (shouldMerge) {
           // Use fully resolved definition (without extends field) for merging
@@ -102,6 +148,14 @@ export function resolveExtends(
           } else {
             mergedDef = mergeJobDefinitions(mergedDef, targetNode.definition)
           }
+
+          // If resolveTemplatesOnly and this is a template, collect non-template extends from it
+          if (resolveTemplatesOnly && targetName.startsWith(".")) {
+            collectNonTemplateExtends(extendName)
+          }
+        } else {
+          // Don't merge, but keep in extends (normal job when resolveTemplatesOnly: true)
+          keptExtends.push(extendName)
         }
       }
     }
@@ -119,10 +173,21 @@ export function resolveExtends(
       continue
     }
 
-    // Clean up extends field
-    const cleanedDef = cleanExtendsField(finalDef, node, graph, globalOptions, jobOpts)
+    // Add kept extends to final definition (or remove extends entirely if none kept)
+    let outputDef: JobDefinitionOutput
+    if (keptExtends.length > 0) {
+      if (keptExtends.length === 1) {
+        outputDef = { ...finalDef, extends: keptExtends[0] }
+      } else {
+        outputDef = { ...finalDef, extends: keptExtends }
+      }
+    } else {
+      // Remove extends field entirely if nothing to keep
+      const { extends: _extends, ...rest } = finalDef
+      outputDef = rest as JobDefinitionOutput
+    }
 
-    resolved.set(name, cleanedDef)
+    resolved.set(name, outputDef)
   }
 
   // Convert Map back to Record
@@ -138,61 +203,3 @@ export function resolveExtends(
     skippedChecks: context.skippedChecks,
   }
 }
-
-/**
- * Clean extends field after resolution
- * Remove local extends, keep only remote/external ones
- */
-function cleanExtendsField(
-  definition: JobDefinitionNormalized,
-  node: { extends: string[]; isRemote: boolean },
-  graph: Map<string, { isRemote: boolean }>,
-  globalOptions: GlobalOptions,
-  jobOpts?: JobOptions,
-): JobDefinitionOutput {
-  const mergeExtends = jobOpts?.mergeExtends ?? globalOptions.mergeExtends
-
-  if (mergeExtends === false) {
-    // Keep extends as-is, just optimize to string if single entry
-    if (definition.extends?.length === 1) {
-      return {
-        ...definition,
-        extends: definition.extends[0],
-      } as JobDefinitionOutput
-    }
-    return definition as JobDefinitionOutput
-  }
-
-  // Filter extends to keep only remote/external ones
-  if (definition.extends && definition.extends.length > 0) {
-    const filtered = definition.extends.filter((extendName) => {
-      const targetName = graph.has(extendName) ? extendName : `.${extendName}`
-      const targetNode = graph.get(targetName)
-
-      // Keep if remote or not found in local graph
-      return !targetNode || targetNode.isRemote
-    })
-
-    if (filtered.length === 0) {
-      // Remove extends field entirely
-      const { extends: _extends, ...rest } = definition
-      return rest as JobDefinitionOutput
-    }
-
-    if (filtered.length === 1) {
-      // Optimize to string
-      return {
-        ...definition,
-        extends: filtered[0],
-      } as JobDefinitionOutput
-    }
-
-    // Keep as array
-    return {
-      ...definition,
-      extends: filtered,
-    } as JobDefinitionOutput
-  }
-
-  return definition as JobDefinitionOutput
-}

src/resolution/resolver.ts

@@ -54,8 +54,7 @@ describe("ConfigBuilder - edge cases and branches", () => {
 
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const consumer = out.jobs.consumer!
-    // Local `.local` should be removed from extends (because it refers to a local template),
-    // while `external` remains since it does not match any local job id.
+    // Local `.local` template is merged, only external unknown job remains in extends
     // Single extends is optimized to string format
     expect(consumer.extends).toEqual("external")
   })

tests/unit/config.edgecases.test.ts

@@ -55,7 +55,7 @@ describe("ConfigBuilder - extends", () => {
       paths: ["node_modules/"],
     })
     expect(testJob?.script).toEqual(["npm test"])
-    // Local extends are resolved and removed from output by default
+    // Template extends are resolved and removed from output
     expect(testJob?.extends).toBeUndefined()
   })
 

tests/unit/extends.test.ts

@@ -44,7 +44,8 @@ describe("Job Options", () => {
         script: ["template"],
         stage: "test",
       })
-      expect(result.jobs?.child?.extends).toBeUndefined()
+      // resolveTemplatesOnly: true means templates are merged but normal jobs remain in extends
+      expect(result.jobs?.child?.extends).toBe("basejob")
     })
 
     it("should allow job-level override of resolveTemplatesOnly", () => {
@@ -62,6 +63,7 @@ describe("Job Options", () => {
         script: ["template", "job"],
         stage: "test",
       })
+      // resolveTemplatesOnly: false means both templates and jobs are merged, no extends left
       expect(result.jobs?.child?.extends).toBeUndefined()
     })
   })
@@ -76,6 +78,7 @@ describe("Job Options", () => {
         script: ["base command"],
         stage: "test",
       })
+      // Template extends are fully resolved and removed
       expect(result.jobs?.child?.extends).toBeUndefined()
     })
 
@@ -130,6 +133,7 @@ describe("Job Options", () => {
         script: ["base command"],
         stage: "deploy",
       })
+      // Template extends are fully resolved when mergeExtends is true
       expect(result.jobs?.enabled?.extends).toBeUndefined()
     })
   })
@@ -260,6 +264,7 @@ describe("Job Options", () => {
 
       // job2: local override mergeExtends=true
       expect(result.jobs?.job2?.script).toEqual(["base"])
+      // Template extends are fully resolved
       expect(result.jobs?.job2?.extends).toBeUndefined()
 
       // job3: global mergeExisting=false, replaced

tests/unit/job-options.test.ts

@@ -148,7 +148,7 @@ describe("ConfigBuilder - Real-world extends scenario", () => {
       optional: true,
     })
 
-    // No extends in final output
+    // Templates are resolved, extends removed
     expect(job?.extends).toBeUndefined()
   })
 

tests/unit/real-world-extends.test.ts

@@ -76,7 +76,7 @@ describe("ConfigBuilder - template extends chain resolution", () => {
     // Should have stage from job definition
     expect(job?.stage).toBe("review")
 
-    // Should not have extends in final output (all resolved)
+    // Templates are resolved, extends removed
     expect(job?.extends).toBeUndefined()
   })