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
141 changes: 112 additions & 29 deletions actions/setup/js/resolve_pr_review_thread.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,30 @@ const { getErrorMessage } = require("./error_helpers.cjs");
const { getPRNumber } = require("./update_context_helpers.cjs");
const { logStagedPreviewInfo } = require("./staged_preview.cjs");
const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
const { resolveTargetRepoConfig, validateTargetRepo } = require("./repo_helpers.cjs");

/**
* Type constant for handler identification
*/
const HANDLER_TYPE = "resolve_pull_request_review_thread";

/**
* Look up a review thread's parent PR number via the GraphQL API.
* Used to validate the thread belongs to the triggering PR before resolving.
* Look up a review thread's parent PR number and repository via the GraphQL API.
* Used to validate the thread before resolving.
* @param {any} github - GitHub GraphQL instance
* @param {string} threadId - Review thread node ID (e.g., 'PRRT_kwDOABCD...')
* @returns {Promise<number|null>} The PR number the thread belongs to, or null if not found
* @returns {Promise<{prNumber: number, repoNameWithOwner: string|null}|null>} The PR number and repo, or null if not found
*/
async function getThreadPullRequestNumber(github, threadId) {
async function getThreadPullRequestInfo(github, threadId) {
const query = /* GraphQL */ `
query ($threadId: ID!) {
node(id: $threadId) {
... on PullRequestReviewThread {
pullRequest {
number
repository {
nameWithOwner
}
}
}
}
Expand All @@ -37,7 +41,15 @@ async function getThreadPullRequestNumber(github, threadId) {

const result = await github.graphql(query, { threadId });

return result?.node?.pullRequest?.number ?? null;
const pullRequest = result?.node?.pullRequest;
if (!pullRequest) {
return null;
}

return {
prNumber: pullRequest.number,
repoNameWithOwner: pullRequest.repository?.nameWithOwner ?? null,
};
Comment on lines 20 to 52
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The JSDoc says repoNameWithOwner is a string, but the function can return null (pullRequest.repository?.nameWithOwner ?? null). Update the return type to string|null (and/or enforce non-null if you expect this field to always be present) to keep the docs consistent with runtime behavior.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9c93026 — updated the JSDoc return type to {prNumber: number, repoNameWithOwner: string|null}|null.

}

/**
Expand Down Expand Up @@ -70,14 +82,21 @@ async function resolveReviewThreadAPI(github, threadId) {
* Main handler factory for resolve_pull_request_review_thread
* Returns a message handler function that processes individual resolve messages.
*
* Resolution is scoped to the triggering PR only — the handler validates that each
* thread belongs to the triggering pull request before resolving it. This prevents
* agents from resolving threads on unrelated PRs.
* By default, resolution is scoped to the triggering PR only. When target-repo or
* allowed-repos are specified, cross-repository thread resolution is supported.
* @type {HandlerFactoryFunction}
*/
async function main(config = {}) {
// Extract configuration
const maxCount = config.max || 10;
const resolveTarget = config.target || "triggering";
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);

// Whether the user explicitly configured cross-repo targeting.
// defaultTargetRepo always has a value (falls back to context.repo), so we check
// the raw config keys to distinguish user-configured from default.
const hasExplicitTargetConfig = !!(config["target-repo"] || config.allowed_repos?.length > 0);

const authClient = await createAuthenticatedGitHubClient(config);

// Determine the triggering PR number from context
Expand All @@ -86,7 +105,11 @@ async function main(config = {}) {
// Check if we're in staged mode
const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true";

core.info(`Resolve PR review thread configuration: max=${maxCount}, triggeringPR=${triggeringPRNumber || "none"}`);
core.info(`Resolve PR review thread configuration: max=${maxCount}, target=${resolveTarget}, triggeringPR=${triggeringPRNumber || "none"}`);
core.info(`Default target repo: ${defaultTargetRepo}`);
if (allowedRepos.size > 0) {
core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`);
}

// Track how many items we've processed for max limit
let processedCount = 0;
Expand Down Expand Up @@ -122,34 +145,94 @@ async function main(config = {}) {
};
}

// Validate triggering PR context
if (!triggeringPRNumber) {
core.warning("Cannot resolve review thread: not running in a pull request context");
return {
success: false,
error: "Cannot resolve review threads outside of a pull request context",
};
}

// Look up the thread to validate it belongs to the triggering PR
const threadPRNumber = await getThreadPullRequestNumber(authClient, threadId);
if (threadPRNumber === null) {
// Look up the thread's PR number and repository
const threadInfo = await getThreadPullRequestInfo(authClient, threadId);
if (threadInfo === null) {
core.warning(`Review thread not found or not a PullRequestReviewThread: ${threadId}`);
return {
success: false,
error: `Review thread not found: ${threadId}`,
};
}

if (threadPRNumber !== triggeringPRNumber) {
core.warning(`Thread ${threadId} belongs to PR #${threadPRNumber}, not triggering PR #${triggeringPRNumber}`);
return {
success: false,
error: `Thread belongs to PR #${threadPRNumber}, but only threads on the triggering PR #${triggeringPRNumber} can be resolved`,
};
const { prNumber: threadPRNumber, repoNameWithOwner: threadRepo } = threadInfo;

// When the user explicitly configured target-repo or allowed-repos, validate the thread's
// repository using validateTargetRepo (supports wildcards like "*", "org/*").
// Otherwise, fall back to the legacy behavior of scoping to the triggering PR only.
if (hasExplicitTargetConfig) {
// Cross-repo mode: validate thread repo against configured repos (fail closed if missing)
if (!threadRepo) {
core.warning(`Could not determine repository for thread ${threadId}`);
return {
success: false,
error: `Could not determine the repository for thread ${threadId}`,
};
}
const repoValidation = validateTargetRepo(threadRepo, defaultTargetRepo, allowedRepos);
if (!repoValidation.valid) {
core.warning(`Thread ${threadId} belongs to repo ${threadRepo}, which is not in the allowed repos`);
return {
success: false,
error: repoValidation.error,
};
}

// Determine target PR number based on target config
if (resolveTarget === "triggering") {
if (!triggeringPRNumber) {
core.warning("Cannot resolve review thread: not running in a pull request context");
return {
success: false,
error: "Cannot resolve review threads outside of a pull request context",
};
}
if (threadPRNumber !== triggeringPRNumber) {
core.warning(`Thread ${threadId} belongs to PR #${threadPRNumber}, not triggering PR #${triggeringPRNumber}`);
return {
success: false,
error: `Thread belongs to PR #${threadPRNumber}, but only threads on the triggering PR #${triggeringPRNumber} can be resolved`,
};
}
} else if (resolveTarget !== "*") {
// Explicit PR number target
const targetPRNumber = parseInt(resolveTarget, 10);
if (Number.isNaN(targetPRNumber) || targetPRNumber <= 0) {
core.warning(`Invalid target PR number: '${resolveTarget}'`);
return {
success: false,
error: `Invalid target: '${resolveTarget}' - must be 'triggering', '*', or a positive integer`,
};
}
if (threadPRNumber !== targetPRNumber) {
core.warning(`Thread ${threadId} belongs to PR #${threadPRNumber}, not target PR #${targetPRNumber}`);
return {
success: false,
error: `Thread belongs to PR #${threadPRNumber}, but target is PR #${targetPRNumber}`,
};
}
}
// resolveTarget === "*": any PR in allowed repos — no further PR number check needed
} else {
// Default (legacy) mode: scope to triggering PR only
if (!triggeringPRNumber) {
core.warning("Cannot resolve review thread: not running in a pull request context");
return {
success: false,
error: "Cannot resolve review threads outside of a pull request context",
};
}

if (threadPRNumber !== triggeringPRNumber) {
core.warning(`Thread ${threadId} belongs to PR #${threadPRNumber}, not triggering PR #${triggeringPRNumber}`);
return {
success: false,
error: `Thread belongs to PR #${threadPRNumber}, but only threads on the triggering PR #${triggeringPRNumber} can be resolved`,
};
}
}

core.info(`Resolving review thread: ${threadId} (PR #${triggeringPRNumber})`);
core.info(`Resolving review thread: ${threadId} (PR #${threadPRNumber}${threadRepo ? " in " + threadRepo : ""})`);

// If in staged mode, preview without executing
if (isStaged) {
Expand All @@ -159,7 +242,7 @@ async function main(config = {}) {
staged: true,
previewInfo: {
thread_id: threadId,
pr_number: triggeringPRNumber,
pr_number: threadPRNumber,
},
};
}
Expand Down
Loading
Loading