Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion actions/setup/js/artifact_client.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,12 @@ function createZipFromFiles(files, rootDirectory, outputPath) {
}

async function uploadFileToSignedURL(filePath, signedUploadURL, contentType) {
const stats = fs.statSync(filePath);
let stats;
try {
stats = fs.statSync(filePath);
} catch (err) {
throw new Error(`Failed to read file metadata for ${filePath}: ${getErrorMessage(err)}`, { cause: err });
}
const response = await fetch(signedUploadURL, {
method: "PUT",
headers: {
Expand Down
27 changes: 20 additions & 7 deletions actions/setup/js/build_checkout_manifest.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,28 @@ function readManifestEntriesFromEnv() {

function resolveDefaultBranch(repository, checkoutPath, options = {}) {
const workspace = options.workspace || process.env.GITHUB_WORKSPACE || "";
const runGit = options.runGit || ((args, execOptions = {}) => execFileSync("git", args, { encoding: "utf8", ...execOptions }));
const runGit =
options.runGit ||
((args, execOptions = {}) => {
try {
return execFileSync("git", args, { encoding: "utf8", ...execOptions });
} catch (err) {
throw new Error(`Failed to run git ${args.join(" ")}: ${getErrorMessage(err)}`, { cause: err });
}
});
const runGH =
options.runGH ||
((args, execOptions = {}) =>
execFileSync("gh", args, {
encoding: "utf8",
env: { ...process.env, ...(execOptions.env || {}) },
...execOptions,
}));
((args, execOptions = {}) => {
try {
return execFileSync("gh", args, {
encoding: "utf8",
env: { ...process.env, ...(execOptions.env || {}) },
...execOptions,
});
} catch (err) {
throw new Error(`Failed to run gh ${args.join(" ")}: ${getErrorMessage(err)}`, { cause: err });
}
});
let defaultBranch = "";

const repoPath = checkoutPath ? path.join(workspace, checkoutPath) : workspace;
Expand Down
17 changes: 15 additions & 2 deletions actions/setup/js/check_workflow_timestamp.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
const fs = require("fs");
const path = require("path");
const { ERR_CONFIG } = require("./error_codes.cjs");
const { getErrorMessage } = require("./error_helpers.cjs");

async function main() {
const workspace = process.env.GITHUB_WORKSPACE;
Expand Down Expand Up @@ -52,8 +53,20 @@ async function main() {
}

// Get file stats to compare modification times
const workflowStat = fs.statSync(workflowMdFile);
const lockStat = fs.statSync(lockFile);
let workflowStat;
let lockStat;
try {
workflowStat = fs.statSync(workflowMdFile);
} catch (err) {
core.setFailed(`Failed to inspect workflow source ${workflowMdFile}: ${getErrorMessage(err)}`);
return;
}
try {
lockStat = fs.statSync(lockFile);
} catch (err) {
core.setFailed(`Failed to inspect lock file ${lockFile}: ${getErrorMessage(err)}`);
return;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] Both statSync calls share a single try/catch, so the error message can't tell the operator whether the workflow .md or the .lock.yml file was unreadable.

💡 Suggested fix: separate the guards
try {
  workflowStat = fs.statSync(workflowMdFile);
} catch (err) {
  core.setFailed(`Failed to inspect workflow source ${workflowMdFile}: ${getErrorMessage(err)}`);
  return;
}
try {
  lockStat = fs.statSync(lockFile);
} catch (err) {
  core.setFailed(`Failed to inspect lock file ${lockFile}: ${getErrorMessage(err)}`);
  return;
}

The two files have different diagnostic meanings — one missing is a config problem, the other is a build artifact gap. Separate errors make triage unambiguous.

@copilot please address this.

}

const workflowMtime = workflowStat.mtime.getTime();
const lockMtime = lockStat.mtime.getTime();
Expand Down
86 changes: 86 additions & 0 deletions actions/setup/js/check_workflow_timestamp_error_handling.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import fs from "fs";
import os from "os";
import path from "path";

const mockCore = {
debug: vi.fn(),
info: vi.fn(),
notice: vi.fn(),
warning: vi.fn(),
error: vi.fn(),
setFailed: vi.fn(),
setOutput: vi.fn(),
exportVariable: vi.fn(),
summary: {
addRaw: vi.fn().mockReturnThis(),
write: vi.fn().mockResolvedValue(),
},
};

global.core = mockCore;

const { main } = await import("./check_workflow_timestamp.cjs");

describe("check_workflow_timestamp error handling", () => {
let tmpDir;
let workflowsDir;

beforeEach(() => {
vi.clearAllMocks();
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "workflow-stat-test-"));
workflowsDir = path.join(tmpDir, ".github", "workflows");
fs.mkdirSync(workflowsDir, { recursive: true });
process.env.GITHUB_WORKSPACE = tmpDir;
process.env.GH_AW_WORKFLOW_FILE = "test.lock.yml";
});

afterEach(() => {
vi.restoreAllMocks();
fs.rmSync(tmpDir, { recursive: true, force: true });
delete process.env.GITHUB_WORKSPACE;
delete process.env.GH_AW_WORKFLOW_FILE;
});

it("reports the workflow source path when source stat inspection fails", async () => {
const workflowFile = path.join(workflowsDir, "test.md");
const lockFile = path.join(workflowsDir, "test.lock.yml");
fs.writeFileSync(workflowFile, "# Workflow content");
fs.writeFileSync(lockFile, "# Lock content");

vi.spyOn(fs, "statSync").mockImplementation(targetPath => {
if (targetPath === workflowFile) {
throw new Error("source unreadable");
}
return {
mtime: new Date(),
isFile: () => true,
};
});

await main();

expect(mockCore.setFailed).toHaveBeenCalledWith(`Failed to inspect workflow source ${workflowFile}: source unreadable`);
});

it("reports the lock file path when lock stat inspection fails", async () => {
const workflowFile = path.join(workflowsDir, "test.md");
const lockFile = path.join(workflowsDir, "test.lock.yml");
fs.writeFileSync(workflowFile, "# Workflow content");
fs.writeFileSync(lockFile, "# Lock content");

vi.spyOn(fs, "statSync").mockImplementation(targetPath => {
if (targetPath === lockFile) {
throw new Error("lock unreadable");
}
return {
mtime: new Date(),
isFile: () => true,
};
});

await main();

expect(mockCore.setFailed).toHaveBeenCalledWith(`Failed to inspect lock file ${lockFile}: lock unreadable`);
});
});
23 changes: 14 additions & 9 deletions actions/setup/js/comment_memory_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const fs = require("fs");
const path = require("path");
const { getErrorMessage } = require("./error_helpers.cjs");

const COMMENT_MEMORY_TAG = "gh-aw-comment-memory";
const COMMENT_MEMORY_DIR = "/tmp/gh-aw/comment-memory";
Expand Down Expand Up @@ -118,15 +119,19 @@ function listCommentMemoryFiles(memoryDir = COMMENT_MEMORY_DIR) {
return [];
}

return fs
.readdirSync(memoryDir)
.filter(file => file.endsWith(COMMENT_MEMORY_EXTENSION))
.sort()
.map(file => ({
memoryId: file.slice(0, -COMMENT_MEMORY_EXTENSION.length),
filePath: path.join(memoryDir, file),
}))
.filter(entry => isSafeMemoryId(entry.memoryId));
try {
return fs
.readdirSync(memoryDir)
.filter(file => file.endsWith(COMMENT_MEMORY_EXTENSION))
.sort()
.map(file => ({
memoryId: file.slice(0, -COMMENT_MEMORY_EXTENSION.length),
filePath: path.join(memoryDir, file),
}))
.filter(entry => isSafeMemoryId(entry.memoryId));
} catch (err) {
throw new Error(`Failed to read comment-memory directory ${memoryDir}: ${getErrorMessage(err)}`, { cause: err });
}
}

function resolveCommentMemoryConfig(config) {
Expand Down
22 changes: 21 additions & 1 deletion actions/setup/js/comment_memory_helpers.test.cjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import fs from "fs";
import path from "path";
import { describe, it, expect, vi } from "vitest";
import { extractCommentMemoryEntries, isSafeMemoryId, stripCommentMemoryCodeFence, buildCodeFenceOpener } from "./comment_memory_helpers.cjs";
import { buildCodeFenceOpener, extractCommentMemoryEntries, isSafeMemoryId, listCommentMemoryFiles, stripCommentMemoryCodeFence } from "./comment_memory_helpers.cjs";

describe("comment_memory_helpers", () => {
it("builds code-fence opener with memory id", () => {
Expand Down Expand Up @@ -87,4 +89,22 @@ describe("comment_memory_helpers", () => {
expect(isSafeMemoryId(maxLengthId)).toBe(true);
expect(isSafeMemoryId(tooLongId)).toBe(false);
});

it("wraps directory read failures with the memory directory path", () => {
const memoryDir = fs.mkdtempSync(path.join("/tmp", "comment-memory-"));
const originalReaddirSync = fs.readdirSync;
const readdirSpy = vi.spyOn(fs, "readdirSync").mockImplementation((dirPath, options) => {
if (dirPath === memoryDir) {
throw new Error("EACCES");
}
return originalReaddirSync.call(fs, dirPath, options);
});

try {
expect(() => listCommentMemoryFiles(memoryDir)).toThrow(`Failed to read comment-memory directory ${memoryDir}: EACCES`);
} finally {
readdirSpy.mockRestore();
fs.rmSync(memoryDir, { recursive: true, force: true });
}
});
});
11 changes: 10 additions & 1 deletion actions/setup/js/generate_git_bundle.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,16 @@ async function generateGitBundle(branchName, baseBranch, options = {}) {

// Check if bundle was generated and has content
if (bundleGenerated && fs.existsSync(bundlePath)) {
const stat = fs.statSync(bundlePath);
let stat;
try {
stat = fs.statSync(bundlePath);
} catch (err) {
return {
success: false,
error: `Failed to inspect generated bundle ${bundlePath}: ${getErrorMessage(err)}`,
bundlePath,
};
}
const bundleSize = stat.size;

if (bundleSize === 0) {
Expand Down
12 changes: 11 additions & 1 deletion actions/setup/js/generate_git_patch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,17 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
patchPath,
};
}
if (!fs.statSync(candidate).isDirectory()) {
let candidateStat;
try {
candidateStat = fs.statSync(candidate);
} catch (err) {
return {
success: false,
error: `Failed to inspect workspacePath '${String(options.workspacePath)}': ${getErrorMessage(err)}`,
patchPath,
};
}
if (!candidateStat.isDirectory()) {
return {
success: false,
error: `Invalid workspacePath '${String(options.workspacePath)}': path is not a directory`,
Expand Down
8 changes: 7 additions & 1 deletion actions/setup/js/install_frontmatter_skills.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,13 @@ function countInstalledSkillFiles(skillsDst) {
if (!currentDir) {
continue;
}
for (const entry of fs.readdirSync(currentDir, { withFileTypes: true })) {
let entries;
try {
entries = fs.readdirSync(currentDir, { withFileTypes: true });
} catch (err) {
throw new Error(`Failed to read installed skills directory ${currentDir}: ${getErrorMessage(err)}`, { cause: err });
}
for (const entry of entries) {
const entryPath = path.join(currentDir, entry.name);
if (entry.isDirectory()) {
stack.push(entryPath);
Expand Down
20 changes: 17 additions & 3 deletions actions/setup/js/merge_remote_agent_github_folder.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,23 @@ function pathExists(filePath) {
*/
function getAllFiles(dir, baseDir = dir) {
const files = [];
const items = fs.readdirSync(dir);
let items;
try {
items = fs.readdirSync(dir);
} catch (err) {
throw new Error(`Failed to read directory ${dir}: ${getErrorMessage(err)}`, { cause: err });
}

for (const item of items) {
// Validate that item doesn't contain path traversal sequences
validateSafePath(item, dir, "directory item");
const fullPath = path.join(dir, item);
const stat = fs.statSync(fullPath);
let stat;
try {
stat = fs.statSync(fullPath);
} catch (err) {
throw new Error(`Failed to inspect path ${fullPath}: ${getErrorMessage(err)}`, { cause: err });
}

if (stat.isDirectory()) {
files.push(...getAllFiles(fullPath, baseDir));
Expand Down Expand Up @@ -273,7 +283,11 @@ function mergeGithubFolder(sourcePath, destPath) {
core.info(`Created directory: ${path.relative(destPath, destDir)}`);
}

fs.copyFileSync(sourceFile, destFile);
try {
fs.copyFileSync(sourceFile, destFile);
} catch (err) {
throw new Error(`Failed to copy file ${sourceFile} to ${destFile}: ${getErrorMessage(err)}`, { cause: err });
}
mergedCount++;
core.info(`Merged file: ${relativePath}`);
}
Expand Down
35 changes: 31 additions & 4 deletions actions/setup/js/push_experiment_state.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,13 @@ function checkoutOrCreateBranch(branchName, repoUrl, workspaceDir) {
execGitSync(["checkout", "--orphan", branchName], { stdio: "inherit", cwd: workspaceDir });
execGitSync(["read-tree", "--empty"], { stdio: "pipe", cwd: workspaceDir });
// Remove any pre-existing working-tree files (from sparse checkout).
for (const entry of fs.readdirSync(workspaceDir)) {
let entries;
try {
entries = fs.readdirSync(workspaceDir);
} catch (err) {
throw new Error(`Failed to read workspace directory ${workspaceDir}: ${getErrorMessage(err)}`, { cause: err });
}
for (const entry of entries) {
if (entry !== ".git") {
fs.rmSync(path.join(workspaceDir, entry), { recursive: true, force: true });
}
Expand Down Expand Up @@ -108,10 +114,31 @@ async function main() {
core.info(`Pushing ${stateLabel} to branch "${branchName}" in ${targetRepo}`);

// Collect the JSON files that exist in the state directory.
const filesToPush = candidateFiles.filter(name => {
/** @type {string[]} */
const filesToPush = [];
/** @type {string[]} */
const inspectErrors = [];
for (const name of candidateFiles) {
const full = path.join(stateDir, name);
return fs.existsSync(full) && fs.statSync(full).isFile();
});
if (!fs.existsSync(full)) {
continue;
}
let fileInfo;
try {
fileInfo = fs.statSync(full);
} catch (err) {
inspectErrors.push(`${full}: ${getErrorMessage(err)}`);
continue;
}
if (fileInfo.isFile()) {
filesToPush.push(name);
}
}

if (inspectErrors.length > 0) {
core.setFailed(`Failed to inspect ${stateLabel} files:\n${inspectErrors.join("\n")}`);
return;
}

if (filesToPush.length === 0) {
core.info(`No ${stateLabel} files found – nothing to push`);
Expand Down
Loading
Loading