diff --git a/.github/workflows/smoke-claude-on-copilot.lock.yml b/.github/workflows/smoke-claude-on-copilot.lock.yml index 3afd09a0d35..8a25e6c66d0 100644 --- a/.github/workflows/smoke-claude-on-copilot.lock.yml +++ b/.github/workflows/smoke-claude-on-copilot.lock.yml @@ -1379,8 +1379,7 @@ jobs: needs: - activation - agent - if: > - always() && needs.agent.result != 'skipped' && (needs.agent.outputs.output_types != '' || needs.agent.outputs.has_patch == 'true') + if: always() && needs.agent.result != 'skipped' runs-on: ubuntu-latest permissions: contents: read diff --git a/actions/setup/js/validate_memory_files.cjs b/actions/setup/js/validate_memory_files.cjs index 1043b749b79..194faeb5bf4 100644 --- a/actions/setup/js/validate_memory_files.cjs +++ b/actions/setup/js/validate_memory_files.cjs @@ -20,9 +20,7 @@ const path = require("path"); * @returns {ValidationResult} Validation result with list of invalid files */ function validateMemoryFiles(memoryDir, memoryType = "cache", allowedExtensions) { - const allowAll = !allowedExtensions?.length; - - if (allowAll) { + if (!allowedExtensions?.length) { core.info(`All file extensions are allowed in ${memoryType}-memory directory`); return { valid: true, invalidFiles: [] }; } @@ -33,6 +31,7 @@ function validateMemoryFiles(memoryDir, memoryType = "cache", allowedExtensions) } const extensions = new Set(allowedExtensions.map(ext => ext.trim().toLowerCase())); + /** @type {string[]} */ const invalidFiles = []; /** @@ -50,9 +49,7 @@ function validateMemoryFiles(memoryDir, memoryType = "cache", allowedExtensions) if (entry.isDirectory()) { // Skip .git directory — it is git metadata used for integrity branching // and contains files with no extension (e.g. HEAD, ORIG_HEAD, packed-refs). - if (entry.name === ".git") { - continue; - } + if (entry.name === ".git") continue; scanDirectory(fullPath, relativeFilePath); } else if (entry.isFile()) { const ext = path.extname(entry.name).toLowerCase(); diff --git a/actions/setup/js/validate_memory_files.test.cjs b/actions/setup/js/validate_memory_files.test.cjs index 093ba883789..a3943a153e3 100644 --- a/actions/setup/js/validate_memory_files.test.cjs +++ b/actions/setup/js/validate_memory_files.test.cjs @@ -1,18 +1,18 @@ // @ts-check -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import fs from "fs"; import path from "path"; import os from "os"; const { validateMemoryFiles } = require("./validate_memory_files.cjs"); -// Mock core globally +// Mock core globally with vi.fn() so we can assert on calls global.core = { - info: () => {}, - error: () => {}, - warning: () => {}, - debug: () => {}, + info: vi.fn(), + error: vi.fn(), + warning: vi.fn(), + debug: vi.fn(), }; describe("validateMemoryFiles", () => { @@ -21,6 +21,7 @@ describe("validateMemoryFiles", () => { beforeEach(() => { // Create a temporary directory for testing tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "validate-memory-test-")); + vi.resetAllMocks(); }); afterEach(() => { @@ -28,6 +29,7 @@ describe("validateMemoryFiles", () => { if (tempDir && fs.existsSync(tempDir)) { fs.rmSync(tempDir, { recursive: true, force: true }); } + vi.restoreAllMocks(); }); it("returns valid for empty directory", () => { @@ -243,10 +245,50 @@ describe("validateMemoryFiles", () => { expect(result.invalidFiles).toEqual([]); }); - it("uses repo as memoryType in error messages", () => { - fs.writeFileSync(path.join(tempDir, "invalid.log"), "log"); - const result = validateMemoryFiles(tempDir, "repo", [".txt"]); + it("uses 'cache' as the default memoryType", () => { + const result = validateMemoryFiles(tempDir); + expect(result.valid).toBe(true); + expect(global.core.info).toHaveBeenCalledWith(expect.stringContaining("cache-memory")); + }); + + it("calls core.error with details when files fail custom extension validation", () => { + fs.writeFileSync(path.join(tempDir, "bad.log"), "log"); + const result = validateMemoryFiles(tempDir, "repo", [".json"]); expect(result.valid).toBe(false); - expect(result.invalidFiles).toEqual(["invalid.log"]); + expect(result.invalidFiles).toContain("bad.log"); + expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("Found 1 file(s) with invalid extensions in repo-memory:")); + expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("bad.log (extension: .log)")); + expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("Allowed extensions: .json")); + }); + + it("reports files with no extension as '(no extension)' in error output", () => { + fs.writeFileSync(path.join(tempDir, "noext"), "content"); + const result = validateMemoryFiles(tempDir, "cache", [".json"]); + expect(result.valid).toBe(false); + expect(result.invalidFiles).toContain("noext"); + expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("noext (extension: (no extension))")); + }); + + it("detects invalid files in deeply nested directories with custom extensions", () => { + const level1 = path.join(tempDir, "level1"); + const level2 = path.join(level1, "level2"); + fs.mkdirSync(level1); + fs.mkdirSync(level2); + fs.writeFileSync(path.join(level2, "valid.json"), "{}"); + fs.writeFileSync(path.join(level2, "invalid.bin"), "binary"); + const result = validateMemoryFiles(tempDir, "cache", [".json"]); + expect(result.valid).toBe(false); + expect(result.invalidFiles).toContain(path.join("level1", "level2", "invalid.bin")); + expect(result.invalidFiles).not.toContain(path.join("level1", "level2", "valid.json")); + }); + + it("returns valid=false and empty invalidFiles when directory scan throws", () => { + vi.spyOn(fs, "readdirSync").mockImplementationOnce(() => { + throw new Error("Permission denied"); + }); + const result = validateMemoryFiles(tempDir, "cache", [".json"]); + expect(result.valid).toBe(false); + expect(result.invalidFiles).toEqual([]); + expect(global.core.error).toHaveBeenCalledWith(expect.stringContaining("Failed to scan cache-memory directory: Permission denied")); }); });