Skip to content

[plan] Fix resolve_pull_request_review_thread Context Check #26267

@github-actions

Description

@github-actions

Objective

Fix the resolve_pull_request_review_thread safe output handler so it does not reject calls when an explicit thread_id is provided but the workflow was not triggered by a pull request event.

Context

From discussion #26222 (Safe Output Health Report 2026-04-14):

The Smoke Claude workflow runs on a schedule trigger (not a PR event). When it calls resolve_pull_request_review_thread with an explicit thread_id, the handler fails with:

Cannot resolve review threads outside of a pull request context

The handler correctly resolves the thread's PR number and repo from the GraphQL API (via getThreadPullRequestInfo), but then falls into the default/legacy mode branch which requires triggeringPRNumber — a value only available in PR-triggered workflows.

Root Cause

In actions/setup/js/resolve_pr_review_thread.cjs, the default (legacy) mode path (lines ~217–233) always requires triggeringPRNumber:

} 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",
    };
  }
  // ...
}

When no triggeringPRNumber exists (schedule/manual triggers), the handler rejects the call even though it already has all the information needed to resolve the thread (the explicit thread_id maps to a specific PR via the API).

Fix

File: actions/setup/js/resolve_pr_review_thread.cjs

In the default/legacy mode branch, when triggeringPRNumber is not available, allow the resolution to proceed if a valid thread_id was provided and the thread was successfully resolved to a specific PR via getThreadPullRequestInfo. The guard should only block when there is no thread_id AND no triggering PR context (which is already handled earlier by the thread_id validation at the top of the handler).

Specifically, in the else (default legacy) block:

  • If triggeringPRNumber is null/undefined, emit an info log (not an error) noting that there is no triggering PR, but still allow the resolve to proceed since the explicit thread_id was resolved to threadPRNumber via the API.
  • Only fail if the matching check is needed and threadPRNumber !== triggeringPRNumber (i.e., skip the PR number check entirely when there is no triggering PR).

Files to Modify

  • actions/setup/js/resolve_pr_review_thread.cjs — fix the default/legacy context check
  • actions/setup/js/resolve_pr_review_thread.test.cjs — add/update test covering schedule-triggered context (no triggering PR, explicit thread_id provided)

Acceptance Criteria

  • resolve_pull_request_review_thread succeeds when thread_id is explicitly provided, even if the workflow was not triggered by a PR event (e.g., schedule trigger)
  • Handler still fails with a clear error when thread_id is missing or invalid
  • Handler still validates that the thread belongs to the triggering PR when a triggering PR context IS available (default mode behavior preserved)
  • Existing tests continue to pass
  • New test covers the schedule-triggered scenario (no PR context, explicit thread_id)

References

Generated by Plan Command for issue #discussion #26222 · ● 287.1K ·

  • expires on Apr 16, 2026, 5:18 PM UTC

Metadata

Metadata

Labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions