From e6e33d5a879f923d9640bc98b6f27d76dab2f243 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 21 Jan 2026 12:38:40 +0000 Subject: [PATCH] fix(cli): skip branch-specific files when type is not configured (#7643) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a type (folders, settings, variables, resources, triggers) is NOT configured in specificItems, branch-specific files of that type should be ignored and only base files used. Added isItemTypeConfigured() function to distinguish between: - Type not configured → skip branch-specific file, use base file - Type configured but doesn't match pattern → skip branch-specific file - Type configured and matches → use branch-specific file Added comprehensive tests to prevent regression. Co-authored-by: Claude Opus 4.5 --- cli/src/commands/sync/sync.ts | 55 ++--- cli/src/core/specific_items.ts | 37 ++++ cli/test/specific_items.test.ts | 354 +++++++++++++++++++++++++++++++- 3 files changed, 416 insertions(+), 30 deletions(-) diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 43e408603e..ed4569cb13 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -53,6 +53,7 @@ import { getSpecificItemsForCurrentBranch, isBranchSpecificFile, isCurrentBranchFile, + isItemTypeConfigured, isSpecificItem, SpecificItemsConfig, } from "../../core/specific_items.ts"; @@ -1415,32 +1416,38 @@ export async function elementsToMap( } // Handle branch-specific path mapping after all filtering - if (specificItems) { - if (isCurrentBranchFile(path, branchOverride)) { - // This is a branch-specific file for current branch - // Safe to compute branch here since isCurrentBranchFile already validated it exists - const currentBranch = branchOverride || getCurrentGitBranch()!; - const basePath = fromBranchSpecificPath(path, currentBranch); - if (isSpecificItem(basePath, specificItems)) { - // Map to base path for push operations - map[basePath] = content; - processedBasePaths.add(basePath); - } else { - // Branch-specific file doesn't match pattern, skip it - continue; - } - } else if (!isBranchSpecificFile(path)) { - // This is a regular base file, check if we should skip it - if (processedBasePaths.has(path)) { - // Skip base file, we already processed branch-specific version - continue; - } - map[path] = content; + if (isCurrentBranchFile(path, branchOverride)) { + // This is a branch-specific file for current branch + const currentBranch = branchOverride || getCurrentGitBranch()!; + const basePath = fromBranchSpecificPath(path, currentBranch); + + // Only use branch-specific files if the item type IS configured as branch-specific + // AND matches the pattern. Otherwise, skip and use base file instead. + if (!isItemTypeConfigured(basePath, specificItems)) { + // Type not configured as branch-specific - skip, use base file instead + continue; } - } else { - // No specific items configuration, use regular path - map[entry.path] = content; + if (!isSpecificItem(basePath, specificItems)) { + // Type configured but doesn't match pattern - skip + continue; + } + + // Type configured AND matches - map to base path + map[basePath] = content; + processedBasePaths.add(basePath); + } else if (!isBranchSpecificFile(path)) { + // This is a regular base file + if (processedBasePaths.has(path)) { + // Skip base file, we already processed branch-specific version + continue; + } + // Skip base file if it's configured as branch-specific (expect branch version) + if (isSpecificItem(path, specificItems)) { + continue; + } + map[path] = content; } + // Note: branch-specific files for other branches are already filtered out earlier } return map; } diff --git a/cli/src/core/specific_items.ts b/cli/src/core/specific_items.ts index 30ac828375..e0484fb53a 100644 --- a/cli/src/core/specific_items.ts +++ b/cli/src/core/specific_items.ts @@ -135,6 +135,43 @@ function matchesPatterns(path: string, patterns: string[]): boolean { return patterns.some(pattern => minimatch(path, pattern)); } +/** + * Check if the item type for a given path is configured in specificItems. + * This checks if the TYPE is configured, not whether it matches the pattern. + * Used to determine if branch-specific files should be used for this type. + */ +export function isItemTypeConfigured(path: string, specificItems: SpecificItemsConfig | undefined): boolean { + if (!specificItems) { + return false; + } + + if (path.endsWith('.variable.yaml')) { + return specificItems.variables !== undefined; + } + + if (path.endsWith('.resource.yaml')) { + return specificItems.resources !== undefined; + } + + if (isTriggerFile(path)) { + return specificItems.triggers !== undefined; + } + + if (path.endsWith('/folder.meta.yaml')) { + return specificItems.folders !== undefined; + } + + if (path === 'settings.yaml') { + return specificItems.settings !== undefined; + } + + if (isFileResource(path)) { + return specificItems.resources !== undefined; + } + + return false; +} + /** * Check if a file path should be treated as branch-specific */ diff --git a/cli/test/specific_items.test.ts b/cli/test/specific_items.test.ts index e0d6782fa6..c3ddaff990 100644 --- a/cli/test/specific_items.test.ts +++ b/cli/test/specific_items.test.ts @@ -8,9 +8,13 @@ import { assertEquals, assertExists, assert } from "https://deno.land/std@0.224. // Import the functions we need to test import { isSpecificItem, + isItemTypeConfigured, toBranchSpecificPath, fromBranchSpecificPath, isBranchSpecificFile, + isCurrentBranchFile, + getBranchSpecificPath, + getSpecificItemsForCurrentBranch, } from "../src/core/specific_items.ts"; import type { SpecificItemsConfig } from "../src/core/specific_items.ts"; @@ -212,12 +216,6 @@ Deno.test("round-trip: resource file with extension", () => { // These tests validate that functions work correctly with explicit branch override // ============================================================================= -import { - getBranchSpecificPath, - isCurrentBranchFile, - getSpecificItemsForCurrentBranch, -} from "../src/core/specific_items.ts"; - Deno.test("branchOverride: getBranchSpecificPath with override returns branch-specific path", () => { // This test verifies that when branchOverride is provided, the function uses it // instead of detecting the current git branch @@ -513,3 +511,347 @@ Deno.test("round-trip: settings with sanitized branch", () => { const restored = fromBranchSpecificPath(branchSpecific, branch); assertEquals(restored, original); }); + +// ============================================================================= +// isItemTypeConfigured TESTS +// This function checks if the TYPE is configured, not whether it matches pattern. +// Used to determine if branch-specific files should be used for this type. +// ============================================================================= + +Deno.test("isItemTypeConfigured: returns false when specificItems is undefined", () => { + assertEquals(isItemTypeConfigured("f/test.variable.yaml", undefined), false); + assertEquals(isItemTypeConfigured("f/test.resource.yaml", undefined), false); + assertEquals(isItemTypeConfigured("f/folder/folder.meta.yaml", undefined), false); + assertEquals(isItemTypeConfigured("settings.yaml", undefined), false); +}); + +Deno.test("isItemTypeConfigured: returns true for variables when variables is configured", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + // Type is configured (even if path doesn't match the pattern) + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), true); + assertEquals(isItemTypeConfigured("g/other.variable.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false for variables when variables is NOT configured", () => { + const config: SpecificItemsConfig = { + resources: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true for resources when resources is configured", () => { + const config: SpecificItemsConfig = { + resources: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/test.resource.yaml", config), true); + assertEquals(isItemTypeConfigured("g/other.resource.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false for resources when resources is NOT configured", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/test.resource.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true for triggers when triggers is configured", () => { + const config: SpecificItemsConfig = { + triggers: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/my.http_trigger.yaml", config), true); + assertEquals(isItemTypeConfigured("f/my.kafka_trigger.yaml", config), true); + assertEquals(isItemTypeConfigured("g/other.websocket_trigger.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false for triggers when triggers is NOT configured", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/my.http_trigger.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true for folders when folders is configured", () => { + const config: SpecificItemsConfig = { + folders: ["f/env_*"], + }; + // Type is configured (even if path doesn't match the pattern) + assertEquals(isItemTypeConfigured("f/env_staging/folder.meta.yaml", config), true); + assertEquals(isItemTypeConfigured("f/other/folder.meta.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false for folders when folders is NOT configured", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/my_folder/folder.meta.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true for settings when settings is configured (true)", () => { + const config: SpecificItemsConfig = { + settings: true, + }; + assertEquals(isItemTypeConfigured("settings.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns true for settings when settings is configured (false)", () => { + // settings: false still means the type is "configured" (explicitly disabled) + const config: SpecificItemsConfig = { + settings: false, + }; + assertEquals(isItemTypeConfigured("settings.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false for settings when settings is NOT configured", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + assertEquals(isItemTypeConfigured("settings.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true for resource files (with extension) when resources is configured", () => { + const config: SpecificItemsConfig = { + resources: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/config.resource.file.json", config), true); + assertEquals(isItemTypeConfigured("f/data.resource.file.ini", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false for resource files when resources is NOT configured", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + assertEquals(isItemTypeConfigured("f/config.resource.file.json", config), false); +}); + +// ============================================================================= +// BRANCH-SPECIFIC FILE FILTERING TESTS +// These tests verify the expected filtering behavior: +// - When type IS configured: use branch-specific files, skip base files +// - When type is NOT configured: skip branch-specific files, use base files +// ============================================================================= + +Deno.test("filtering logic: folders - when NOT configured, branch-specific should be ignored", () => { + // Config has variables but NOT folders + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + + const basePath = "f/my_folder/folder.meta.yaml"; + const branchSpecificPath = "f/my_folder/folder.main.meta.yaml"; + + // Folder type is NOT configured + assertEquals(isItemTypeConfigured(basePath, config), false); + + // Therefore, branch-specific file detection should not apply to this type + // The sync logic should: + // 1. Skip branch-specific folder files (isBranchSpecificFile returns true) + // 2. Use the base file + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(isBranchSpecificFile(basePath), false); +}); + +Deno.test("filtering logic: folders - when IS configured and matches, use branch-specific", () => { + const config: SpecificItemsConfig = { + folders: ["f/my_folder"], + }; + + const basePath = "f/my_folder/folder.meta.yaml"; + const branchSpecificPath = "f/my_folder/folder.main.meta.yaml"; + + // Folder type IS configured + assertEquals(isItemTypeConfigured(basePath, config), true); + + // And path matches the pattern + assertEquals(isSpecificItem(basePath, config), true); + + // The sync logic should: + // 1. Use branch-specific folder file (map to base path) + // 2. Skip the base file + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(fromBranchSpecificPath(branchSpecificPath, "main"), basePath); +}); + +Deno.test("filtering logic: folders - when IS configured but doesn't match, skip branch-specific", () => { + const config: SpecificItemsConfig = { + folders: ["f/env_*"], // Only env_ folders are branch-specific + }; + + const basePath = "f/other_folder/folder.meta.yaml"; + const branchSpecificPath = "f/other_folder/folder.main.meta.yaml"; + + // Folder type IS configured + assertEquals(isItemTypeConfigured(basePath, config), true); + + // But this path doesn't match the pattern + assertEquals(isSpecificItem(basePath, config), false); + + // The sync logic should: + // 1. Skip the branch-specific file (type configured but doesn't match) + // 2. Use the base file +}); + +Deno.test("filtering logic: settings - when NOT configured, branch-specific should be ignored", () => { + // Config has variables but NOT settings + const config: SpecificItemsConfig = { + variables: ["f/**"], + }; + + const basePath = "settings.yaml"; + const branchSpecificPath = "settings.main.yaml"; + + // Settings type is NOT configured + assertEquals(isItemTypeConfigured(basePath, config), false); + + // Therefore, branch-specific file detection should not apply to this type + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(isBranchSpecificFile(basePath), false); +}); + +Deno.test("filtering logic: settings - when IS configured (true), use branch-specific", () => { + const config: SpecificItemsConfig = { + settings: true, + }; + + const basePath = "settings.yaml"; + const branchSpecificPath = "settings.main.yaml"; + + // Settings type IS configured + assertEquals(isItemTypeConfigured(basePath, config), true); + + // And settings: true means it matches + assertEquals(isSpecificItem(basePath, config), true); + + // The sync logic should use branch-specific file + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(fromBranchSpecificPath(branchSpecificPath, "main"), basePath); +}); + +Deno.test("filtering logic: settings - when IS configured (false), skip branch-specific", () => { + // settings: false means type is configured but explicitly disabled + const config: SpecificItemsConfig = { + settings: false, + }; + + const basePath = "settings.yaml"; + const branchSpecificPath = "settings.main.yaml"; + + // Settings type IS configured (even though value is false) + assertEquals(isItemTypeConfigured(basePath, config), true); + + // But settings: false means it doesn't match (not a specific item) + assertEquals(isSpecificItem(basePath, config), false); + + // The sync logic should skip branch-specific file and use base +}); + +Deno.test("filtering logic: variables - when NOT configured, branch-specific should be ignored", () => { + // Config has folders but NOT variables + const config: SpecificItemsConfig = { + folders: ["f/env_*"], + }; + + const basePath = "f/test.variable.yaml"; + const branchSpecificPath = "f/test.main.variable.yaml"; + + // Variable type is NOT configured + assertEquals(isItemTypeConfigured(basePath, config), false); + + // Branch-specific variable files should be ignored + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(isBranchSpecificFile(basePath), false); +}); + +Deno.test("filtering logic: resources - when NOT configured, branch-specific should be ignored", () => { + // Config has folders but NOT resources + const config: SpecificItemsConfig = { + folders: ["f/env_*"], + }; + + const basePath = "f/db.resource.yaml"; + const branchSpecificPath = "f/db.main.resource.yaml"; + + // Resource type is NOT configured + assertEquals(isItemTypeConfigured(basePath, config), false); + + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(isBranchSpecificFile(basePath), false); +}); + +Deno.test("filtering logic: triggers - when NOT configured, branch-specific should be ignored", () => { + // Config has folders but NOT triggers + const config: SpecificItemsConfig = { + folders: ["f/env_*"], + }; + + const basePath = "f/webhook.http_trigger.yaml"; + const branchSpecificPath = "f/webhook.main.http_trigger.yaml"; + + // Trigger type is NOT configured + assertEquals(isItemTypeConfigured(basePath, config), false); + + assertEquals(isBranchSpecificFile(branchSpecificPath), true); + assertEquals(isBranchSpecificFile(basePath), false); +}); + +// ============================================================================= +// MIXED CONFIGURATION TESTS +// Tests for configs that have some types configured but not others +// ============================================================================= + +Deno.test("mixed config: only folders configured - other types use base files", () => { + const config: SpecificItemsConfig = { + folders: ["f/env_*"], + }; + + // Folders IS configured + assertEquals(isItemTypeConfigured("f/env_staging/folder.meta.yaml", config), true); + assertEquals(isSpecificItem("f/env_staging/folder.meta.yaml", config), true); + + // Variables, resources, triggers, settings are NOT configured + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), false); + assertEquals(isItemTypeConfigured("f/db.resource.yaml", config), false); + assertEquals(isItemTypeConfigured("f/hook.http_trigger.yaml", config), false); + assertEquals(isItemTypeConfigured("settings.yaml", config), false); +}); + +Deno.test("mixed config: only settings configured - other types use base files", () => { + const config: SpecificItemsConfig = { + settings: true, + }; + + // Settings IS configured + assertEquals(isItemTypeConfigured("settings.yaml", config), true); + assertEquals(isSpecificItem("settings.yaml", config), true); + + // Other types are NOT configured + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), false); + assertEquals(isItemTypeConfigured("f/db.resource.yaml", config), false); + assertEquals(isItemTypeConfigured("f/hook.http_trigger.yaml", config), false); + assertEquals(isItemTypeConfigured("f/my_folder/folder.meta.yaml", config), false); +}); + +Deno.test("mixed config: variables and folders configured - resources and triggers use base", () => { + const config: SpecificItemsConfig = { + variables: ["f/**"], + folders: ["f/env_*"], + }; + + // Variables IS configured + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), true); + assertEquals(isSpecificItem("f/test.variable.yaml", config), true); + + // Folders IS configured (path matches) + assertEquals(isItemTypeConfigured("f/env_staging/folder.meta.yaml", config), true); + assertEquals(isSpecificItem("f/env_staging/folder.meta.yaml", config), true); + + // Folders IS configured but path doesn't match + assertEquals(isItemTypeConfigured("f/other/folder.meta.yaml", config), true); + assertEquals(isSpecificItem("f/other/folder.meta.yaml", config), false); + + // Resources and triggers are NOT configured + assertEquals(isItemTypeConfigured("f/db.resource.yaml", config), false); + assertEquals(isItemTypeConfigured("f/hook.http_trigger.yaml", config), false); + assertEquals(isItemTypeConfigured("settings.yaml", config), false); +});