diff --git a/actions/setup/js/check_workflow_timestamp_api.cjs b/actions/setup/js/check_workflow_timestamp_api.cjs index b94271f19b..05ec62454c 100644 --- a/actions/setup/js/check_workflow_timestamp_api.cjs +++ b/actions/setup/js/check_workflow_timestamp_api.cjs @@ -63,19 +63,20 @@ async function main() { } // Helper function to compute and compare frontmatter hashes - async function logFrontmatterHashComparison() { + // Returns: { match: boolean, storedHash: string, recomputedHash: string } or null on error + async function compareFrontmatterHashes() { try { // Fetch lock file content to extract stored hash const lockFileContent = await getFileContent(github, owner, repo, lockFilePath, ref); if (!lockFileContent) { core.info("Unable to fetch lock file content for hash comparison"); - return; + return null; } const storedHash = extractHashFromLockFile(lockFileContent); if (!storedHash) { core.info("No frontmatter hash found in lock file"); - return; + return null; } // Compute hash using pure JavaScript implementation @@ -83,19 +84,19 @@ async function main() { const fileReader = createGitHubFileReader(github, owner, repo, ref); const recomputedHash = await computeFrontmatterHash(workflowMdPath, { fileReader }); + const match = storedHash === recomputedHash; + // Log hash comparison core.info(`Frontmatter hash comparison:`); core.info(` Lock file hash: ${storedHash}`); core.info(` Recomputed hash: ${recomputedHash}`); + core.info(` Status: ${match ? "✅ Hashes match" : "⚠️ Hashes differ"}`); - if (storedHash === recomputedHash) { - core.info(` Status: ✅ Hashes match`); - } else { - core.info(` Status: ⚠️ Hashes differ`); - } + return { match, storedHash, recomputedHash }; } catch (error) { const errorMessage = getErrorMessage(error); core.info(`Could not compute frontmatter hash: ${errorMessage}`); + return null; } } @@ -124,10 +125,13 @@ async function main() { core.info(` Source last commit: ${workflowDate.toISOString()} (${workflowCommit.sha.substring(0, 7)})`); core.info(` Lock last commit: ${lockDate.toISOString()} (${lockCommit.sha.substring(0, 7)})`); + const workflowTime = workflowDate.getTime(); + const lockTime = lockDate.getTime(); + // Check if workflow file is newer than lock file - if (workflowDate > lockDate) { - // Log frontmatter hash comparison only when timestamp check fails - await logFrontmatterHashComparison(); + if (workflowTime > lockTime) { + // Clear case: workflow file is newer - needs recompilation + await compareFrontmatterHashes(); // Log for diagnostic purposes const warningMessage = `Lock file '${lockFilePath}' is outdated! The workflow file '${workflowMdPath}' has been modified more recently. Run 'gh aw compile' to regenerate the lock file.`; // Format timestamps and commits for display @@ -152,9 +156,69 @@ async function main() { // Fail the step to prevent workflow from running with outdated configuration core.setFailed(warningMessage); } else if (workflowCommit.sha === lockCommit.sha) { + // Same commit - definitely up to date core.info("✅ Lock file is up to date (same commit)"); + } else if (workflowTime === lockTime) { + // Timestamps are equal (coarse timestamp) but different commits + // Use frontmatter hash comparison to determine if recompilation is needed + core.info("Timestamps are equal - using frontmatter hash comparison"); + const hashComparison = await compareFrontmatterHashes(); + + if (!hashComparison) { + // Could not compute hash - be conservative and assume it's ok + core.info("⚠️ Could not compare frontmatter hashes - assuming lock file is up to date"); + core.info("✅ Lock file is up to date (timestamp check passed, hash comparison unavailable)"); + } else if (hashComparison.match) { + // Hashes match - lock file is up to date + core.info("✅ Lock file is up to date (hashes match)"); + } else { + // Hashes differ - lock file needs recompilation + const warningMessage = `Lock file '${lockFilePath}' is outdated! Frontmatter hash mismatch detected. Run 'gh aw compile' to regenerate the lock file.`; + + // Format timestamps and commits for display + const workflowTimestamp = workflowDate.toISOString(); + const lockTimestamp = lockDate.toISOString(); + + // Add summary to GitHub Step Summary + let summary = core.summary + .addRaw("### ⚠️ Workflow Lock File Warning\n\n") + .addRaw("**WARNING**: Lock file is outdated (frontmatter hash mismatch).\n\n") + .addRaw("**Files:**\n") + .addRaw(`- Source: \`${workflowMdPath}\`\n`) + .addRaw(` - Last commit: ${workflowTimestamp}\n`) + .addRaw(` - Commit SHA: [\`${workflowCommit.sha.substring(0, 7)}\`](https://github.com/${owner}/${repo}/commit/${workflowCommit.sha})\n`) + .addRaw(` - Frontmatter hash: \`${hashComparison.recomputedHash.substring(0, 12)}...\`\n`) + .addRaw(`- Lock: \`${lockFilePath}\`\n`) + .addRaw(` - Last commit: ${lockTimestamp}\n`) + .addRaw(` - Commit SHA: [\`${lockCommit.sha.substring(0, 7)}\`](https://github.com/${owner}/${repo}/commit/${lockCommit.sha})\n`) + .addRaw(` - Stored hash: \`${hashComparison.storedHash.substring(0, 12)}...\`\n\n`) + .addRaw("**Action Required:** Run `gh aw compile` to regenerate the lock file.\n\n"); + + await summary.write(); + + // Fail the step to prevent workflow from running with outdated configuration + core.setFailed(warningMessage); + } } else { - core.info("✅ Lock file is up to date"); + // Lock file is newer than workflow file + // This means the lock was recompiled after the .md file, so it's up to date + // We verify the hash for informational purposes but don't fail + core.info("Lock file is newer - verifying frontmatter hash for consistency"); + const hashComparison = await compareFrontmatterHashes(); + + if (!hashComparison) { + // Could not compute hash + core.info("⚠️ Could not compare frontmatter hashes"); + core.info("✅ Lock file is up to date (lock is newer than source)"); + } else if (hashComparison.match) { + // Hashes match - perfect consistency + core.info("✅ Lock file is up to date (lock is newer and hashes match)"); + } else { + // Hashes differ but lock is newer, so it's still considered up to date + // The .md file may have been edited after the lock was compiled + core.info("⚠️ Frontmatter hash mismatch detected, but lock file is newer than source"); + core.info("✅ Lock file is up to date (lock was recompiled after source changes)"); + } } } diff --git a/actions/setup/js/check_workflow_timestamp_api.test.cjs b/actions/setup/js/check_workflow_timestamp_api.test.cjs index 5d3c22f0c9..60d06b5d87 100644 --- a/actions/setup/js/check_workflow_timestamp_api.test.cjs +++ b/actions/setup/js/check_workflow_timestamp_api.test.cjs @@ -527,4 +527,373 @@ jobs: expect(mockCore.setFailed).toHaveBeenCalled(); // Should fail because timestamp check failed }); }); + + describe("coarse timestamp handling", () => { + beforeEach(() => { + process.env.GH_AW_WORKFLOW_FILE = "test.lock.yml"; + }); + + it("should use hash comparison when timestamps are equal and hashes match", async () => { + // Hash for frontmatter "engine: copilot" + const validHash = "c2a79263dc72f28c76177afda9bf0935481b26da094407a50155a6e0244084e3"; + const lockFileContent = `# frontmatter-hash: ${validHash} +name: Test Workflow +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo "test"`; + + const mdFileContent = `--- +engine: copilot +--- +# Test Workflow`; + + mockGithub.rest.repos.listCommits + .mockResolvedValueOnce({ + data: [ + { + sha: "src123", + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Same timestamp + message: "Source commit", + }, + }, + ], + }) + .mockResolvedValueOnce({ + data: [ + { + sha: "lock456", // Different commit SHA + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Same timestamp + message: "Lock commit", + }, + }, + ], + }); + + mockGithub.rest.repos.getContent + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(lockFileContent).toString("base64"), + }, + }) + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(mdFileContent).toString("base64"), + }, + }); + + await main(); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Timestamps are equal")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Frontmatter hash comparison")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("✅ Lock file is up to date (hashes match)")); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it("should fail when timestamps are equal but hashes differ", async () => { + const storedHash = "cdb5fdf551a14f93f6a8bb32b4f8ee5a6e93a8075052ecd915180be7fbc168ca"; + const lockFileContent = `# frontmatter-hash: ${storedHash} +name: Test Workflow +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo "test"`; + + // Different frontmatter - will produce different hash + const mdFileContent = `--- +engine: claude +model: claude-sonnet-4 +--- +# Test Workflow`; + + mockGithub.rest.repos.listCommits + .mockResolvedValueOnce({ + data: [ + { + sha: "src123", + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Same timestamp + message: "Source commit", + }, + }, + ], + }) + .mockResolvedValueOnce({ + data: [ + { + sha: "lock456", // Different commit SHA + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Same timestamp + message: "Lock commit", + }, + }, + ], + }); + + mockGithub.rest.repos.getContent + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(lockFileContent).toString("base64"), + }, + }) + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(mdFileContent).toString("base64"), + }, + }); + + await main(); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Timestamps are equal")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("⚠️ Hashes differ")); + expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Frontmatter hash mismatch")); + expect(mockCore.summary.addRaw).toHaveBeenCalledWith(expect.stringContaining("frontmatter hash mismatch")); + }); + + it("should pass when timestamps are equal and hash comparison fails", async () => { + const lockFileContent = `name: Test Workflow +on: push +jobs: + test: + runs-on: ubuntu-latest`; + + mockGithub.rest.repos.listCommits + .mockResolvedValueOnce({ + data: [ + { + sha: "src123", + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Same timestamp + message: "Source commit", + }, + }, + ], + }) + .mockResolvedValueOnce({ + data: [ + { + sha: "lock456", // Different commit SHA + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Same timestamp + message: "Lock commit", + }, + }, + ], + }); + + mockGithub.rest.repos.getContent.mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(lockFileContent).toString("base64"), + }, + }); + + await main(); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Timestamps are equal")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("No frontmatter hash found")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Could not compare frontmatter hashes")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("✅ Lock file is up to date")); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + }); + + describe("lock file newer than source file", () => { + beforeEach(() => { + process.env.GH_AW_WORKFLOW_FILE = "test.lock.yml"; + }); + + it("should pass when lock file is newer and hashes match", async () => { + // Hash for frontmatter "engine: copilot" + const validHash = "c2a79263dc72f28c76177afda9bf0935481b26da094407a50155a6e0244084e3"; + const lockFileContent = `# frontmatter-hash: ${validHash} +name: Test Workflow +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo "test"`; + + const mdFileContent = `--- +engine: copilot +--- +# Test Workflow`; + + mockGithub.rest.repos.listCommits + .mockResolvedValueOnce({ + data: [ + { + sha: "src123", + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Source is older + message: "Source commit", + }, + }, + ], + }) + .mockResolvedValueOnce({ + data: [ + { + sha: "lock456", + commit: { + committer: { date: "2024-01-01T13:00:00Z" }, // Lock is newer + message: "Lock commit", + }, + }, + ], + }); + + mockGithub.rest.repos.getContent + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(lockFileContent).toString("base64"), + }, + }) + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(mdFileContent).toString("base64"), + }, + }); + + await main(); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Lock file is newer")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("✅ Lock file is up to date (lock is newer and hashes match)")); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it("should pass when lock file is newer but hashes differ", async () => { + const storedHash = "c2a79263dc72f28c76177afda9bf0935481b26da094407a50155a6e0244084e3"; + const lockFileContent = `# frontmatter-hash: ${storedHash} +name: Test Workflow +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - run: echo "test"`; + + // Different frontmatter - will produce different hash + const mdFileContent = `--- +engine: claude +model: claude-sonnet-4 +--- +# Test Workflow`; + + mockGithub.rest.repos.listCommits + .mockResolvedValueOnce({ + data: [ + { + sha: "src123", + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Source is older + message: "Source commit", + }, + }, + ], + }) + .mockResolvedValueOnce({ + data: [ + { + sha: "lock456", + commit: { + committer: { date: "2024-01-01T13:00:00Z" }, // Lock is newer + message: "Lock commit", + }, + }, + ], + }); + + mockGithub.rest.repos.getContent + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(lockFileContent).toString("base64"), + }, + }) + .mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(mdFileContent).toString("base64"), + }, + }); + + await main(); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Lock file is newer")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("⚠️ Frontmatter hash mismatch")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("✅ Lock file is up to date")); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + + it("should pass when lock file is newer and hash comparison fails", async () => { + const lockFileContent = `name: Test Workflow +on: push +jobs: + test: + runs-on: ubuntu-latest`; + + mockGithub.rest.repos.listCommits + .mockResolvedValueOnce({ + data: [ + { + sha: "src123", + commit: { + committer: { date: "2024-01-01T12:00:00Z" }, // Source is older + message: "Source commit", + }, + }, + ], + }) + .mockResolvedValueOnce({ + data: [ + { + sha: "lock456", + commit: { + committer: { date: "2024-01-01T13:00:00Z" }, // Lock is newer + message: "Lock commit", + }, + }, + ], + }); + + mockGithub.rest.repos.getContent.mockResolvedValueOnce({ + data: { + type: "file", + encoding: "base64", + content: Buffer.from(lockFileContent).toString("base64"), + }, + }); + + await main(); + + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Lock file is newer")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("No frontmatter hash found")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("Could not compare frontmatter hashes")); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("✅ Lock file is up to date (lock is newer than source)")); + expect(mockCore.setFailed).not.toHaveBeenCalled(); + }); + }); }); diff --git a/actions/setup/js/frontmatter_hash_github_api.test.cjs b/actions/setup/js/frontmatter_hash_github_api.test.cjs index 2cab88cccb..b07af7a1e5 100644 --- a/actions/setup/js/frontmatter_hash_github_api.test.cjs +++ b/actions/setup/js/frontmatter_hash_github_api.test.cjs @@ -245,6 +245,93 @@ describe("frontmatter_hash with GitHub API", () => { }); }); + describe("path resolution with GitHub API", () => { + it("should use repository-relative paths for imports (not absolute filesystem paths)", async () => { + // This test validates the fix for issue where path.resolve() created absolute paths + // that broke GitHub API calls + const owner = "githubnext"; + const repo = "gh-aw"; + const ref = "main"; + + // Track all paths requested from GitHub API + const requestedPaths = []; + const trackingMockGitHub = { + rest: { + repos: { + getContent: async ({ path: filePath }) => { + requestedPaths.push(filePath); + + // Verify path is repository-relative (not absolute) + if (path.isAbsolute(filePath)) { + throw new Error(`GitHub API should not receive absolute paths: ${filePath}`); + } + + // Read from local filesystem for testing + const fsPath = require("path"); + const repoRoot = fsPath.resolve(__dirname, "../../.."); + const fullPath = fsPath.join(repoRoot, filePath); + + if (!fs.existsSync(fullPath)) { + const error = new Error(`Not Found: ${filePath}`); + error.status = 404; + throw error; + } + + const content = fs.readFileSync(fullPath, "utf8"); + const base64Content = Buffer.from(content).toString("base64"); + + return { + data: { + content: base64Content, + encoding: "base64", + }, + }; + }, + }, + }, + }; + + const fileReader = createGitHubFileReader(trackingMockGitHub, owner, repo, ref); + + // Test with smoke-codex.md which has imports + const workflowPath = ".github/workflows/smoke-codex.md"; + const hash = await computeFrontmatterHash(workflowPath, { fileReader }); + + // Verify hash was computed successfully + expect(hash).toMatch(/^[a-f0-9]{64}$/); + + // Verify all requested paths are repository-relative + expect(requestedPaths.length).toBeGreaterThan(1); // Should include main file + imports + for (const requestedPath of requestedPaths) { + expect(path.isAbsolute(requestedPath)).toBe(false); + expect(requestedPath).toMatch(/^\.github\/workflows\//); + } + + // Log the paths for debugging + console.log("Requested paths from GitHub API:", requestedPaths); + }); + + it("should compute same hash with GitHub API reader as with filesystem reader", async () => { + const owner = "githubnext"; + const repo = "gh-aw"; + const ref = "main"; + + // Compute hash with filesystem reader (absolute path) + const repoRoot = path.resolve(__dirname, "../../.."); + const absolutePath = path.join(repoRoot, ".github/workflows/smoke-codex.md"); + const fsHash = await computeFrontmatterHash(absolutePath); + + // Compute hash with GitHub API reader (relative path) + const fileReader = createGitHubFileReader(mockGitHub, owner, repo, ref); + const apiHash = await computeFrontmatterHash(".github/workflows/smoke-codex.md", { + fileReader, + }); + + // Hashes should match (this was broken before the fix) + expect(apiHash).toBe(fsHash); + }); + }); + describe("live GitHub API integration", () => { it("should compute hash using real GitHub API (no mocks)", async () => { // Skip this test if no GitHub token is available diff --git a/actions/setup/js/frontmatter_hash_pure.cjs b/actions/setup/js/frontmatter_hash_pure.cjs index d094fc3796..06a008bdd0 100644 --- a/actions/setup/js/frontmatter_hash_pure.cjs +++ b/actions/setup/js/frontmatter_hash_pure.cjs @@ -129,8 +129,8 @@ async function processImportsTextBased(frontmatterText, baseDir, visited = new S const sortedImports = [...imports].sort(); for (const importPath of sortedImports) { - // Resolve import path relative to base directory - const fullPath = path.resolve(baseDir, importPath); + // Join import path with base directory (preserves relative paths for GitHub API compatibility) + const fullPath = path.join(baseDir, importPath); // Skip if already visited (cycle detection) if (visited.has(fullPath)) continue;