diff --git a/actions/setup/js/resolve_pr_review_thread.cjs b/actions/setup/js/resolve_pr_review_thread.cjs index b629aec598c..e4f2bd33a97 100644 --- a/actions/setup/js/resolve_pr_review_thread.cjs +++ b/actions/setup/js/resolve_pr_review_thread.cjs @@ -250,6 +250,57 @@ function isIntegrationAccessError(error) { return messages.some(message => message.toLowerCase().includes(integrationErrorFragment)); } +/** + * Check whether an error indicates the referenced GraphQL node no longer exists. + * Review thread node IDs can become stale between the agent turn and the safe-outputs + * replay (thread already resolved, deleted, or superseded), which surfaces as a + * "Could not resolve to a node" or "Not Found" error. These are treated as skippable. + * @param {unknown} error + * @returns {boolean} + */ +function isMissingNodeError(error) { + /** @type {string[]} */ + const messages = [getErrorMessage(error)]; + /** @type {Array<{type?: unknown, message?: unknown, path?: unknown}>} */ + const graphQLErrors = []; + + if (error && typeof error === "object" && "errors" in error && Array.isArray(error.errors)) { + for (const graphQLError of error.errors) { + graphQLErrors.push(graphQLError); + if (typeof graphQLError?.message === "string") { + messages.push(graphQLError.message); + } + } + } + + const hasNodeScopedNotFoundType = graphQLErrors.some(graphQLError => { + if (typeof graphQLError?.type !== "string" || graphQLError.type.toUpperCase() !== "NOT_FOUND") { + return false; + } + const hasNodeScopedMessage = typeof graphQLError?.message === "string" && graphQLError.message.toLowerCase().includes("could not resolve to a node"); + const hasNodeScopedPath = + Array.isArray(graphQLError?.path) && + graphQLError.path.some(pathPart => { + if (typeof pathPart !== "string") return false; + const normalized = pathPart.toLowerCase(); + // GraphQL paths for stale-thread mutation failures are rooted at "resolveReviewThread". + return normalized === "node" || normalized === "resolvereviewthread"; + }); + return hasNodeScopedMessage || hasNodeScopedPath; + }); + if (hasNodeScopedNotFoundType) { + return true; + } + + return messages.some(message => { + const normalized = message.trim().toLowerCase(); + // Match the stale-node GraphQL error, or Octokit's bare "Not Found" 404 message. + // Deliberately avoid a loose "not found" substring match so unrelated errors + // (e.g. "Repository not found") still surface as real failures. + return normalized.includes("could not resolve to a node") || normalized === "not found"; + }); +} + /** * Main handler factory for resolve_pull_request_review_thread * Returns a message handler function that processes individual resolve messages. @@ -323,7 +374,22 @@ async function main(config = {}) { } // Look up the thread's PR number and repository - const threadInfo = await getThreadPullRequestInfo(githubClient, threadId); + /** @type {Awaited>} */ + let threadInfo; + try { + threadInfo = await getThreadPullRequestInfo(githubClient, threadId); + } catch (error) { + if (isMissingNodeError(error)) { + core.info(`Review thread ${threadId} could not be resolved (${getErrorMessage(error)}) — already resolved or stale; skipping`); + return { + success: true, + thread_id: threadId, + is_resolved: true, + skipped: true, + }; + } + throw error; + } if (threadInfo.status === "missing") { core.info(`Review thread ${threadId} not found — already resolved or stale; skipping`); return { @@ -478,6 +544,15 @@ async function main(config = {}) { try { resolveResult = await resolveReviewThreadAPI(githubClient, resolvedThreadId); } catch (error) { + if (isMissingNodeError(error)) { + core.info(`Review thread ${resolvedThreadId} could not be resolved (${getErrorMessage(error)}) — already resolved or stale; skipping`); + return { + success: true, + thread_id: resolvedThreadId, + is_resolved: true, + skipped: true, + }; + } if (isIntegrationAccessError(error)) { const warningMessage = `Skipping resolve_pull_request_review_thread for ${resolvedThreadId}: configuration mismatch ` + diff --git a/actions/setup/js/resolve_pr_review_thread.test.cjs b/actions/setup/js/resolve_pr_review_thread.test.cjs index 4cd8294603a..9f6f1f5e79c 100644 --- a/actions/setup/js/resolve_pr_review_thread.test.cjs +++ b/actions/setup/js/resolve_pr_review_thread.test.cjs @@ -154,6 +154,104 @@ describe("resolve_pr_review_thread", () => { expect(mockGraphql).toHaveBeenCalledTimes(1); }); + it("should succeed as a no-op when the lookup throws a stale-node GraphQL error", async () => { + mockGraphql.mockImplementation(() => { + const error = new Error("Request failed"); + error.errors = [{ type: "NOT_FOUND", message: "Could not resolve to a node with the global id of 'PRRT_kwDOPc1QR87fJc0o'" }]; + return Promise.reject(error); + }); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const result = await freshHandler({ type: "resolve_pull_request_review_thread", thread_id: "PRRT_kwDOPc1QR87fJc0o" }, {}); + + expect(result.success).toBe(true); + expect(result.skipped).toBe(true); + expect(result.is_resolved).toBe(true); + expect(mockCore.error).not.toHaveBeenCalled(); + }); + + it("should succeed as a no-op when the lookup throws a plain Not Found error", async () => { + mockGraphql.mockImplementation(() => Promise.reject(new Error("Not Found"))); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const result = await freshHandler({ type: "resolve_pull_request_review_thread", thread_id: "PRRT_stale" }, {}); + + expect(result.success).toBe(true); + expect(result.skipped).toBe(true); + expect(mockCore.error).not.toHaveBeenCalled(); + }); + + it("should succeed as a no-op when the resolve mutation reports a stale node", async () => { + mockGraphql.mockImplementation(query => { + if (query.includes("resolveReviewThread")) { + const error = new Error("Could not resolve to a node with the global id of 'PRRT_kwDOABCD123456'"); + return Promise.reject(error); + } + return Promise.resolve({ + node: { + __typename: "PullRequestReviewThread", + id: "PRRT_kwDOABCD123456", + isResolved: false, + pullRequest: { number: 42, repository: { nameWithOwner: "test-owner/test-repo" } }, + }, + }); + }); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const result = await freshHandler({ type: "resolve_pull_request_review_thread", thread_id: "PRRT_kwDOABCD123456" }, {}); + + expect(result.success).toBe(true); + expect(result.skipped).toBe(true); + expect(result.is_resolved).toBe(true); + expect(mockCore.error).not.toHaveBeenCalled(); + }); + + it("should still fail for unrelated lookup errors", async () => { + mockGraphql.mockImplementation(() => Promise.reject(new Error("Internal server error"))); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const result = await freshHandler({ type: "resolve_pull_request_review_thread", thread_id: "PRRT_kwDOABCD123456" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Internal server error"); + }); + + it("should still fail for unrelated 'not found' errors such as a missing repository", async () => { + mockGraphql.mockImplementation(() => Promise.reject(new Error("Repository not found"))); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const result = await freshHandler({ type: "resolve_pull_request_review_thread", thread_id: "PRRT_kwDOABCD123456" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Repository not found"); + }); + + it("should still fail for structured NOT_FOUND errors unrelated to stale thread nodes", async () => { + mockGraphql.mockImplementation(() => { + const error = new Error("Repository not found"); + error.errors = [{ type: "NOT_FOUND", message: "Repository not found", path: ["repository"] }]; + return Promise.reject(error); + }); + + const { main } = require("./resolve_pr_review_thread.cjs"); + const freshHandler = await main({ max: 10 }); + + const result = await freshHandler({ type: "resolve_pull_request_review_thread", thread_id: "PRRT_kwDOABCD123456" }, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Repository not found"); + }); + it("should resolve a review comment node ID by finding its parent thread", async () => { mockGraphql.mockImplementation(query => { if (query.includes("resolveReviewThread")) { @@ -582,7 +680,7 @@ describe("resolve_pr_review_thread", () => { }); it("should handle API errors gracefully", async () => { - mockGraphql.mockRejectedValue(new Error("Could not resolve. Thread not found.")); + mockGraphql.mockRejectedValue(new Error("Internal server error while loading thread")); const message = { type: "resolve_pull_request_review_thread", @@ -592,7 +690,7 @@ describe("resolve_pr_review_thread", () => { const result = await handler(message, {}); expect(result.success).toBe(false); - expect(result.error).toContain("Could not resolve"); + expect(result.error).toContain("Internal server error"); }); it("should soft-skip when resolve mutation returns integration access error", async () => {