diff --git a/.vscode/settings.json b/.vscode/settings.json index dbb05b4..c01085e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,11 +1,11 @@ { "chat.modeFilesLocations": { - ".github/chatmodes": true + "chatmodes": true }, "chat.promptFilesLocations": { - ".github/prompts": true + "prompts": true }, "chat.instructionsFilesLocations": { - ".github/instructions": true + "instructions": true } -} \ No newline at end of file +} diff --git a/apply-config.js b/apply-config.js index b1e5c65..9000d35 100755 --- a/apply-config.js +++ b/apply-config.js @@ -101,25 +101,10 @@ async function applyConfig(configPath = "awesome-copilot.config.yml") { }; // Import config manager for effective state computation - const { computeEffectiveItemStates } = require("./config-manager"); + const { getEffectivelyEnabledItems } = require("./config-manager"); - // Compute effective states using precedence rules - const effectiveStates = computeEffectiveItemStates(config); - - // Create sets of effectively enabled items for performance - const effectivelyEnabledSets = { - prompts: new Set(), - instructions: new Set(), - chatmodes: new Set() - }; - - for (const section of ["prompts", "instructions", "chatmodes"]) { - for (const [itemName, state] of Object.entries(effectiveStates[section])) { - if (state.enabled) { - effectivelyEnabledSets[section].add(itemName); - } - } - } + // Get precomputed sets of effectively enabled items for O(1) performance + const effectivelyEnabledSets = getEffectivelyEnabledItems(config); // Count enabled collections for summary if (config.collections) { diff --git a/config-manager.js b/config-manager.js index 0fa1a68..d074fd1 100644 --- a/config-manager.js +++ b/config-manager.js @@ -101,6 +101,11 @@ function toBoolean(value) { if (normalized === "false") return false; } + // Preserve undefined as undefined for "no explicit override" + if (value === undefined) { + return undefined; + } + return Boolean(value); } @@ -185,11 +190,17 @@ function generateConfigHash(config) { * Compute effective item states respecting explicit overrides over collections * * This function builds membership maps per section and returns effectively enabled items with reasons. - * It uses the following precedence rules: - * 1. Explicit true/false overrides everything (highest priority) + * It uses strict comparisons to ensure undefined values are never treated as explicitly disabled. + * + * Precedence rules with strict undefined handling: + * 1. Explicit true/false overrides everything (highest priority) - uses strict === comparisons * 2. If undefined and enabled by collection, use true * 3. Otherwise, use false (disabled) * + * CRITICAL: Only values that are strictly === false are treated as explicitly disabled. + * undefined, null, 0, '', or other falsy values are NOT treated as explicit disabling. + * This allows collections to enable items that are not explicitly configured. + * * @param {Object} config - Configuration object with sections * @returns {Object} Effective states for each section with { itemName: { enabled: boolean, reason: string } } * Reason can be: 'explicit', 'collection', or 'disabled' @@ -248,10 +259,13 @@ function computeEffectiveItemStates(config) { const explicitValue = sectionConfig[itemName]; const isEnabledByCollection = collectionEnabled.has(itemName); - // Precedence rules: - // 1. If explicitly set to true or false, use that value + // Precedence rules with strict undefined handling: + // 1. If explicitly set to true or false, use that value (highest priority) // 2. If undefined and enabled by collection, use true - // 3. Otherwise, use false + // 3. Otherwise, use false (disabled) + // + // IMPORTANT: Only strict === false comparisons are used to determine explicit disabling. + // undefined values are NEVER treated as explicitly disabled, allowing collections to enable them. let enabled = false; let reason = "disabled"; @@ -260,9 +274,11 @@ function computeEffectiveItemStates(config) { enabled = true; reason = "explicit"; } else if (explicitValue === false) { + // Strict comparison ensures only explicit false disables items enabled = false; reason = "explicit"; } else if (explicitValue === undefined && isEnabledByCollection) { + // undefined values can be enabled by collections (not treated as disabled) enabled = true; reason = "collection"; } diff --git a/test-effective-states.js b/test-effective-states.js index 621f04b..4755d7c 100644 --- a/test-effective-states.js +++ b/test-effective-states.js @@ -164,7 +164,50 @@ function runTests() { 'Chat mode should be enabled by collection'); }); - // Test 8: getEffectivelyEnabledItems returns Sets format + // Test 9: TASK-006 - Strict false checks prevent undefined treated as disabled + test("TASK-006: Strict false checks prevent undefined treated as disabled", () => { + const config = { + prompts: { + 'explicit-false-item': false, + 'explicit-true-item': true, + // 'undefined-item' is undefined (not set) + }, + collections: { + 'testing-automation': true + } + }; + const result = computeEffectiveItemStates(config); + + // Explicit false should be disabled with reason 'explicit' + const explicitFalse = result.prompts['explicit-false-item']; + if (explicitFalse) { + assert(explicitFalse.reason === 'explicit' && !explicitFalse.enabled, + 'Items with explicit false should be disabled with reason explicit'); + } + + // Explicit true should be enabled with reason 'explicit' + const explicitTrue = result.prompts['explicit-true-item']; + if (explicitTrue) { + assert(explicitTrue.reason === 'explicit' && explicitTrue.enabled, + 'Items with explicit true should be enabled with reason explicit'); + } + + // Undefined item in collection should be enabled with reason 'collection' + const undefinedInCollection = result.prompts['playwright-generate-test']; + if (undefinedInCollection) { + assert(undefinedInCollection.reason === 'collection' && undefinedInCollection.enabled, + 'Undefined items should be enabled by collections, not treated as explicitly disabled'); + } + + // Undefined item NOT in collection should be disabled with reason 'disabled' (not 'explicit') + const undefinedNotInCollection = result.prompts['some-random-item-not-in-any-collection']; + if (undefinedNotInCollection) { + assert(undefinedNotInCollection.reason === 'disabled' && !undefinedNotInCollection.enabled, + 'Undefined items not in collections should have reason disabled, not explicit'); + } + }); + + // Test 10: getEffectivelyEnabledItems returns Sets format test("getEffectivelyEnabledItems returns Sets format", () => { const config = { prompts: { @@ -214,6 +257,37 @@ function runTests() { assert(result.prompts.size > 0, 'Should have enabled prompts'); }); + // Test 10: Undefined values are not treated as explicitly disabled (TASK-004) + test("Undefined values are not treated as explicitly disabled", () => { + const config = { + prompts: { + 'playwright-generate-test': true, // explicit true + 'csharp-nunit': false, // explicit false + // 'playwright-explore-website' is undefined (not mentioned) + }, + collections: { + 'testing-automation': true + } + }; + + const result = computeEffectiveItemStates(config); + + // Explicit true should be explicit + const explicitTrue = result.prompts['playwright-generate-test']; + assert(explicitTrue && explicitTrue.enabled && explicitTrue.reason === 'explicit', + 'Explicit true should be enabled with explicit reason'); + + // Explicit false should be explicit (strict === false comparison) + const explicitFalse = result.prompts['csharp-nunit']; + assert(explicitFalse && !explicitFalse.enabled && explicitFalse.reason === 'explicit', + 'Explicit false should be disabled with explicit reason'); + + // Undefined should inherit from collection + const undefinedItem = result.prompts['playwright-explore-website']; + assert(undefinedItem && undefinedItem.enabled && undefinedItem.reason === 'collection', + 'Undefined items should inherit from collection, not be treated as explicitly disabled'); + }); + console.log(`\nTest Results: ${passedTests}/${totalTests} passed`); if (passedTests === totalTests) {