From bbeed2ccefb296cb8d0b315d5ea796cb20b64d57 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 21 Jan 2026 12:22:28 +0000 Subject: [PATCH] fix(cli): fix branch-specific item filtering to check if type is configured The previous logic incorrectly skipped branch-specific files when: - specificItems was configured but didn't include the item type - e.g., folders: ["f/**"] in config, but a folder.dev.meta.yaml file would be skipped because isSpecificItem returned false New logic with isItemTypeConfigured: - If item type is NOT configured in specificItems -> process normally - If item type IS configured but doesn't match pattern -> skip - If item type IS configured and matches pattern -> process Added tests for: - isItemTypeConfigured function - Filtering behavior with different config combinations - getSpecificItemsForCurrentBranch with branch override Co-Authored-By: Claude Opus 4.5 --- cli/src/commands/sync/sync.ts | 15 ++-- cli/src/core/specific_items.ts | 36 ++++++++++ cli/test/specific_items.test.ts | 118 ++++++++++++++++++++++++++++++-- 3 files changed, 157 insertions(+), 12 deletions(-) diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 0986b2c5c3..18d3661d73 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"; @@ -1394,9 +1395,11 @@ export async function elementsToMap( const currentBranch = branchOverride || getCurrentGitBranch()!; const basePath = fromBranchSpecificPath(path, currentBranch); - // If specificItems is configured, only process if it matches the pattern - if (specificItems && !isSpecificItem(basePath, specificItems)) { - // Branch-specific file doesn't match pattern, skip it + // Only apply specificItems filtering if the item TYPE is configured + // If the type is configured but doesn't match the pattern, skip it + // If the type is NOT configured, process the file normally + if (isItemTypeConfigured(basePath, specificItems) && !isSpecificItem(basePath, specificItems)) { + // Branch-specific file doesn't match the configured pattern, skip it continue; } @@ -1409,9 +1412,9 @@ export async function elementsToMap( // Skip base file, we already processed branch-specific version continue; } - // If specificItems is configured and this is a specific item, skip it - // (we should only use branch-specific versions for specific items) - if (specificItems && isSpecificItem(path, specificItems)) { + // Only skip base files if this item type IS configured as branch-specific + // AND it matches the pattern (meaning we expect a branch-specific version) + if (isSpecificItem(path, specificItems)) { continue; } map[path] = content; diff --git a/cli/src/core/specific_items.ts b/cli/src/core/specific_items.ts index 30ac828375..64709c909a 100644 --- a/cli/src/core/specific_items.ts +++ b/cli/src/core/specific_items.ts @@ -135,6 +135,42 @@ 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 is different from isSpecificItem which checks if it MATCHES the pattern + */ +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..03a1e6267c 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,111 @@ Deno.test("round-trip: settings with sanitized branch", () => { const restored = fromBranchSpecificPath(branchSpecific, branch); assertEquals(restored, original); }); + +// ============================================================================= +// isItemTypeConfigured TESTS +// ============================================================================= + +Deno.test("isItemTypeConfigured: returns true when folders is configured", () => { + const config: SpecificItemsConfig = { folders: ["f/**"] }; + assertEquals(isItemTypeConfigured("f/test/folder.meta.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false when folders is NOT configured", () => { + const config: SpecificItemsConfig = { variables: ["f/**"] }; + assertEquals(isItemTypeConfigured("f/test/folder.meta.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true when variables is configured", () => { + const config: SpecificItemsConfig = { variables: ["f/**"] }; + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false when variables is NOT configured", () => { + const config: SpecificItemsConfig = { folders: ["f/**"] }; + assertEquals(isItemTypeConfigured("f/test.variable.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns true when settings is configured", () => { + const config: SpecificItemsConfig = { settings: true }; + assertEquals(isItemTypeConfigured("settings.yaml", config), true); +}); + +Deno.test("isItemTypeConfigured: returns false when settings is NOT configured", () => { + const config: SpecificItemsConfig = { folders: ["f/**"] }; + assertEquals(isItemTypeConfigured("settings.yaml", config), false); +}); + +Deno.test("isItemTypeConfigured: returns false for undefined config", () => { + assertEquals(isItemTypeConfigured("f/test/folder.meta.yaml", undefined), false); +}); + +// ============================================================================= +// Branch-specific item filtering behavior tests +// ============================================================================= + +Deno.test("filtering: folder type configured and matches - should process branch-specific file", () => { + const config: SpecificItemsConfig = { folders: ["f/**"] }; + const basePath = "f/compliance/folder.meta.yaml"; + + // Type is configured AND matches pattern = should process + const typeConfigured = isItemTypeConfigured(basePath, config); + const matchesPattern = isSpecificItem(basePath, config); + + assertEquals(typeConfigured, true); + assertEquals(matchesPattern, true); + // Skip condition: typeConfigured && !matchesPattern = false (should NOT skip) + assertEquals(typeConfigured && !matchesPattern, false); +}); + +Deno.test("filtering: folder type configured but does NOT match - should skip branch-specific file", () => { + const config: SpecificItemsConfig = { folders: ["f/other/**"] }; + const basePath = "f/compliance/folder.meta.yaml"; + + // Type is configured but does NOT match pattern = should skip + const typeConfigured = isItemTypeConfigured(basePath, config); + const matchesPattern = isSpecificItem(basePath, config); + + assertEquals(typeConfigured, true); + assertEquals(matchesPattern, false); + // Skip condition: typeConfigured && !matchesPattern = true (should skip) + assertEquals(typeConfigured && !matchesPattern, true); +}); + +Deno.test("filtering: folder type NOT configured - should process branch-specific file", () => { + const config: SpecificItemsConfig = { variables: ["f/**"] }; + const basePath = "f/compliance/folder.meta.yaml"; + + // Type is NOT configured = should process (not subject to specificItems rules) + const typeConfigured = isItemTypeConfigured(basePath, config); + + assertEquals(typeConfigured, false); + // Skip condition: typeConfigured && !matchesPattern = false (should NOT skip) + assertEquals(typeConfigured && !isSpecificItem(basePath, config), false); +}); + +Deno.test("filtering: getSpecificItemsForCurrentBranch returns correct config", () => { + const config = { + gitBranches: { + dev: { + specificItems: { + folders: ["f/**"] + } + }, + prod: { + specificItems: { + folders: ["f/prod/**"] + } + } + } + }; + + const devItems = getSpecificItemsForCurrentBranch(config as any, "dev"); + assertEquals(devItems?.folders, ["f/**"]); + + const prodItems = getSpecificItemsForCurrentBranch(config as any, "prod"); + assertEquals(prodItems?.folders, ["f/prod/**"]); + + const unknownItems = getSpecificItemsForCurrentBranch(config as any, "unknown"); + assertEquals(unknownItems, undefined); +});