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
2 changes: 1 addition & 1 deletion actions/setup-cli/install.sh

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 40 additions & 0 deletions actions/setup/js/add_comment.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,29 @@ async function hideOlderComments(github, owner, repo, itemNumber, workflowIds, i
return hiddenCount;
}

/**
* Check whether an error from a GitHub GraphQL or REST call indicates that the
* integration token lacks the permissions required to write to a discussion.
* @param {unknown} error
* @returns {boolean}
*/
function isDiscussionIntegrationAccessError(error) {
// Lowercase for case-insensitive comparison via .toLowerCase()
const fragment = "resource not accessible by integration";
/** @type {string[]} */
const messages = [getErrorMessage(error)];

if (error && typeof error === "object" && "errors" in error && Array.isArray(/** @type {any} */ error.errors)) {
for (const graphQLError of /** @type {any} */ error.errors) {
if (typeof graphQLError?.message === "string") {
messages.push(graphQLError.message);
}
}
}

return messages.some(message => message.toLowerCase().includes(fragment));
}

/**
* Comment on a GitHub Discussion using GraphQL
* @param {any} github - GitHub REST API instance
Expand Down Expand Up @@ -915,6 +938,7 @@ async function main(config = {}) {
} catch (discussionError) {
const discussionErrorMessage = getErrorMessage(discussionError);
const isDiscussion404 = discussionError?.status === 404 || discussionErrorMessage.toLowerCase().includes("not found");
const isIntegrationAccessError = isDiscussionIntegrationAccessError(discussionError);

if (isDiscussion404) {
// Neither issue/PR nor discussion found - truly doesn't exist
Expand All @@ -926,6 +950,21 @@ async function main(config = {}) {
};
}

if (isIntegrationAccessError) {
// The integration token lacks discussions:write scope — surface as a configuration
// warning (skip) rather than failing the entire safe-outputs job.
const warningMessage =
`Skipping add_comment for discussion #${itemNumber}: configuration mismatch ` +
`(GitHub integration token cannot add comments to discussions: Resource not accessible by integration). ` +
`Use safe-outputs.add-comment.github-token with a token that has discussions:write scope.`;
core.warning(warningMessage);
return {
success: false,
skipped: true,
error: warningMessage,
};
}

// Other error when trying as discussion
core.error(`Failed to add comment to discussion: ${discussionErrorMessage}`);
return {
Expand Down Expand Up @@ -972,4 +1011,5 @@ module.exports = {
MAX_MENTIONS,
MAX_LINKS,
enforceCommentLimits,
isDiscussionIntegrationAccessError,
};
121 changes: 121 additions & 0 deletions actions/setup/js/add_comment.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2078,6 +2078,127 @@ describe("add_comment", () => {
// Should only call GraphQL once (not retry)
expect(graphqlCallCount).toBe(1);
});

it("should return skipped (not fail) when discussion comment fails with Resource not accessible by integration", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

let warningCalls = [];
let errorCalls = [];
mockCore.warning = msg => {
warningCalls.push(msg);
};
mockCore.error = msg => {
errorCalls.push(msg);
};

// Mock REST API to return 404 (not found as issue/PR)
mockGithub.rest.issues.createComment = async () => {
const error = new Error("Not Found");
// @ts-ignore -- Error type does not include status, but octokit adds it at runtime
error.status = 404;
throw error;
};

// Mock GraphQL to return the discussion node ID but fail on mutation
let graphqlCalls = [];
mockGithub.graphql = async (query, vars) => {
graphqlCalls.push({ query, vars });

// First call: fetch discussion node ID
if (query.includes("query") && query.includes("discussion(number:")) {
return {
repository: {
discussion: {
id: "D_kwDOTest335",
url: "https://github.com/owner/repo/discussions/335",
},
},
};
}

// Second call: mutation — simulate "Resource not accessible by integration"
if (query.includes("mutation") && query.includes("addDiscussionComment")) {
const err = new Error("Request failed due to following response errors:\n - Resource not accessible by integration");
throw err;
}
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ target: 'triggering' }); })()`);

const message = {
type: "add_comment",
item_number: 335,
body: "Test comment on discussion",
};

const result = await handler(message, {});

// Should be skipped (not a hard failure) so the safe-outputs job doesn't fail
expect(result.success).toBe(false);
expect(result.skipped).toBe(true);
expect(result.error).toContain("configuration mismatch");
expect(result.error).toContain("discussions:write");

// Warning should be emitted, no error
const configMismatchWarning = warningCalls.find(msg => msg.includes("configuration mismatch"));
expect(configMismatchWarning).toBeTruthy();
expect(errorCalls.length).toBe(0);
});

it("should return skipped when discussion GraphQL errors array contains Resource not accessible", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

let warningCalls = [];
mockCore.warning = msg => {
warningCalls.push(msg);
};

// Mock REST API to return 404
mockGithub.rest.issues.createComment = async () => {
const error = new Error("Not Found");
// @ts-ignore
error.status = 404;
throw error;
};

// GraphQL: discussion exists but mutation fails with errors array
mockGithub.graphql = async (query, vars) => {
if (query.includes("query") && query.includes("discussion(number:")) {
return {
repository: {
discussion: {
id: "D_kwDOTest336",
url: "https://github.com/owner/repo/discussions/336",
},
},
};
}

if (query.includes("mutation") && query.includes("addDiscussionComment")) {
const err = new Error("Request failed");
// @ts-ignore -- Error type does not include errors, but GraphQL errors surface this way at runtime
err.errors = [{ type: "FORBIDDEN", message: "Resource not accessible by integration" }];
throw err;
}
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ target: 'triggering' }); })()`);

const message = {
type: "add_comment",
item_number: 336,
body: "Test comment",
};

const result = await handler(message, {});

expect(result.success).toBe(false);
expect(result.skipped).toBe(true);
expect(result.error).toContain("configuration mismatch");

const configMismatchWarning = warningCalls.find(msg => msg.includes("configuration mismatch"));
expect(configMismatchWarning).toBeTruthy();
});
});

describe("temporary ID resolution", () => {
Expand Down
14 changes: 14 additions & 0 deletions actions/setup/js/safe_outputs_handlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,20 @@ function createHandlers(server, appendSafeOutput, config = {}) {
};
}

// Refuse discussion-specific requests when discussions are not enabled in config.
// reply_to_id is a discussion-only field; its presence unambiguously means the
// agent is targeting a GitHub Discussion. Guard here (MCP phase) so the agent
// gets immediate, actionable feedback rather than a late failure at execution time.
const addCommentConfig = getSafeOutputsToolConfig(config, "add_comment");
const discussionsEnabled = addCommentConfig.discussions === true;
const hasReplyToId = args?.reply_to_id != null && String(args.reply_to_id).trim() !== "";
if (hasReplyToId && !discussionsEnabled) {
return buildIntentErrorResponse(
"add_comment with reply_to_id targets a GitHub Discussion, but discussion comments are not enabled for this workflow. " +
"Set 'discussions: true' in the workflow's safe-outputs.add-comment configuration to enable discussion comments and request discussions:write permission."
);
}

// Build the entry with a temporary_id
const entry = { ...(args || {}), type: "add_comment" };
const wildcardTargetValidationError = validateWildcardTargetRequirement(entry);
Expand Down
37 changes: 37 additions & 0 deletions actions/setup/js/safe_outputs_handlers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1741,6 +1741,43 @@ describe("safe_outputs_handlers", () => {
expect(responseData.error).toContain("requires item_number");
expect(mockAppendSafeOutput).not.toHaveBeenCalled();
});

it("should refuse reply_to_id when discussions are not enabled in config", () => {
// Default handlers have no discussions: true in config
const result = handlers.addCommentHandler({
body: "Reply to a discussion thread",
reply_to_id: "DC_kwDOABcD1M4AaBbC",
});

expect(result.isError).toBe(true);
const responseData = JSON.parse(result.content[0].text);
expect(responseData.result).toBe("error");
expect(responseData.error).toContain("discussion comments are not enabled");
expect(responseData.error).toContain("discussions: true");
expect(responseData.error).toContain("safe-outputs.add-comment");
expect(mockAppendSafeOutput).not.toHaveBeenCalled();
});

it("should allow reply_to_id when discussions are enabled in config", () => {
const discussionHandlers = createHandlers(mockServer, mockAppendSafeOutput, {
"add-comment": { enabled: true, discussions: true },
});

const result = discussionHandlers.addCommentHandler({
body: "Reply to a discussion thread with real content that is not a test placeholder",
reply_to_id: "DC_kwDOABcD1M4AaBbC",
});

expect(result.isError).toBeUndefined();
const responseData = JSON.parse(result.content[0].text);
expect(responseData.result).toBe("success");
expect(mockAppendSafeOutput).toHaveBeenCalledWith(
expect.objectContaining({
type: "add_comment",
reply_to_id: "DC_kwDOABcD1M4AaBbC",
})
);
});
});

describe("createIssueHandler", () => {
Expand Down
2 changes: 1 addition & 1 deletion install-gh-aw.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash
set +o histexpand

# Kept in sync with actions/setup-cli/install.sh — edit this file, then copy to that path.
# Kept in sync with actions/setup-cli/install.sh — edit install-gh-aw.sh, then copy to that path.

# Script to download and install gh-aw binary for the current OS and architecture
# Supports: Linux, macOS (Darwin), FreeBSD, Windows (Git Bash/MSYS/Cygwin)
Expand Down
Loading