From 157c4f6f4d5d15e93c12639890a34b7fd01eb7c7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:30:08 +0000 Subject: [PATCH 1/7] Initial plan From aa8d26c35575c00996a7743f120c38d24157d1e6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:49:57 +0000 Subject: [PATCH 2/7] feat: add commit-id config option to submit-pull-request-review and create-pull-request-review-comment Adds a `commit-id` configuration option to both `submit-pull-request-review` and `create-pull-request-review-comment` safe output handlers. When set, the configured SHA is used as the `commit_id` for `pulls.createReview()` instead of the live PR head SHA fetched during `safe_outputs`. This prevents attribution drift in workflow_run scenarios where a new commit may be pushed to the PR during the agent's run. Workflow authors can pin the review to the exact commit that was checked out and reviewed: ```yaml safe-outputs: submit-pull-request-review: commit-id: ${{ needs.eligibility.outputs.head_sha }} ``` Changes: - Go compiler: add CommitId field to SubmitPullRequestReviewConfig and CreatePullRequestReviewCommentsConfig, parse commit-id from YAML config, emit commit_id in handler registry JSON - JS runtime: read config.commit_id in submit_pr_review.cjs and create_pr_review_comment.cjs, store as commitId on ReviewContext - JS buffer: use reviewContext.commitId || pullRequest.head.sha as commit_id when calling pulls.createReview() - Tests: add unit tests for all new behavior in Go and JavaScript Closes #48661 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pr_review_comment.cjs | 10 + .../js/create_pr_review_comment.test.cjs | 37 ++++ actions/setup/js/pr_review_buffer.cjs | 13 +- actions/setup/js/pr_review_buffer.test.cjs | 49 +++++ actions/setup/js/submit_pr_review.cjs | 10 + actions/setup/js/submit_pr_review.test.cjs | 148 +++++++++++++++ pkg/workflow/create_pr_review_comment.go | 9 + pkg/workflow/safe_outputs_handler_registry.go | 2 + pkg/workflow/submit_pr_review.go | 9 + pkg/workflow/submit_pr_review_footer_test.go | 174 ++++++++++++++++++ 10 files changed, 459 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index e33a5f43cbb..b523bbc32f4 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -42,6 +42,15 @@ async function main(config = {}) { const requiredTitlePrefix = config.required_title_prefix || ""; if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`); + + // Optional pinned commit SHA. When set, overrides the live PR head SHA so that the + // review is attributed to the commit the agent actually reviewed, not whatever head + // the PR has at post time (which may differ under workflow_run triggers). + const pinnedCommitId = typeof config.commit_id === "string" && config.commit_id.trim() ? config.commit_id.trim() : null; + if (pinnedCommitId) { + core.info(`create_pull_request_review_comment: using pinned commit-id: ${pinnedCommitId}`); + } + let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -343,6 +352,7 @@ async function main(config = {}) { repoParts: repoParts, pullRequestNumber: pullRequestNumber, pullRequest: pullRequest, + ...(pinnedCommitId ? { commitId: pinnedCommitId } : {}), }); // Buffer the comment instead of posting it individually diff --git a/actions/setup/js/create_pr_review_comment.test.cjs b/actions/setup/js/create_pr_review_comment.test.cjs index e18bb40d1df..189577e4e52 100644 --- a/actions/setup/js/create_pr_review_comment.test.cjs +++ b/actions/setup/js/create_pr_review_comment.test.cjs @@ -523,6 +523,43 @@ describe("create_pr_review_comment.cjs", () => { // Footer context is set on the buffer for review-level footer generation expect(buffer.getBufferedCount()).toBe(1); }); + + it("should store pinned commitId in review context when commit_id is configured", async () => { + const handler = await createHandler({ commit_id: "pinned-sha-xyz789" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Review comment body", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + + const ctx = buffer.getReviewContext(); + expect(ctx).not.toBeNull(); + // The pinned commitId must be stored in the review context + expect(ctx.commitId).toBe("pinned-sha-xyz789"); + // The live PR head SHA should still be in pullRequest.head.sha + expect(ctx.pullRequest.head.sha).toBe("abc123def456"); + }); + + it("should not set commitId when commit_id config is not provided", async () => { + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Review comment without pinning", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + const ctx = buffer.getReviewContext(); + expect(ctx).not.toBeNull(); + expect(ctx.commitId).toBeUndefined(); + }); }); describe("create_pr_review_comment.cjs — registry mode (multiple reviews)", () => { diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 3a3e3cf2a51..2383e0c0abe 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -82,6 +82,7 @@ const REVIEW_RATE_LIMIT_RETRY_CONFIG = { * @property {{owner: string, repo: string}} repoParts - Parsed owner and repo * @property {number} pullRequestNumber - PR number * @property {Object} pullRequest - Full PR object with head.sha + * @property {string} [commitId] - Optional commit SHA override. When present, used instead of pullRequest.head.sha as the commit_id for pulls.createReview(). Pins the review to the commit the agent actually reviewed, preventing attribution drift when new commits are pushed during the run. */ /** @@ -291,13 +292,21 @@ function createReviewBuffer() { }; } - const { repo, repoParts, pullRequestNumber, pullRequest } = reviewContext; + const { repo, repoParts, pullRequestNumber, pullRequest, commitId } = reviewContext; if (!pullRequest || !pullRequest.head || !pullRequest.head.sha) { core.warning("Pull request head SHA not available - cannot submit review"); return { success: false, error: "Pull request head SHA not available" }; } + // Use the pinned commit ID when configured, falling back to the live PR head SHA. + // A pinned commitId ensures the review is attributed to the commit the agent actually + // reviewed, preventing attribution drift when new commits are pushed during the run. + const resolvedCommitId = commitId || pullRequest.head.sha; + if (commitId) { + core.info(`Using pinned commit ID for review: ${commitId} (PR head is ${pullRequest.head.sha})`); + } + // Determine review event and body let event = reviewMetadata ? reviewMetadata.event : "COMMENT"; let body = reviewMetadata ? reviewMetadata.body : ""; @@ -465,7 +474,7 @@ function createReviewBuffer() { owner: repoParts.owner, repo: repoParts.repo, pull_number: pullRequestNumber, - commit_id: pullRequest.head.sha, + commit_id: resolvedCommitId, event: event, }; diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index 31079debc65..8c62dff7fda 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -556,6 +556,55 @@ describe("pr_review_buffer (factory pattern)", () => { expect(callArgs.comments).toBeUndefined(); }); + it("should use pinned commitId from review context instead of live PR head SHA", async () => { + buffer.setReviewMetadata("Review with pinned commit", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "live-head-sha-pushed-later" } }, + commitId: "original-reviewed-sha", + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 700, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-700", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + // The pinned sha must be passed as commit_id, not the live head sha + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.commit_id).toBe("original-reviewed-sha"); + }); + + it("should fall back to pullRequest.head.sha when commitId is not set in review context", async () => { + buffer.setReviewMetadata("Regular review", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "current-head-sha" } }, + // No commitId field + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 800, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-800", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.commit_id).toBe("current-head-sha"); + }); + it("should include multi-line comment fields with side fallback for start_side", async () => { buffer.addComment({ path: "src/index.js", diff --git a/actions/setup/js/submit_pr_review.cjs b/actions/setup/js/submit_pr_review.cjs index d9d8e8a01d8..bb7c3031b11 100644 --- a/actions/setup/js/submit_pr_review.cjs +++ b/actions/setup/js/submit_pr_review.cjs @@ -44,6 +44,14 @@ async function main(config = {}) { const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); + // Optional pinned commit SHA. When set, overrides the live PR head SHA so that the + // review is attributed to the commit the agent actually reviewed, not whatever head + // the PR has at post time (which may differ under workflow_run triggers). + const pinnedCommitId = typeof config.commit_id === "string" && config.commit_id.trim() ? config.commit_id.trim() : null; + if (pinnedCommitId) { + core.info(`submit_pull_request_review: using pinned commit-id: ${pinnedCommitId}`); + } + const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : []; const requiredTitlePrefix = config.required_title_prefix || ""; if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); @@ -276,6 +284,7 @@ async function main(config = {}) { repoParts, pullRequestNumber: payloadPR.number, pullRequest: payloadPR, + ...(pinnedCommitId ? { commitId: pinnedCommitId } : {}), }); core.info(`Set review context from triggering PR: ${repo}#${payloadPR.number}`); } else { @@ -291,6 +300,7 @@ async function main(config = {}) { repoParts, pullRequestNumber: fetchedPR.number, pullRequest: fetchedPR, + ...(pinnedCommitId ? { commitId: pinnedCommitId } : {}), }); core.info(`Set review context from target: ${repo}#${fetchedPR.number}`); } else { diff --git a/actions/setup/js/submit_pr_review.test.cjs b/actions/setup/js/submit_pr_review.test.cjs index 6b019debebb..78bfa3b1274 100644 --- a/actions/setup/js/submit_pr_review.test.cjs +++ b/actions/setup/js/submit_pr_review.test.cjs @@ -876,4 +876,152 @@ describe("submit_pr_review multi-buffer (registry mode)", () => { await main({ max: 1, target: "*", supersede_older_reviews: true, _prReviewBufferRegistry: registry }); expect(setSuperSpy).toHaveBeenCalledWith(true); }); + + it("pinned commit_id is stored in review context for each buffer (registry mode)", async () => { + const registry = createPrReviewBufferRegistry(); + const { main } = require("./submit_pr_review.cjs"); + const handler = await main({ + max: 5, + target: "*", + commit_id: "pinned-sha-abc123", + _prReviewBufferRegistry: registry, + }); + + await handler({ body: "Review for PR 10", event: "COMMENT", pull_request_number: 10 }, {}); + + const entries = registry.getAllEntries(); + expect(entries).toHaveLength(1); + const ctx = entries[0].buffer.getReviewContext(); + expect(ctx).not.toBeNull(); + expect(ctx.commitId).toBe("pinned-sha-abc123"); + }); +}); + +describe("submit_pr_review: commit-id pinning", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + delete global.context; + delete global.github; + }); + + it("should store pinned commitId in review context from triggering PR payload", async () => { + global.context = { + eventName: "pull_request", + repo: { owner: "test-owner", repo: "test-repo" }, + payload: { + pull_request: { number: 42, head: { sha: "live-head-sha" } }, + }, + }; + global.github = { + rest: { pulls: { get: vi.fn() } }, + graphql: vi.fn(), + }; + + const localBuffer = createReviewBuffer(); + const { main } = require("./submit_pr_review.cjs"); + const localHandler = await main({ + max: 1, + _prReviewBuffer: localBuffer, + commit_id: "pinned-sha-from-eligibility", + }); + + const result = await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); + + expect(result.success).toBe(true); + const ctx = localBuffer.getReviewContext(); + expect(ctx).not.toBeNull(); + // commitId should be the pinned value, not the live head SHA + expect(ctx.commitId).toBe("pinned-sha-from-eligibility"); + // The payload SHA should still be present in pullRequest.head.sha + expect(ctx.pullRequest.head.sha).toBe("live-head-sha"); + // github.rest.pulls.get should NOT have been called (payload PR was used) + expect(global.github.rest.pulls.get).not.toHaveBeenCalled(); + }); + + it("should store pinned commitId in review context fetched from API (workflow_run scenario)", async () => { + const fetchedPR = { number: 55, head: { sha: "live-head-after-push" } }; + global.context = { + eventName: "workflow_run", + repo: { owner: "org", repo: "repo" }, + payload: {}, + }; + global.github = { + rest: { + pulls: { + get: vi.fn().mockResolvedValue({ data: fetchedPR }), + }, + }, + graphql: vi.fn(), + }; + + const localBuffer = createReviewBuffer(); + const { main } = require("./submit_pr_review.cjs"); + const localHandler = await main({ + max: 1, + target: "55", + _prReviewBuffer: localBuffer, + commit_id: "original-reviewed-sha", + }); + + const result = await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); + + expect(result.success).toBe(true); + const ctx = localBuffer.getReviewContext(); + expect(ctx).not.toBeNull(); + // commitId should be the pinned value, NOT the live head SHA returned by the API + expect(ctx.commitId).toBe("original-reviewed-sha"); + // The live head SHA is still stored in pullRequest (used for other purposes) + expect(ctx.pullRequest.head.sha).toBe("live-head-after-push"); + }); + + it("should not set commitId when commit_id config is not provided", async () => { + global.context = { + eventName: "pull_request", + repo: { owner: "test-owner", repo: "test-repo" }, + payload: { + pull_request: { number: 7, head: { sha: "head-sha" } }, + }, + }; + global.github = { + rest: { pulls: { get: vi.fn() } }, + graphql: vi.fn(), + }; + + const localBuffer = createReviewBuffer(); + const { main } = require("./submit_pr_review.cjs"); + const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer }); + + await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); + + const ctx = localBuffer.getReviewContext(); + expect(ctx).not.toBeNull(); + expect(ctx.commitId).toBeUndefined(); + }); + + it("should not set commitId when commit_id is empty string", async () => { + global.context = { + eventName: "pull_request", + repo: { owner: "test-owner", repo: "test-repo" }, + payload: { + pull_request: { number: 7, head: { sha: "head-sha" } }, + }, + }; + global.github = { + rest: { pulls: { get: vi.fn() } }, + graphql: vi.fn(), + }; + + const localBuffer = createReviewBuffer(); + const { main } = require("./submit_pr_review.cjs"); + const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer, commit_id: "" }); + + await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); + + const ctx = localBuffer.getReviewContext(); + expect(ctx).not.toBeNull(); + expect(ctx.commitId).toBeUndefined(); + }); }); diff --git a/pkg/workflow/create_pr_review_comment.go b/pkg/workflow/create_pr_review_comment.go index 4c281831f5c..cd34efc79c8 100644 --- a/pkg/workflow/create_pr_review_comment.go +++ b/pkg/workflow/create_pr_review_comment.go @@ -14,6 +14,7 @@ type CreatePullRequestReviewCommentsConfig struct { Target string `yaml:"target,omitempty"` // Target for comments: "triggering" (default), "*" (any PR), or explicit PR number TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository PR review comments AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that PR review comments can be added to (additionally to the target-repo) + CommitId string `yaml:"commit-id,omitempty"` // Optional commit SHA to use as the reviewed commit. Pins the review to the commit the agent actually reviewed, preventing attribution drift when new commits are pushed during the run. } func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any) *CreatePullRequestReviewCommentsConfig { @@ -52,6 +53,14 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any } prReviewCommentsConfig.TargetRepoSlug = targetRepoSlug + // Parse optional commit-id override + if commitId, exists := configMap["commit-id"]; exists { + if commitIdStr, ok := commitId.(string); ok && commitIdStr != "" { + prReviewCommentsConfig.CommitId = commitIdStr + createPRReviewCommentLog.Printf("Commit ID override: %s", commitIdStr) + } + } + // Parse common base fields with default max of 10 c.parseBaseSafeOutputConfig(configMap, &prReviewCommentsConfig.BaseSafeOutputConfig, 10) } else { diff --git a/pkg/workflow/safe_outputs_handler_registry.go b/pkg/workflow/safe_outputs_handler_registry.go index cba92bfb055..64ae8a9f83d 100644 --- a/pkg/workflow/safe_outputs_handler_registry.go +++ b/pkg/workflow/safe_outputs_handler_registry.go @@ -425,6 +425,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_repos", c.AllowedRepos). AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). + AddIfNotEmpty("commit_id", c.CommitId). Build() }, "submit_pull_request_review": func(cfg *SafeOutputsConfig) map[string]any { @@ -442,6 +443,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).AddIfNotEmpty("github-token", c.GitHubToken). AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). + AddIfNotEmpty("commit_id", c.CommitId). Build() }, "reply_to_pull_request_review_comment": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go index 18ecdf44997..54714ab9f1f 100644 --- a/pkg/workflow/submit_pr_review.go +++ b/pkg/workflow/submit_pr_review.go @@ -20,6 +20,7 @@ type SubmitPullRequestReviewConfig struct { Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text) AllowedEvents []string `yaml:"allowed-events,omitempty"` // Optional list of allowed review event types: APPROVE, COMMENT, REQUEST_CHANGES. If omitted, all event types are allowed. SupersedeOlderReviews bool `yaml:"supersede-older-reviews,omitempty"` // When true, dismisses older same-workflow REQUEST_CHANGES reviews after a replacement review is posted. + CommitId string `yaml:"commit-id,omitempty"` // Optional commit SHA to use as the reviewed commit. Pins the review to the commit the agent actually reviewed, preventing attribution drift when new commits are pushed during the run. } // parseSubmitPullRequestReviewConfig handles submit-pull-request-review configuration @@ -112,6 +113,14 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) } } + // Parse optional commit-id override + if commitId, exists := configMap["commit-id"]; exists { + if commitIdStr, ok := commitId.(string); ok && commitIdStr != "" { + config.CommitId = commitIdStr + submitPRReviewLog.Printf("Commit ID override: %s", commitIdStr) + } + } + submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s, allowed_events=%v, supersede_older_reviews=%t", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug, config.AllowedEvents, config.SupersedeOlderReviews) } else { // If configData is nil or not a map, set the default max diff --git a/pkg/workflow/submit_pr_review_footer_test.go b/pkg/workflow/submit_pr_review_footer_test.go index 8b6db8d56fd..c49ed69acab 100644 --- a/pkg/workflow/submit_pr_review_footer_test.go +++ b/pkg/workflow/submit_pr_review_footer_test.go @@ -622,3 +622,177 @@ func TestSubmitPRReviewFooterInHandlerConfig(t *testing.T) { } }) } + +func TestParseCommitIdConfig(t *testing.T) { + t.Run("parses commit-id for submit-pull-request-review", func(t *testing.T) { + compiler := NewCompiler() + outputMap := map[string]any{ + "submit-pull-request-review": map[string]any{ + "max": 1, + "commit-id": "abc123def456", + }, + } + + config := compiler.parseSubmitPullRequestReviewConfig(outputMap) + require.NotNil(t, config, "Config should be parsed") + assert.Equal(t, "abc123def456", config.CommitId, "CommitId should be parsed") + }) + + t.Run("commit-id empty when not provided for submit-pull-request-review", func(t *testing.T) { + compiler := NewCompiler() + outputMap := map[string]any{ + "submit-pull-request-review": map[string]any{ + "max": 1, + }, + } + + config := compiler.parseSubmitPullRequestReviewConfig(outputMap) + require.NotNil(t, config, "Config should be parsed") + assert.Empty(t, config.CommitId, "CommitId should be empty when not configured") + }) + + t.Run("parses commit-id for create-pull-request-review-comment", func(t *testing.T) { + compiler := NewCompiler() + outputMap := map[string]any{ + "create-pull-request-review-comment": map[string]any{ + "commit-id": "deadbeef1234", + }, + } + + config := compiler.parsePullRequestReviewCommentsConfig(outputMap) + require.NotNil(t, config, "Config should be parsed") + assert.Equal(t, "deadbeef1234", config.CommitId, "CommitId should be parsed") + }) + + t.Run("commit-id empty when not provided for create-pull-request-review-comment", func(t *testing.T) { + compiler := NewCompiler() + outputMap := map[string]any{ + "create-pull-request-review-comment": map[string]any{ + "side": "RIGHT", + }, + } + + config := compiler.parsePullRequestReviewCommentsConfig(outputMap) + require.NotNil(t, config, "Config should be parsed") + assert.Empty(t, config.CommitId, "CommitId should be empty when not configured") + }) + + t.Run("commit-id emitted in submit_pull_request_review handler config", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + CommitId: "pinned-sha-abc123", + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + require.NotEmpty(t, steps, "Steps should not be empty") + + for _, step := range steps { + if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") + if len(parts) == 2 { + jsonStr := strings.TrimSpace(parts[1]) + jsonStr = strings.Trim(jsonStr, "\"") + jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") + var handlerConfig map[string]any + err := json.Unmarshal([]byte(jsonStr), &handlerConfig) + require.NoError(t, err, "Should unmarshal handler config") + + submitConfig, ok := handlerConfig["submit_pull_request_review"].(map[string]any) + require.True(t, ok, "submit_pull_request_review config should exist") + assert.Equal(t, "pinned-sha-abc123", submitConfig["commit_id"], "commit_id should be emitted in handler config") + } + } + } + }) + + t.Run("commit-id not emitted in submit_pull_request_review handler config when empty", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + require.NotEmpty(t, steps, "Steps should not be empty") + + for _, step := range steps { + if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") + if len(parts) == 2 { + jsonStr := strings.TrimSpace(parts[1]) + jsonStr = strings.Trim(jsonStr, "\"") + jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") + var handlerConfig map[string]any + err := json.Unmarshal([]byte(jsonStr), &handlerConfig) + require.NoError(t, err, "Should unmarshal handler config") + + submitConfig, ok := handlerConfig["submit_pull_request_review"].(map[string]any) + require.True(t, ok, "submit_pull_request_review config should exist") + _, hasCommitId := submitConfig["commit_id"] + assert.False(t, hasCommitId, "commit_id should not be in handler config when not set") + } + } + } + }) + + t.Run("commit-id emitted in create_pull_request_review_comment handler config", func(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + Name: "Test", + SafeOutputs: &SafeOutputsConfig{ + CreatePullRequestReviewComments: &CreatePullRequestReviewCommentsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("10")}, + CommitId: "pinned-sha-for-comments", + }, + }, + } + + var steps []string + compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) + require.NotEmpty(t, steps, "Steps should not be empty") + + for _, step := range steps { + if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { + parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") + if len(parts) == 2 { + jsonStr := strings.TrimSpace(parts[1]) + jsonStr = strings.Trim(jsonStr, "\"") + jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") + var handlerConfig map[string]any + err := json.Unmarshal([]byte(jsonStr), &handlerConfig) + require.NoError(t, err, "Should unmarshal handler config") + + reviewCommentConfig, ok := handlerConfig["create_pull_request_review_comment"].(map[string]any) + require.True(t, ok, "create_pull_request_review_comment config should exist") + assert.Equal(t, "pinned-sha-for-comments", reviewCommentConfig["commit_id"], "commit_id should be emitted in review comment handler config") + } + } + } + }) + + t.Run("accepts GitHub expression as commit-id value for submit-pull-request-review", func(t *testing.T) { + compiler := NewCompiler() + outputMap := map[string]any{ + "submit-pull-request-review": map[string]any{ + "max": 1, + "commit-id": "${{ needs.eligibility.outputs.head_sha }}", + }, + } + + config := compiler.parseSubmitPullRequestReviewConfig(outputMap) + require.NotNil(t, config, "Config should be parsed") + assert.Equal(t, "${{ needs.eligibility.outputs.head_sha }}", config.CommitId, "CommitId should accept GitHub expression") + }) +} From 34f3236c27e8969af98b2b9697918f9035df96e0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:02:13 +0000 Subject: [PATCH 3/7] docs: add draft ADR-48738 for commit-id review attribution pinning --- ...n-review-attribution-to-reviewed-commit.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 docs/adr/48738-pin-review-attribution-to-reviewed-commit.md diff --git a/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md b/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md new file mode 100644 index 00000000000..56d457c0fcb --- /dev/null +++ b/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md @@ -0,0 +1,48 @@ +# ADR-48738: Pin PR Review Attribution to the Reviewed Commit via Optional `commit-id` Config + +**Date**: 2026-07-28 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +Under `workflow_run` triggers, the safe-outputs runtime submits a PR review after a parent workflow completes. A new commit can land on the PR branch while the agent is running. When that happens, `pulls.get()` returns the new HEAD SHA, and the review is attributed to a commit the agent never actually reviewed — making inline comments appear fabricated, outdated, or misaligned. There was no mechanism for callers to pass the specific SHA the agent reviewed to the review submission step. This race condition is most acute in `workflow_run` contexts where an eligibility job captures the PR HEAD at trigger time and passes it to downstream jobs. + +### Decision + +We will add an optional `commit-id` configuration field to both `submit-pull-request-review` and `create-pull-request-review-comment` safe-output handlers. When set, this SHA is used as the `commit_id` argument to `pulls.createReview()` instead of the live PR head SHA. The field is optional and backward-compatible: when omitted, behavior is unchanged (the live PR head SHA is used). Callers explicitly pass the reviewed SHA — typically captured by an upstream eligibility job — via `commit-id: ${{ needs.eligibility.outputs.head_sha }}` in their workflow YAML. + +### Alternatives Considered + +#### Alternative 1: Automatically capture and freeze the PR HEAD SHA at agent startup + +Freeze the SHA once when the safe-outputs handler initialises, before the review accumulation phase begins, and always use that frozen value. This would require no caller-side configuration. + +Why not chosen: The agent may run for an extended period before submitting the review. A commit pushed after handler initialisation but during the agent run would still cause attribution drift. More importantly, in `workflow_run` scenarios the correct SHA to attribute is the one the *triggering* workflow saw — which may already be older than what the safe-outputs job sees at startup. That SHA is only knowable by the caller via an upstream job output, not by the handler itself. + +#### Alternative 2: Always use the triggering `workflow_run` payload SHA + +For `workflow_run` events, automatically extract the SHA from the triggering workflow's context payload instead of calling `pulls.get()`. + +Why not chosen: The triggering payload does not always carry the SHA of the PR commit the agent reviewed (it may contain the SHA of the triggering workflow's default branch commit). Callers need explicit control over which commit to attribute, particularly when the reviewed SHA is computed by a separate eligibility job and stored as a job output. This approach also does not address non-`workflow_run` triggers that may have similar drift in edge cases. + +### Consequences + +#### Positive +- Reviews are attributed to the commit the agent actually reviewed, eliminating false "outdated" or misaligned inline comments. +- GitHub correctly marks the review as outdated when HEAD moves, rather than incorrectly attaching stale comments to the new commit. +- The feature is fully opt-in and backward-compatible — existing callers are unaffected. + +#### Negative +- Callers must explicitly wire the reviewed SHA through their workflow YAML (e.g., from an eligibility job output); forgetting to set `commit-id` leaves the race condition unaddressed in `workflow_run` contexts. +- The `commit-id` field is threaded through three layers (Go config struct → handler registry JSON → JS runtime), increasing the surface area of the config pipeline. + +#### Neutral +- The Go config structs (`SubmitPullRequestReviewConfig`, `CreatePullRequestReviewCommentsConfig`) gain a new `CommitId string` field, which is zero-valued (empty) when not configured — no breaking change to existing parsed configs. +- The JS runtime falls back to `pullRequest.head.sha` when `commitId` is absent, preserving existing runtime behaviour. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* From 74843e1089c2c34df73877f108ffcd9679f1e6c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:28:19 +0000 Subject: [PATCH 4/7] refactor: replace user-facing commit-id YAML config with automatic GH_AW_HEAD_SHA env var injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler now automatically injects GH_AW_HEAD_SHA into the safe_outputs job env based on trigger type (workflow_run → github.event.workflow_run.head_sha, pull_request/pull_request_target → github.event.pull_request.head.sha). The pr_review_buffer.cjs submitReview() function reads process.env.GH_AW_HEAD_SHA and uses it as commit_id when set, falling back to pullRequest.head.sha. This removes the need for users to add commit-id to their workflow YAML and resolves all reviewer concerns about multi-target ambiguity and first-wins context coupling. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pr_review_comment.cjs | 9 - .../js/create_pr_review_comment.test.cjs | 52 +++-- actions/setup/js/pr_review_buffer.cjs | 18 +- actions/setup/js/pr_review_buffer.test.cjs | 94 +++++---- actions/setup/js/submit_pr_review.cjs | 10 - actions/setup/js/submit_pr_review.test.cjs | 86 +------- ...n-review-attribution-to-reviewed-commit.md | 47 +++-- pkg/workflow/compiler_safe_outputs_job.go | 40 ++++ pkg/workflow/create_pr_review_comment.go | 9 - pkg/workflow/safe_outputs_handler_registry.go | 2 - pkg/workflow/submit_pr_review.go | 9 - pkg/workflow/submit_pr_review_footer_test.go | 199 ++++++------------ 12 files changed, 239 insertions(+), 336 deletions(-) diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index b523bbc32f4..a11fca3db2b 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -43,14 +43,6 @@ async function main(config = {}) { if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); if (requiredTitlePrefix) core.info(`Required title prefix: ${requiredTitlePrefix}`); - // Optional pinned commit SHA. When set, overrides the live PR head SHA so that the - // review is attributed to the commit the agent actually reviewed, not whatever head - // the PR has at post time (which may differ under workflow_run triggers). - const pinnedCommitId = typeof config.commit_id === "string" && config.commit_id.trim() ? config.commit_id.trim() : null; - if (pinnedCommitId) { - core.info(`create_pull_request_review_comment: using pinned commit-id: ${pinnedCommitId}`); - } - let allowedMentionAliases = []; if (Array.isArray(config.allowedMentionAliases)) { allowedMentionAliases = config.allowedMentionAliases; @@ -352,7 +344,6 @@ async function main(config = {}) { repoParts: repoParts, pullRequestNumber: pullRequestNumber, pullRequest: pullRequest, - ...(pinnedCommitId ? { commitId: pinnedCommitId } : {}), }); // Buffer the comment instead of posting it individually diff --git a/actions/setup/js/create_pr_review_comment.test.cjs b/actions/setup/js/create_pr_review_comment.test.cjs index 189577e4e52..694cba41c66 100644 --- a/actions/setup/js/create_pr_review_comment.test.cjs +++ b/actions/setup/js/create_pr_review_comment.test.cjs @@ -524,28 +524,40 @@ describe("create_pr_review_comment.cjs", () => { expect(buffer.getBufferedCount()).toBe(1); }); - it("should store pinned commitId in review context when commit_id is configured", async () => { - const handler = await createHandler({ commit_id: "pinned-sha-xyz789" }); - const message = { - type: "create_pull_request_review_comment", - path: "src/main.js", - line: 10, - body: "Review comment body", - }; - const result = await handler(message, {}); - - expect(result.success).toBe(true); - expect(result.buffered).toBe(true); - - const ctx = buffer.getReviewContext(); - expect(ctx).not.toBeNull(); - // The pinned commitId must be stored in the review context - expect(ctx.commitId).toBe("pinned-sha-xyz789"); - // The live PR head SHA should still be in pullRequest.head.sha - expect(ctx.pullRequest.head.sha).toBe("abc123def456"); + it("should use GH_AW_HEAD_SHA env var as commit_id when submitting review", async () => { + const previousHeadSHA = process.env.GH_AW_HEAD_SHA; + process.env.GH_AW_HEAD_SHA = "trigger-time-sha-xyz789"; + try { + // The buffer's submitReview uses GH_AW_HEAD_SHA automatically — verify the + // review context is set correctly (no commitId field needed on context) + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Review comment body", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + + const ctx = buffer.getReviewContext(); + expect(ctx).not.toBeNull(); + // No commitId on the context; the env var is used at submit time instead + expect(ctx.commitId).toBeUndefined(); + // The live PR head SHA should still be in pullRequest.head.sha + expect(ctx.pullRequest.head.sha).toBe("abc123def456"); + } finally { + if (previousHeadSHA !== undefined) { + process.env.GH_AW_HEAD_SHA = previousHeadSHA; + } else { + delete process.env.GH_AW_HEAD_SHA; + } + } }); - it("should not set commitId when commit_id config is not provided", async () => { + it("review context has no commitId field regardless of handler config", async () => { const handler = await createHandler(); const message = { type: "create_pull_request_review_comment", diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 2383e0c0abe..8a830cfede6 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -82,7 +82,6 @@ const REVIEW_RATE_LIMIT_RETRY_CONFIG = { * @property {{owner: string, repo: string}} repoParts - Parsed owner and repo * @property {number} pullRequestNumber - PR number * @property {Object} pullRequest - Full PR object with head.sha - * @property {string} [commitId] - Optional commit SHA override. When present, used instead of pullRequest.head.sha as the commit_id for pulls.createReview(). Pins the review to the commit the agent actually reviewed, preventing attribution drift when new commits are pushed during the run. */ /** @@ -292,19 +291,22 @@ function createReviewBuffer() { }; } - const { repo, repoParts, pullRequestNumber, pullRequest, commitId } = reviewContext; + const { repo, repoParts, pullRequestNumber, pullRequest } = reviewContext; if (!pullRequest || !pullRequest.head || !pullRequest.head.sha) { core.warning("Pull request head SHA not available - cannot submit review"); return { success: false, error: "Pull request head SHA not available" }; } - // Use the pinned commit ID when configured, falling back to the live PR head SHA. - // A pinned commitId ensures the review is attributed to the commit the agent actually - // reviewed, preventing attribution drift when new commits are pushed during the run. - const resolvedCommitId = commitId || pullRequest.head.sha; - if (commitId) { - core.info(`Using pinned commit ID for review: ${commitId} (PR head is ${pullRequest.head.sha})`); + // Use the head SHA captured at trigger time (GH_AW_HEAD_SHA, injected by the compiler) + // when available, falling back to the live PR head SHA. This pins the review to the + // commit the agent actually reviewed, preventing attribution drift when new commits are + // pushed during the run (most common under workflow_run triggers where the safe_outputs + // job runs after the agent job and pulls.get() may return a newer HEAD sha). + const awHeadSHA = process.env.GH_AW_HEAD_SHA || ""; + const resolvedCommitId = awHeadSHA || pullRequest.head.sha; + if (awHeadSHA && awHeadSHA !== pullRequest.head.sha) { + core.info(`Using trigger-time head SHA: ${awHeadSHA} (PR head is now ${pullRequest.head.sha})`); } // Determine review event and body diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs index 8c62dff7fda..14099689aae 100644 --- a/actions/setup/js/pr_review_buffer.test.cjs +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -556,53 +556,69 @@ describe("pr_review_buffer (factory pattern)", () => { expect(callArgs.comments).toBeUndefined(); }); - it("should use pinned commitId from review context instead of live PR head SHA", async () => { - buffer.setReviewMetadata("Review with pinned commit", "COMMENT"); - buffer.setReviewContext({ - repo: "owner/repo", - repoParts: { owner: "owner", repo: "repo" }, - pullRequestNumber: 42, - pullRequest: { head: { sha: "live-head-sha-pushed-later" } }, - commitId: "original-reviewed-sha", - }); + it("should use GH_AW_HEAD_SHA from env instead of live PR head SHA", async () => { + const previousHeadSHA = process.env.GH_AW_HEAD_SHA; + process.env.GH_AW_HEAD_SHA = "original-reviewed-sha"; + try { + buffer.setReviewMetadata("Review with pinned commit", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "live-head-sha-pushed-later" } }, + }); - mockGithub.rest.pulls.createReview.mockResolvedValue({ - data: { - id: 700, - html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-700", - }, - }); + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 700, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-700", + }, + }); - const result = await buffer.submitReview(); + const result = await buffer.submitReview(); - expect(result.success).toBe(true); - // The pinned sha must be passed as commit_id, not the live head sha - const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; - expect(callArgs.commit_id).toBe("original-reviewed-sha"); + expect(result.success).toBe(true); + // The env var sha must be passed as commit_id, not the live head sha + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.commit_id).toBe("original-reviewed-sha"); + } finally { + if (previousHeadSHA !== undefined) { + process.env.GH_AW_HEAD_SHA = previousHeadSHA; + } else { + delete process.env.GH_AW_HEAD_SHA; + } + } }); - it("should fall back to pullRequest.head.sha when commitId is not set in review context", async () => { - buffer.setReviewMetadata("Regular review", "COMMENT"); - buffer.setReviewContext({ - repo: "owner/repo", - repoParts: { owner: "owner", repo: "repo" }, - pullRequestNumber: 42, - pullRequest: { head: { sha: "current-head-sha" } }, - // No commitId field - }); + it("should fall back to pullRequest.head.sha when GH_AW_HEAD_SHA is not set", async () => { + const previousHeadSHA = process.env.GH_AW_HEAD_SHA; + delete process.env.GH_AW_HEAD_SHA; + try { + buffer.setReviewMetadata("Regular review", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "current-head-sha" } }, + }); - mockGithub.rest.pulls.createReview.mockResolvedValue({ - data: { - id: 800, - html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-800", - }, - }); + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 800, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-800", + }, + }); - const result = await buffer.submitReview(); + const result = await buffer.submitReview(); - expect(result.success).toBe(true); - const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; - expect(callArgs.commit_id).toBe("current-head-sha"); + expect(result.success).toBe(true); + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.commit_id).toBe("current-head-sha"); + } finally { + if (previousHeadSHA !== undefined) { + process.env.GH_AW_HEAD_SHA = previousHeadSHA; + } + } }); it("should include multi-line comment fields with side fallback for start_side", async () => { diff --git a/actions/setup/js/submit_pr_review.cjs b/actions/setup/js/submit_pr_review.cjs index bb7c3031b11..d9d8e8a01d8 100644 --- a/actions/setup/js/submit_pr_review.cjs +++ b/actions/setup/js/submit_pr_review.cjs @@ -44,14 +44,6 @@ async function main(config = {}) { const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); - // Optional pinned commit SHA. When set, overrides the live PR head SHA so that the - // review is attributed to the commit the agent actually reviewed, not whatever head - // the PR has at post time (which may differ under workflow_run triggers). - const pinnedCommitId = typeof config.commit_id === "string" && config.commit_id.trim() ? config.commit_id.trim() : null; - if (pinnedCommitId) { - core.info(`submit_pull_request_review: using pinned commit-id: ${pinnedCommitId}`); - } - const requiredLabels = Array.isArray(config.required_labels) ? config.required_labels : []; const requiredTitlePrefix = config.required_title_prefix || ""; if (requiredLabels.length > 0) core.info(`Required labels (all): ${requiredLabels.join(", ")}`); @@ -284,7 +276,6 @@ async function main(config = {}) { repoParts, pullRequestNumber: payloadPR.number, pullRequest: payloadPR, - ...(pinnedCommitId ? { commitId: pinnedCommitId } : {}), }); core.info(`Set review context from triggering PR: ${repo}#${payloadPR.number}`); } else { @@ -300,7 +291,6 @@ async function main(config = {}) { repoParts, pullRequestNumber: fetchedPR.number, pullRequest: fetchedPR, - ...(pinnedCommitId ? { commitId: pinnedCommitId } : {}), }); core.info(`Set review context from target: ${repo}#${fetchedPR.number}`); } else { diff --git a/actions/setup/js/submit_pr_review.test.cjs b/actions/setup/js/submit_pr_review.test.cjs index 78bfa3b1274..82128e35268 100644 --- a/actions/setup/js/submit_pr_review.test.cjs +++ b/actions/setup/js/submit_pr_review.test.cjs @@ -876,28 +876,9 @@ describe("submit_pr_review multi-buffer (registry mode)", () => { await main({ max: 1, target: "*", supersede_older_reviews: true, _prReviewBufferRegistry: registry }); expect(setSuperSpy).toHaveBeenCalledWith(true); }); - - it("pinned commit_id is stored in review context for each buffer (registry mode)", async () => { - const registry = createPrReviewBufferRegistry(); - const { main } = require("./submit_pr_review.cjs"); - const handler = await main({ - max: 5, - target: "*", - commit_id: "pinned-sha-abc123", - _prReviewBufferRegistry: registry, - }); - - await handler({ body: "Review for PR 10", event: "COMMENT", pull_request_number: 10 }, {}); - - const entries = registry.getAllEntries(); - expect(entries).toHaveLength(1); - const ctx = entries[0].buffer.getReviewContext(); - expect(ctx).not.toBeNull(); - expect(ctx.commitId).toBe("pinned-sha-abc123"); - }); }); -describe("submit_pr_review: commit-id pinning", () => { +describe("submit_pr_review: GH_AW_HEAD_SHA automatic commit pinning", () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -907,7 +888,7 @@ describe("submit_pr_review: commit-id pinning", () => { delete global.github; }); - it("should store pinned commitId in review context from triggering PR payload", async () => { + it("review context has no commitId field (GH_AW_HEAD_SHA is used at submit time instead)", async () => { global.context = { eventName: "pull_request", repo: { owner: "test-owner", repo: "test-repo" }, @@ -925,7 +906,6 @@ describe("submit_pr_review: commit-id pinning", () => { const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer, - commit_id: "pinned-sha-from-eligibility", }); const result = await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); @@ -933,15 +913,13 @@ describe("submit_pr_review: commit-id pinning", () => { expect(result.success).toBe(true); const ctx = localBuffer.getReviewContext(); expect(ctx).not.toBeNull(); - // commitId should be the pinned value, not the live head SHA - expect(ctx.commitId).toBe("pinned-sha-from-eligibility"); - // The payload SHA should still be present in pullRequest.head.sha + // No commitId on the context; GH_AW_HEAD_SHA is read at submitReview() time + expect(ctx.commitId).toBeUndefined(); expect(ctx.pullRequest.head.sha).toBe("live-head-sha"); - // github.rest.pulls.get should NOT have been called (payload PR was used) expect(global.github.rest.pulls.get).not.toHaveBeenCalled(); }); - it("should store pinned commitId in review context fetched from API (workflow_run scenario)", async () => { + it("review context has no commitId in workflow_run scenario (GH_AW_HEAD_SHA used automatically)", async () => { const fetchedPR = { number: 55, head: { sha: "live-head-after-push" } }; global.context = { eventName: "workflow_run", @@ -963,7 +941,6 @@ describe("submit_pr_review: commit-id pinning", () => { max: 1, target: "55", _prReviewBuffer: localBuffer, - commit_id: "original-reviewed-sha", }); const result = await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); @@ -971,57 +948,8 @@ describe("submit_pr_review: commit-id pinning", () => { expect(result.success).toBe(true); const ctx = localBuffer.getReviewContext(); expect(ctx).not.toBeNull(); - // commitId should be the pinned value, NOT the live head SHA returned by the API - expect(ctx.commitId).toBe("original-reviewed-sha"); - // The live head SHA is still stored in pullRequest (used for other purposes) - expect(ctx.pullRequest.head.sha).toBe("live-head-after-push"); - }); - - it("should not set commitId when commit_id config is not provided", async () => { - global.context = { - eventName: "pull_request", - repo: { owner: "test-owner", repo: "test-repo" }, - payload: { - pull_request: { number: 7, head: { sha: "head-sha" } }, - }, - }; - global.github = { - rest: { pulls: { get: vi.fn() } }, - graphql: vi.fn(), - }; - - const localBuffer = createReviewBuffer(); - const { main } = require("./submit_pr_review.cjs"); - const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer }); - - await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); - - const ctx = localBuffer.getReviewContext(); - expect(ctx).not.toBeNull(); - expect(ctx.commitId).toBeUndefined(); - }); - - it("should not set commitId when commit_id is empty string", async () => { - global.context = { - eventName: "pull_request", - repo: { owner: "test-owner", repo: "test-repo" }, - payload: { - pull_request: { number: 7, head: { sha: "head-sha" } }, - }, - }; - global.github = { - rest: { pulls: { get: vi.fn() } }, - graphql: vi.fn(), - }; - - const localBuffer = createReviewBuffer(); - const { main } = require("./submit_pr_review.cjs"); - const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer, commit_id: "" }); - - await localHandler({ type: "submit_pull_request_review", body: "Review", event: "COMMENT" }, {}); - - const ctx = localBuffer.getReviewContext(); - expect(ctx).not.toBeNull(); + // No commitId on context; GH_AW_HEAD_SHA env var is used at submit time expect(ctx.commitId).toBeUndefined(); + expect(ctx.pullRequest.head.sha).toBe("live-head-after-push"); }); }); diff --git a/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md b/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md index 56d457c0fcb..e0348e2cdcd 100644 --- a/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md +++ b/docs/adr/48738-pin-review-attribution-to-reviewed-commit.md @@ -1,48 +1,55 @@ -# ADR-48738: Pin PR Review Attribution to the Reviewed Commit via Optional `commit-id` Config +# ADR-48738: Automatic PR Review Attribution via Compiler-Injected Head SHA -**Date**: 2026-07-28 -**Status**: Draft +**Date**: 2026-07-29 +**Status**: Accepted **Deciders**: pelikhan, copilot-swe-agent --- ### Context -Under `workflow_run` triggers, the safe-outputs runtime submits a PR review after a parent workflow completes. A new commit can land on the PR branch while the agent is running. When that happens, `pulls.get()` returns the new HEAD SHA, and the review is attributed to a commit the agent never actually reviewed — making inline comments appear fabricated, outdated, or misaligned. There was no mechanism for callers to pass the specific SHA the agent reviewed to the review submission step. This race condition is most acute in `workflow_run` contexts where an eligibility job captures the PR HEAD at trigger time and passes it to downstream jobs. +Under `workflow_run` triggers, the safe-outputs runtime submits a PR review after a parent workflow completes. A new commit can land on the PR branch while the agent is running. When that happens, `pulls.get()` returns the new HEAD SHA, and the review is attributed to a commit the agent never actually reviewed — making inline comments appear fabricated, outdated, or misaligned. ### Decision -We will add an optional `commit-id` configuration field to both `submit-pull-request-review` and `create-pull-request-review-comment` safe-output handlers. When set, this SHA is used as the `commit_id` argument to `pulls.createReview()` instead of the live PR head SHA. The field is optional and backward-compatible: when omitted, behavior is unchanged (the live PR head SHA is used). Callers explicitly pass the reviewed SHA — typically captured by an upstream eligibility job — via `commit-id: ${{ needs.eligibility.outputs.head_sha }}` in their workflow YAML. +The compiler automatically captures the PR head SHA at trigger time and injects it into the compiled workflow as the `GH_AW_HEAD_SHA` environment variable, without requiring any user YAML configuration. The JS safe-outputs runtime reads `process.env.GH_AW_HEAD_SHA` in `submitReview()` and uses it as the `commit_id` argument to `pulls.createReview()` when present, falling back to the live PR head SHA otherwise. + +The compiler emits the correct expression based on the workflow trigger: + +| Trigger | `GH_AW_HEAD_SHA` value | +|---|---| +| `workflow_run` | `${{ github.event.workflow_run.head_sha }}` | +| `pull_request` | `${{ github.event.pull_request.head.sha }}` | +| `pull_request_target` | `${{ github.event.pull_request.head.sha }}` | +| Other triggers (push, schedule, issues, …) | Not injected; live PR head SHA is used | + +No user YAML configuration is needed. The feature is transparent and automatic for all affected trigger types. ### Alternatives Considered -#### Alternative 1: Automatically capture and freeze the PR HEAD SHA at agent startup +#### Alternative 1: Optional user-configurable `commit-id` YAML field -Freeze the SHA once when the safe-outputs handler initialises, before the review accumulation phase begins, and always use that frozen value. This would require no caller-side configuration. +Add a `commit-id` field to `submit-pull-request-review` and `create-pull-request-review-comment` so callers can explicitly pass the reviewed SHA. -Why not chosen: The agent may run for an extended period before submitting the review. A commit pushed after handler initialisation but during the agent run would still cause attribution drift. More importantly, in `workflow_run` scenarios the correct SHA to attribute is the one the *triggering* workflow saw — which may already be older than what the safe-outputs job sees at startup. That SHA is only knowable by the caller via an upstream job output, not by the handler itself. +Why not chosen: Requires users to explicitly wire the SHA through their workflow YAML (e.g., from an eligibility job output). Forgetting to set `commit-id` leaves the race condition unaddressed. Also introduces multi-target ambiguity when `target: "*"` is configured, since a single handler-level SHA cannot be correct for multiple distinct PRs. The compiler knows the trigger type at compile time and can inject the right expression automatically. -#### Alternative 2: Always use the triggering `workflow_run` payload SHA +#### Alternative 2: Freeze the PR HEAD SHA at handler initialization -For `workflow_run` events, automatically extract the SHA from the triggering workflow's context payload instead of calling `pulls.get()`. +Capture and freeze the SHA once when the handler initializes, before the review accumulation phase begins. -Why not chosen: The triggering payload does not always carry the SHA of the PR commit the agent reviewed (it may contain the SHA of the triggering workflow's default branch commit). Callers need explicit control over which commit to attribute, particularly when the reviewed SHA is computed by a separate eligibility job and stored as a job output. This approach also does not address non-`workflow_run` triggers that may have similar drift in edge cases. +Why not chosen: A commit pushed after handler initialization but during the agent run would still cause attribution drift. The correct SHA is the one that was in place when the triggering workflow ran, not when the safe-outputs job starts. ### Consequences #### Positive - Reviews are attributed to the commit the agent actually reviewed, eliminating false "outdated" or misaligned inline comments. - GitHub correctly marks the review as outdated when HEAD moves, rather than incorrectly attaching stale comments to the new commit. -- The feature is fully opt-in and backward-compatible — existing callers are unaffected. +- Zero user configuration required — all existing workflows benefit automatically after recompile. +- Multi-target (`target: "*"`) scenarios are unaffected because the SHA is read from an environment variable at runtime, not passed as a single handler-level config value. #### Negative -- Callers must explicitly wire the reviewed SHA through their workflow YAML (e.g., from an eligibility job output); forgetting to set `commit-id` leaves the race condition unaddressed in `workflow_run` contexts. -- The `commit-id` field is threaded through three layers (Go config struct → handler registry JSON → JS runtime), increasing the surface area of the config pipeline. +- For workflows with triggers other than `workflow_run`, `pull_request`, and `pull_request_target`, the env var is not injected and the live PR head SHA is used (existing behavior). If drift is a concern for those triggers, a separate mechanism would be needed. #### Neutral -- The Go config structs (`SubmitPullRequestReviewConfig`, `CreatePullRequestReviewCommentsConfig`) gain a new `CommitId string` field, which is zero-valued (empty) when not configured — no breaking change to existing parsed configs. -- The JS runtime falls back to `pullRequest.head.sha` when `commitId` is absent, preserving existing runtime behaviour. - ---- - -*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* +- The `GH_AW_HEAD_SHA` env var follows the existing pattern of compiler-injected context variables (e.g., `GH_AW_WORKFLOW_ID`, `GH_AW_CALLER_WORKFLOW_ID`, `GH_AW_AMBIENT_CONTEXT`). +- When `GH_AW_HEAD_SHA` is absent or empty, `submitReview()` falls back to `pullRequest.head.sha`, preserving existing runtime behavior for unsupported trigger types. diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 8f27c1646a2..ae8d7cd2e30 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -743,6 +743,36 @@ func (c *Compiler) buildSafeOutputsJobNeeds(data *WorkflowData, mainJobName stri return needs } +// headSHAExpressionForTrigger returns the GitHub Actions expression for the PR head SHA +// based on the workflow's `on:` field. Returns an empty string if the trigger type does +// not carry a directly accessible PR head SHA (e.g. push, schedule, issues). +// +// The returned expression is injected as GH_AW_HEAD_SHA in the safe_outputs job so that +// PR-review handlers automatically pin their reviews to the commit that was in place when +// the workflow triggered, preventing attribution drift if new commits land during the run. +func headSHAExpressionForTrigger(onField any) string { + switch v := onField.(type) { + case map[string]any: + if _, ok := v["workflow_run"]; ok { + return "${{ github.event.workflow_run.head_sha }}" + } + if _, ok := v["pull_request"]; ok { + return "${{ github.event.pull_request.head.sha }}" + } + if _, ok := v["pull_request_target"]; ok { + return "${{ github.event.pull_request.head.sha }}" + } + case string: + switch v { + case "workflow_run": + return "${{ github.event.workflow_run.head_sha }}" + case "pull_request", "pull_request_target": + return "${{ github.event.pull_request.head.sha }}" + } + } + return "" +} + // buildJobLevelSafeOutputEnvVars builds environment variables that should be set at the job level // for the consolidated safe_outputs job. These are variables that are common to all safe output steps. func (c *Compiler) buildJobLevelSafeOutputEnvVars(data *WorkflowData, workflowID string) map[string]string { @@ -872,6 +902,16 @@ func (c *Compiler) buildJobLevelSafeOutputEnvVars(data *WorkflowData, workflowID envVars["GH_AW_THREAT_DETECTION_AIC"] = fmt.Sprintf("${{ needs.%s.outputs.aic }}", constants.DetectionJobName) } + // Automatically inject the PR head SHA captured at trigger time so that PR-review + // handlers (submit_pull_request_review, create_pull_request_review_comment) pin their + // reviews to the reviewed commit without requiring any user YAML configuration. + // For workflow_run triggers this prevents attribution drift when a new commit lands + // on the PR while the agent is running (the safe_outputs job runs after the agent job + // and pulls.get() would otherwise return the new HEAD sha). + if headSHAExpr := headSHAExpressionForTrigger(data.RawFrontmatter["on"]); headSHAExpr != "" { + envVars["GH_AW_HEAD_SHA"] = headSHAExpr + } + return envVars } diff --git a/pkg/workflow/create_pr_review_comment.go b/pkg/workflow/create_pr_review_comment.go index cd34efc79c8..4c281831f5c 100644 --- a/pkg/workflow/create_pr_review_comment.go +++ b/pkg/workflow/create_pr_review_comment.go @@ -14,7 +14,6 @@ type CreatePullRequestReviewCommentsConfig struct { Target string `yaml:"target,omitempty"` // Target for comments: "triggering" (default), "*" (any PR), or explicit PR number TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository PR review comments AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that PR review comments can be added to (additionally to the target-repo) - CommitId string `yaml:"commit-id,omitempty"` // Optional commit SHA to use as the reviewed commit. Pins the review to the commit the agent actually reviewed, preventing attribution drift when new commits are pushed during the run. } func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any) *CreatePullRequestReviewCommentsConfig { @@ -53,14 +52,6 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any } prReviewCommentsConfig.TargetRepoSlug = targetRepoSlug - // Parse optional commit-id override - if commitId, exists := configMap["commit-id"]; exists { - if commitIdStr, ok := commitId.(string); ok && commitIdStr != "" { - prReviewCommentsConfig.CommitId = commitIdStr - createPRReviewCommentLog.Printf("Commit ID override: %s", commitIdStr) - } - } - // Parse common base fields with default max of 10 c.parseBaseSafeOutputConfig(configMap, &prReviewCommentsConfig.BaseSafeOutputConfig, 10) } else { diff --git a/pkg/workflow/safe_outputs_handler_registry.go b/pkg/workflow/safe_outputs_handler_registry.go index 64ae8a9f83d..cba92bfb055 100644 --- a/pkg/workflow/safe_outputs_handler_registry.go +++ b/pkg/workflow/safe_outputs_handler_registry.go @@ -425,7 +425,6 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_repos", c.AllowedRepos). AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). - AddIfNotEmpty("commit_id", c.CommitId). Build() }, "submit_pull_request_review": func(cfg *SafeOutputsConfig) map[string]any { @@ -443,7 +442,6 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).AddIfNotEmpty("github-token", c.GitHubToken). AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). - AddIfNotEmpty("commit_id", c.CommitId). Build() }, "reply_to_pull_request_review_comment": func(cfg *SafeOutputsConfig) map[string]any { diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go index 54714ab9f1f..18ecdf44997 100644 --- a/pkg/workflow/submit_pr_review.go +++ b/pkg/workflow/submit_pr_review.go @@ -20,7 +20,6 @@ type SubmitPullRequestReviewConfig struct { Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text) AllowedEvents []string `yaml:"allowed-events,omitempty"` // Optional list of allowed review event types: APPROVE, COMMENT, REQUEST_CHANGES. If omitted, all event types are allowed. SupersedeOlderReviews bool `yaml:"supersede-older-reviews,omitempty"` // When true, dismisses older same-workflow REQUEST_CHANGES reviews after a replacement review is posted. - CommitId string `yaml:"commit-id,omitempty"` // Optional commit SHA to use as the reviewed commit. Pins the review to the commit the agent actually reviewed, preventing attribution drift when new commits are pushed during the run. } // parseSubmitPullRequestReviewConfig handles submit-pull-request-review configuration @@ -113,14 +112,6 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) } } - // Parse optional commit-id override - if commitId, exists := configMap["commit-id"]; exists { - if commitIdStr, ok := commitId.(string); ok && commitIdStr != "" { - config.CommitId = commitIdStr - submitPRReviewLog.Printf("Commit ID override: %s", commitIdStr) - } - } - submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s, allowed_events=%v, supersede_older_reviews=%t", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug, config.AllowedEvents, config.SupersedeOlderReviews) } else { // If configData is nil or not a map, set the default max diff --git a/pkg/workflow/submit_pr_review_footer_test.go b/pkg/workflow/submit_pr_review_footer_test.go index c49ed69acab..09eaf6dfe83 100644 --- a/pkg/workflow/submit_pr_review_footer_test.go +++ b/pkg/workflow/submit_pr_review_footer_test.go @@ -623,176 +623,113 @@ func TestSubmitPRReviewFooterInHandlerConfig(t *testing.T) { }) } -func TestParseCommitIdConfig(t *testing.T) { - t.Run("parses commit-id for submit-pull-request-review", func(t *testing.T) { - compiler := NewCompiler() - outputMap := map[string]any{ - "submit-pull-request-review": map[string]any{ - "max": 1, - "commit-id": "abc123def456", - }, +func TestHeadSHAExpressionForTrigger(t *testing.T) { + t.Run("workflow_run map trigger returns workflow_run head_sha expression", func(t *testing.T) { + onField := map[string]any{ + "workflow_run": map[string]any{"workflows": []any{"CI"}}, } - - config := compiler.parseSubmitPullRequestReviewConfig(outputMap) - require.NotNil(t, config, "Config should be parsed") - assert.Equal(t, "abc123def456", config.CommitId, "CommitId should be parsed") + result := headSHAExpressionForTrigger(onField) + assert.Equal(t, "${{ github.event.workflow_run.head_sha }}", result) }) - t.Run("commit-id empty when not provided for submit-pull-request-review", func(t *testing.T) { - compiler := NewCompiler() - outputMap := map[string]any{ - "submit-pull-request-review": map[string]any{ - "max": 1, - }, + t.Run("pull_request map trigger returns pull_request head sha expression", func(t *testing.T) { + onField := map[string]any{ + "pull_request": map[string]any{"types": []any{"opened"}}, } - - config := compiler.parseSubmitPullRequestReviewConfig(outputMap) - require.NotNil(t, config, "Config should be parsed") - assert.Empty(t, config.CommitId, "CommitId should be empty when not configured") + result := headSHAExpressionForTrigger(onField) + assert.Equal(t, "${{ github.event.pull_request.head.sha }}", result) }) - t.Run("parses commit-id for create-pull-request-review-comment", func(t *testing.T) { - compiler := NewCompiler() - outputMap := map[string]any{ - "create-pull-request-review-comment": map[string]any{ - "commit-id": "deadbeef1234", - }, + t.Run("pull_request_target map trigger returns pull_request head sha expression", func(t *testing.T) { + onField := map[string]any{ + "pull_request_target": nil, } + result := headSHAExpressionForTrigger(onField) + assert.Equal(t, "${{ github.event.pull_request.head.sha }}", result) + }) - config := compiler.parsePullRequestReviewCommentsConfig(outputMap) - require.NotNil(t, config, "Config should be parsed") - assert.Equal(t, "deadbeef1234", config.CommitId, "CommitId should be parsed") + t.Run("workflow_run string trigger returns workflow_run head_sha expression", func(t *testing.T) { + result := headSHAExpressionForTrigger("workflow_run") + assert.Equal(t, "${{ github.event.workflow_run.head_sha }}", result) }) - t.Run("commit-id empty when not provided for create-pull-request-review-comment", func(t *testing.T) { - compiler := NewCompiler() - outputMap := map[string]any{ - "create-pull-request-review-comment": map[string]any{ - "side": "RIGHT", - }, + t.Run("pull_request string trigger returns pull_request head sha expression", func(t *testing.T) { + result := headSHAExpressionForTrigger("pull_request") + assert.Equal(t, "${{ github.event.pull_request.head.sha }}", result) + }) + + t.Run("issues trigger returns empty string", func(t *testing.T) { + onField := map[string]any{ + "issues": map[string]any{"types": []any{"opened"}}, } + result := headSHAExpressionForTrigger(onField) + assert.Empty(t, result) + }) - config := compiler.parsePullRequestReviewCommentsConfig(outputMap) - require.NotNil(t, config, "Config should be parsed") - assert.Empty(t, config.CommitId, "CommitId should be empty when not configured") + t.Run("push trigger returns empty string", func(t *testing.T) { + onField := map[string]any{"push": nil} + result := headSHAExpressionForTrigger(onField) + assert.Empty(t, result) }) - t.Run("commit-id emitted in submit_pull_request_review handler config", func(t *testing.T) { + t.Run("nil trigger returns empty string", func(t *testing.T) { + result := headSHAExpressionForTrigger(nil) + assert.Empty(t, result) + }) +} + +func TestBuildJobLevelSafeOutputEnvVarsHeadSHA(t *testing.T) { + t.Run("GH_AW_HEAD_SHA set for workflow_run trigger", func(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{ Name: "Test", - SafeOutputs: &SafeOutputsConfig{ - SubmitPullRequestReview: &SubmitPullRequestReviewConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, - CommitId: "pinned-sha-abc123", + RawFrontmatter: map[string]any{ + "on": map[string]any{ + "workflow_run": map[string]any{"workflows": []any{"CI"}}, }, }, } - - var steps []string - compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) - require.NotEmpty(t, steps, "Steps should not be empty") - - for _, step := range steps { - if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { - parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") - if len(parts) == 2 { - jsonStr := strings.TrimSpace(parts[1]) - jsonStr = strings.Trim(jsonStr, "\"") - jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") - var handlerConfig map[string]any - err := json.Unmarshal([]byte(jsonStr), &handlerConfig) - require.NoError(t, err, "Should unmarshal handler config") - - submitConfig, ok := handlerConfig["submit_pull_request_review"].(map[string]any) - require.True(t, ok, "submit_pull_request_review config should exist") - assert.Equal(t, "pinned-sha-abc123", submitConfig["commit_id"], "commit_id should be emitted in handler config") - } - } - } + envVars := compiler.buildJobLevelSafeOutputEnvVars(workflowData, "test-workflow") + assert.Equal(t, "${{ github.event.workflow_run.head_sha }}", envVars["GH_AW_HEAD_SHA"]) }) - t.Run("commit-id not emitted in submit_pull_request_review handler config when empty", func(t *testing.T) { + t.Run("GH_AW_HEAD_SHA set for pull_request trigger", func(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{ Name: "Test", - SafeOutputs: &SafeOutputsConfig{ - SubmitPullRequestReview: &SubmitPullRequestReviewConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("1")}, + RawFrontmatter: map[string]any{ + "on": map[string]any{ + "pull_request": map[string]any{"types": []any{"opened"}}, }, }, } - - var steps []string - compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) - require.NotEmpty(t, steps, "Steps should not be empty") - - for _, step := range steps { - if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { - parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") - if len(parts) == 2 { - jsonStr := strings.TrimSpace(parts[1]) - jsonStr = strings.Trim(jsonStr, "\"") - jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") - var handlerConfig map[string]any - err := json.Unmarshal([]byte(jsonStr), &handlerConfig) - require.NoError(t, err, "Should unmarshal handler config") - - submitConfig, ok := handlerConfig["submit_pull_request_review"].(map[string]any) - require.True(t, ok, "submit_pull_request_review config should exist") - _, hasCommitId := submitConfig["commit_id"] - assert.False(t, hasCommitId, "commit_id should not be in handler config when not set") - } - } - } + envVars := compiler.buildJobLevelSafeOutputEnvVars(workflowData, "test-workflow") + assert.Equal(t, "${{ github.event.pull_request.head.sha }}", envVars["GH_AW_HEAD_SHA"]) }) - t.Run("commit-id emitted in create_pull_request_review_comment handler config", func(t *testing.T) { + t.Run("GH_AW_HEAD_SHA not set for issues trigger", func(t *testing.T) { compiler := NewCompiler() workflowData := &WorkflowData{ Name: "Test", - SafeOutputs: &SafeOutputsConfig{ - CreatePullRequestReviewComments: &CreatePullRequestReviewCommentsConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{Max: strPtr("10")}, - CommitId: "pinned-sha-for-comments", + RawFrontmatter: map[string]any{ + "on": map[string]any{ + "issues": map[string]any{"types": []any{"opened"}}, }, }, } - - var steps []string - compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) - require.NotEmpty(t, steps, "Steps should not be empty") - - for _, step := range steps { - if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { - parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") - if len(parts) == 2 { - jsonStr := strings.TrimSpace(parts[1]) - jsonStr = strings.Trim(jsonStr, "\"") - jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") - var handlerConfig map[string]any - err := json.Unmarshal([]byte(jsonStr), &handlerConfig) - require.NoError(t, err, "Should unmarshal handler config") - - reviewCommentConfig, ok := handlerConfig["create_pull_request_review_comment"].(map[string]any) - require.True(t, ok, "create_pull_request_review_comment config should exist") - assert.Equal(t, "pinned-sha-for-comments", reviewCommentConfig["commit_id"], "commit_id should be emitted in review comment handler config") - } - } - } + envVars := compiler.buildJobLevelSafeOutputEnvVars(workflowData, "test-workflow") + _, hasHeadSHA := envVars["GH_AW_HEAD_SHA"] + assert.False(t, hasHeadSHA, "GH_AW_HEAD_SHA should not be set for issues trigger") }) - t.Run("accepts GitHub expression as commit-id value for submit-pull-request-review", func(t *testing.T) { + t.Run("GH_AW_HEAD_SHA not set when no frontmatter on field", func(t *testing.T) { compiler := NewCompiler() - outputMap := map[string]any{ - "submit-pull-request-review": map[string]any{ - "max": 1, - "commit-id": "${{ needs.eligibility.outputs.head_sha }}", - }, + workflowData := &WorkflowData{ + Name: "Test", + RawFrontmatter: map[string]any{}, } - - config := compiler.parseSubmitPullRequestReviewConfig(outputMap) - require.NotNil(t, config, "Config should be parsed") - assert.Equal(t, "${{ needs.eligibility.outputs.head_sha }}", config.CommitId, "CommitId should accept GitHub expression") + envVars := compiler.buildJobLevelSafeOutputEnvVars(workflowData, "test-workflow") + _, hasHeadSHA := envVars["GH_AW_HEAD_SHA"] + assert.False(t, hasHeadSHA, "GH_AW_HEAD_SHA should not be set when no on: field") }) } From 7ff4a0c7877ac1add23bf56adf2c8afda25ef4c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:33:57 +0000 Subject: [PATCH 5/7] docs: document automatic GH_AW_HEAD_SHA review attribution pinning in safe-outputs spec Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- docs/src/content/docs/specs/safe-outputs-specification.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/src/content/docs/specs/safe-outputs-specification.md b/docs/src/content/docs/specs/safe-outputs-specification.md index a5af862cdb9..f7f92bf249b 100644 --- a/docs/src/content/docs/specs/safe-outputs-specification.md +++ b/docs/src/content/docs/specs/safe-outputs-specification.md @@ -1650,6 +1650,8 @@ submit-pull-request-review: footer: "always" | "none" | "if-body" # Footer on review body ``` +> **Review attribution pinning**: Under `workflow_run` triggers, the compiler automatically injects `GH_AW_HEAD_SHA` (set to `${{ github.event.workflow_run.head_sha }}`) into the safe-outputs job environment. For `pull_request` and `pull_request_target` triggers, it is set to `${{ github.event.pull_request.head.sha }}`. The runtime uses this value as `commit_id` when posting the review, so the review is always attributed to the commit the agent actually saw — not a newer HEAD that may have landed while the agent was running. No user configuration is needed. + **Pull Request Extensions**: ```yaml From 461e208e5944d8d7743c5879d21607156fb846de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:46:12 +0000 Subject: [PATCH 6/7] chore: initial plan for commit-id config option support Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/ai-moderator.lock.yml | 1 + .github/workflows/changeset.lock.yml | 1 + .github/workflows/design-decision-gate.lock.yml | 1 + .github/workflows/dev-hawk.lock.yml | 1 + .github/workflows/firewall-escape.lock.yml | 1 + .github/workflows/impeccable-skills-reviewer.lock.yml | 1 + .github/workflows/mattpocock-skills-reviewer.lock.yml | 1 + .github/workflows/pr-code-quality-reviewer.lock.yml | 1 + .github/workflows/pr-description-caveman.lock.yml | 1 + .github/workflows/refiner.lock.yml | 1 + .github/workflows/smoke-agent-all-merged.lock.yml | 1 + .github/workflows/smoke-agent-all-none.lock.yml | 1 + .github/workflows/smoke-agent-public-approved.lock.yml | 1 + .github/workflows/smoke-agent-public-none.lock.yml | 1 + .github/workflows/smoke-agent-scoped-approved.lock.yml | 1 + .github/workflows/smoke-antigravity.lock.yml | 1 + .github/workflows/smoke-call-workflow.lock.yml | 1 + .github/workflows/smoke-ci.lock.yml | 1 + .github/workflows/smoke-claude.lock.yml | 1 + .github/workflows/smoke-codex.lock.yml | 1 + .github/workflows/smoke-copilot-arm.lock.yml | 1 + .github/workflows/smoke-create-cross-repo-pr.lock.yml | 1 + .github/workflows/smoke-gemini.lock.yml | 1 + .github/workflows/smoke-multi-pr.lock.yml | 1 + .github/workflows/smoke-opencode.lock.yml | 1 + .github/workflows/smoke-pi.lock.yml | 1 + .github/workflows/smoke-project.lock.yml | 1 + .github/workflows/smoke-temporary-id.lock.yml | 1 + .github/workflows/smoke-test-tools.lock.yml | 1 + .github/workflows/smoke-update-cross-repo-pr.lock.yml | 1 + .github/workflows/test-quality-sentinel.lock.yml | 1 + .github/workflows/visual-regression-checker.lock.yml | 1 + 32 files changed, 32 insertions(+) diff --git a/.github/workflows/ai-moderator.lock.yml b/.github/workflows/ai-moderator.lock.yml index 985b372a8f7..8fc5a91d703 100644 --- a/.github/workflows/ai-moderator.lock.yml +++ b/.github/workflows/ai-moderator.lock.yml @@ -1860,6 +1860,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "codex" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_WORKFLOW_EMOJI: "🤖" diff --git a/.github/workflows/changeset.lock.yml b/.github/workflows/changeset.lock.yml index f3e6c70ec07..34d838e200a 100644 --- a/.github/workflows/changeset.lock.yml +++ b/.github/workflows/changeset.lock.yml @@ -2077,6 +2077,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "codex" GH_AW_ENGINE_MODEL: "gpt-5.4" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_THREAT_DETECTION_AIC: ${{ needs.detection.outputs.aic }} diff --git a/.github/workflows/design-decision-gate.lock.yml b/.github/workflows/design-decision-gate.lock.yml index 21e5e28c548..db7c33dafa5 100644 --- a/.github/workflows/design-decision-gate.lock.yml +++ b/.github/workflows/design-decision-gate.lock.yml @@ -2175,6 +2175,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: "claude-sonnet-4-6" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🏗️ *ADR gate enforced by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔍 [{workflow_name}]({run_url}) is checking for design decision records on this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed the design decision gate check.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during design decision gate check.\"}" diff --git a/.github/workflows/dev-hawk.lock.yml b/.github/workflows/dev-hawk.lock.yml index 0485d9f9a95..db95f658939 100644 --- a/.github/workflows/dev-hawk.lock.yml +++ b/.github/workflows/dev-hawk.lock.yml @@ -2056,6 +2056,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.workflow_run.head_sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🦅 *Observed from above by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🦅 Dev Hawk circles the sky! [{workflow_name}]({run_url}) is monitoring this {event_type} from above...\",\"runSuccess\":\"🦅 Hawk eyes report! [{workflow_name}]({run_url}) has completed reconnaissance. Intel delivered! 🎯\",\"runFailure\":\"🦅 Hawk down! [{workflow_name}]({run_url}) {status}. The skies grow quiet...\"}" diff --git a/.github/workflows/firewall-escape.lock.yml b/.github/workflows/firewall-escape.lock.yml index 9487be95d3c..0103860af2b 100644 --- a/.github/workflows/firewall-escape.lock.yml +++ b/.github/workflows/firewall-escape.lock.yml @@ -1867,6 +1867,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_THREAT_DETECTION_AIC: ${{ needs.detection.outputs.aic }} diff --git a/.github/workflows/impeccable-skills-reviewer.lock.yml b/.github/workflows/impeccable-skills-reviewer.lock.yml index 16cfd16d4cb..671ada4e944 100644 --- a/.github/workflows/impeccable-skills-reviewer.lock.yml +++ b/.github/workflows/impeccable-skills-reviewer.lock.yml @@ -1745,6 +1745,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: "claude-sonnet-4.6" GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧵 *Reviewed using Impeccable skills by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🧵 [{workflow_name}]({run_url}) is reviewing this {event_type} using Impeccable skills...\",\"runSuccess\":\"🧵 [{workflow_name}]({run_url}) has completed the skills-based review. ✅\",\"runFailure\":\"🧵 [{workflow_name}]({run_url}) {status} during the skills-based review.\"}" diff --git a/.github/workflows/mattpocock-skills-reviewer.lock.yml b/.github/workflows/mattpocock-skills-reviewer.lock.yml index 673104abfb2..d87d1f8b1de 100644 --- a/.github/workflows/mattpocock-skills-reviewer.lock.yml +++ b/.github/workflows/mattpocock-skills-reviewer.lock.yml @@ -1929,6 +1929,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: "claude-sonnet-4.6" GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧠 *Reviewed using Matt Pocock's skills by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🧠 [{workflow_name}]({run_url}) is reviewing this {event_type} using Matt Pocock's engineering skills...\",\"runSuccess\":\"🧠 [{workflow_name}]({run_url}) has completed the skills-based review. ✅\",\"runFailure\":\"🧠 [{workflow_name}]({run_url}) {status} during the skills-based review.\"}" diff --git a/.github/workflows/pr-code-quality-reviewer.lock.yml b/.github/workflows/pr-code-quality-reviewer.lock.yml index 16494735bf2..b8765dbe8d6 100644 --- a/.github/workflows/pr-code-quality-reviewer.lock.yml +++ b/.github/workflows/pr-code-quality-reviewer.lock.yml @@ -2143,6 +2143,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🔎 *Code quality review by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔎 [{workflow_name}]({run_url}) is reviewing code quality for this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed the code quality review.\",\"runFailure\":\"⚠️ [{workflow_name}]({run_url}) {status} during code quality review.\"}" diff --git a/.github/workflows/pr-description-caveman.lock.yml b/.github/workflows/pr-description-caveman.lock.yml index 4d64566e66e..04d4357abeb 100644 --- a/.github/workflows/pr-description-caveman.lock.yml +++ b/.github/workflows/pr-description-caveman.lock.yml @@ -1643,6 +1643,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_THREAT_DETECTION_AIC: ${{ needs.detection.outputs.aic }} diff --git a/.github/workflows/refiner.lock.yml b/.github/workflows/refiner.lock.yml index 4e9cffa3bff..7c54f074765 100644 --- a/.github/workflows/refiner.lock.yml +++ b/.github/workflows/refiner.lock.yml @@ -2075,6 +2075,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"runStarted\":\"🔍 Starting code refinement... [{workflow_name}]({run_url}) is analyzing PR #${{ github.event.pull_request.number }} for style alignment and security issues\",\"runSuccess\":\"✅ Refinement complete! [{workflow_name}]({run_url}) has created a PR with improvements for PR #${{ github.event.pull_request.number }}\",\"runFailure\":\"❌ Refinement failed! [{workflow_name}]({run_url}) {status} while processing PR #${{ github.event.pull_request.number }}\"}" diff --git a/.github/workflows/smoke-agent-all-merged.lock.yml b/.github/workflows/smoke-agent-all-merged.lock.yml index 30aa014230a..cdbcc4bd2b7 100644 --- a/.github/workflows/smoke-agent-all-merged.lock.yml +++ b/.github/workflows/smoke-agent-all-merged.lock.yml @@ -1769,6 +1769,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🤖 *Guard policy smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔍 [{workflow_name}]({run_url}) testing guard policy: `repos=all, min-integrity=merged`...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed guard policy test.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status}. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-agent-all-none.lock.yml b/.github/workflows/smoke-agent-all-none.lock.yml index cf1b1e67ea7..3b43785a144 100644 --- a/.github/workflows/smoke-agent-all-none.lock.yml +++ b/.github/workflows/smoke-agent-all-none.lock.yml @@ -1769,6 +1769,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🤖 *Guard policy smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔍 [{workflow_name}]({run_url}) testing guard policy: `repos=all, min-integrity=none`...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed guard policy test.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status}. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-agent-public-approved.lock.yml b/.github/workflows/smoke-agent-public-approved.lock.yml index bb4c1a59b43..74bab5e9f0a 100644 --- a/.github/workflows/smoke-agent-public-approved.lock.yml +++ b/.github/workflows/smoke-agent-public-approved.lock.yml @@ -1821,6 +1821,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🤖 *Smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🤖 [{workflow_name}]({run_url}) is looking for a Smoke issue to assign...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed. Issue assigned to the agentic-workflows agent.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status}. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-agent-public-none.lock.yml b/.github/workflows/smoke-agent-public-none.lock.yml index c2624578972..ca8376f0244 100644 --- a/.github/workflows/smoke-agent-public-none.lock.yml +++ b/.github/workflows/smoke-agent-public-none.lock.yml @@ -1769,6 +1769,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🤖 *Guard policy smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔍 [{workflow_name}]({run_url}) testing guard policy: `repos=public, min-integrity=none`...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed guard policy test.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status}. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-agent-scoped-approved.lock.yml b/.github/workflows/smoke-agent-scoped-approved.lock.yml index db04b271e9a..2df11c2444b 100644 --- a/.github/workflows/smoke-agent-scoped-approved.lock.yml +++ b/.github/workflows/smoke-agent-scoped-approved.lock.yml @@ -1776,6 +1776,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🤖 *Guard policy smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔍 [{workflow_name}]({run_url}) testing guard policy: `repos=[github/gh-aw, github/*], min-integrity=approved`...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed guard policy test.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status}. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-antigravity.lock.yml b/.github/workflows/smoke-antigravity.lock.yml index 91a8b3c06df..aaef7a060c6 100644 --- a/.github/workflows/smoke-antigravity.lock.yml +++ b/.github/workflows/smoke-antigravity.lock.yml @@ -1886,6 +1886,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "antigravity" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e ✨ *[{workflow_name}]({run_url}) — Powered by Antigravity*{ai_credits_suffix}{history_link}\",\"runStarted\":\"✨ Antigravity awakens... [{workflow_name}]({run_url}) begins its journey on this {event_type}...\",\"runSuccess\":\"🚀 [{workflow_name}]({run_url}) **MISSION COMPLETE!** Antigravity has spoken. ✨\",\"runFailure\":\"⚠️ [{workflow_name}]({run_url}) {status}. Antigravity encountered unexpected challenges...\"}" diff --git a/.github/workflows/smoke-call-workflow.lock.yml b/.github/workflows/smoke-call-workflow.lock.yml index 3e0fe38be95..cc8c6e11d2d 100644 --- a/.github/workflows/smoke-call-workflow.lock.yml +++ b/.github/workflows/smoke-call-workflow.lock.yml @@ -1813,6 +1813,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "codex" GH_AW_ENGINE_MODEL: "gpt-5.4-mini" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_THREAT_DETECTION_AIC: ${{ needs.detection.outputs.aic }} diff --git a/.github/workflows/smoke-ci.lock.yml b/.github/workflows/smoke-ci.lock.yml index 6497e9f9115..201788236e7 100644 --- a/.github/workflows/smoke-ci.lock.yml +++ b/.github/workflows/smoke-ci.lock.yml @@ -1738,6 +1738,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_WORKFLOW_EMOJI: "🧪" diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index d6739a5587c..1d2d5f938d5 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -2665,6 +2665,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "claude" GH_AW_ENGINE_MODEL: "claude-sonnet-4-6" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 💥 *[THE END] — Illustrated by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"💥 **WHOOSH!** [{workflow_name}]({run_url}) springs into action on this {event_type}! *[Panel 1 begins...]*\",\"runSuccess\":\"🎬 **THE END** — [{workflow_name}]({run_url}) **MISSION: ACCOMPLISHED!** The hero saves the day! ✨\",\"runFailure\":\"💫 **TO BE CONTINUED...** [{workflow_name}]({run_url}) {status}! Our hero faces unexpected challenges...\"}" diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index 046c677846b..e7361735842 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -2277,6 +2277,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "codex" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🔮 *The oracle has spoken through [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔮 The ancient spirits stir... [{workflow_name}]({run_url}) awakens to divine this {event_type}...\",\"runSuccess\":\"✨ The prophecy is fulfilled... [{workflow_name}]({run_url}) has completed its mystical journey. The stars align. 🌟\",\"runFailure\":\"🌑 The shadows whisper... [{workflow_name}]({run_url}) {status}. The oracle requires further meditation...\"}" diff --git a/.github/workflows/smoke-copilot-arm.lock.yml b/.github/workflows/smoke-copilot-arm.lock.yml index f7cb0ee6635..8196f1a4fc1 100644 --- a/.github/workflows/smoke-copilot-arm.lock.yml +++ b/.github/workflows/smoke-copilot-arm.lock.yml @@ -2695,6 +2695,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 📰 *BREAKING: Report filed by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"appendOnlyComments\":true,\"runStarted\":\"📰 BREAKING: [{workflow_name}]({run_url}) is now investigating this {event_type}. Sources say the story is developing...\",\"runSuccess\":\"📰 VERDICT: [{workflow_name}]({run_url}) has concluded. All systems operational. This is a developing story. 🎤\",\"runFailure\":\"📰 DEVELOPING STORY: [{workflow_name}]({run_url}) reports {status}. Our correspondents are investigating the incident...\"}" diff --git a/.github/workflows/smoke-create-cross-repo-pr.lock.yml b/.github/workflows/smoke-create-cross-repo-pr.lock.yml index aca0b67c987..9546938f4f3 100644 --- a/.github/workflows/smoke-create-cross-repo-pr.lock.yml +++ b/.github/workflows/smoke-create-cross-repo-pr.lock.yml @@ -1821,6 +1821,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🔬 *Cross-repo smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is testing cross-repo PR creation in github/gh-aw-side-repo...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) successfully created a cross-repo PR in github/gh-aw-side-repo!\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) failed to create a cross-repo PR: {status}\"}" diff --git a/.github/workflows/smoke-gemini.lock.yml b/.github/workflows/smoke-gemini.lock.yml index e2a524b820c..8bc98d07aea 100644 --- a/.github/workflows/smoke-gemini.lock.yml +++ b/.github/workflows/smoke-gemini.lock.yml @@ -1967,6 +1967,7 @@ jobs: GH_AW_ENGINE_ID: "gemini" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "0.39.1" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e ✨ *[{workflow_name}]({run_url}) — Powered by Gemini*{ai_credits_suffix}{history_link}\",\"runStarted\":\"✨ Gemini awakens... [{workflow_name}]({run_url}) begins its journey on this {event_type}...\",\"runSuccess\":\"🚀 [{workflow_name}]({run_url}) **MISSION COMPLETE!** Gemini has spoken. ✨\",\"runFailure\":\"⚠️ [{workflow_name}]({run_url}) {status}. Gemini encountered unexpected challenges...\"}" diff --git a/.github/workflows/smoke-multi-pr.lock.yml b/.github/workflows/smoke-multi-pr.lock.yml index 64d256d1707..7698ceff09e 100644 --- a/.github/workflows/smoke-multi-pr.lock.yml +++ b/.github/workflows/smoke-multi-pr.lock.yml @@ -1773,6 +1773,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Multi PR smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"appendOnlyComments\":true,\"runStarted\":\"🧪 [{workflow_name}]({run_url}) is now testing multiple PR creation...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) successfully created multiple PRs.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) failed to create multiple PRs. Check the logs.\"}" diff --git a/.github/workflows/smoke-opencode.lock.yml b/.github/workflows/smoke-opencode.lock.yml index c06f689e3a8..d8cbf5af3d3 100644 --- a/.github/workflows/smoke-opencode.lock.yml +++ b/.github/workflows/smoke-opencode.lock.yml @@ -1832,6 +1832,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "opencode" GH_AW_ENGINE_MODEL: "copilot/claude-sonnet-4.5" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🔥 *[{workflow_name}]({run_url}) — Powered by OpenCode*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔥 OpenCode initializing... [{workflow_name}]({run_url}) begins on this {event_type}...\",\"runSuccess\":\"🚀 [{workflow_name}]({run_url}) **MISSION COMPLETE!** OpenCode delivered. 🔥\",\"runFailure\":\"⚠️ [{workflow_name}]({run_url}) {status}. OpenCode encountered unexpected challenges...\"}" diff --git a/.github/workflows/smoke-pi.lock.yml b/.github/workflows/smoke-pi.lock.yml index 0cc62112b7a..69f7c46971f 100644 --- a/.github/workflows/smoke-pi.lock.yml +++ b/.github/workflows/smoke-pi.lock.yml @@ -1776,6 +1776,7 @@ jobs: GH_AW_EFFECTIVE_TOKENS: ${{ needs.agent.outputs.effective_tokens }} GH_AW_ENGINE_ID: "pi" GH_AW_ENGINE_MODEL: "copilot/gpt-5.4" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🥧 *[{workflow_name}]({run_url}) — Powered by Pi*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🥧 Pi initializing... [{workflow_name}]({run_url}) begins on this {event_type}...\",\"runSuccess\":\"🚀 [{workflow_name}]({run_url}) **MISSION COMPLETE!** Pi delivered. 🥧\",\"runFailure\":\"⚠️ [{workflow_name}]({run_url}) {status}. Pi encountered unexpected challenges...\"}" diff --git a/.github/workflows/smoke-project.lock.yml b/.github/workflows/smoke-project.lock.yml index 376e56d34bb..db43de651cb 100644 --- a/.github/workflows/smoke-project.lock.yml +++ b/.github/workflows/smoke-project.lock.yml @@ -2099,6 +2099,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Project smoke test report by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"appendOnlyComments\":true,\"runStarted\":\"🧪 [{workflow_name}]({run_url}) is now testing project operations...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed successfully. All project operations validated.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) encountered failures. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-temporary-id.lock.yml b/.github/workflows/smoke-temporary-id.lock.yml index 0269260310d..d2a13f4298d 100644 --- a/.github/workflows/smoke-temporary-id.lock.yml +++ b/.github/workflows/smoke-temporary-id.lock.yml @@ -1948,6 +1948,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Temporary ID smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"appendOnlyComments\":true,\"runStarted\":\"🧪 [{workflow_name}]({run_url}) is now testing temporary ID functionality...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed successfully. Temporary ID validation passed.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) encountered failures. Check the logs for details.\"}" diff --git a/.github/workflows/smoke-test-tools.lock.yml b/.github/workflows/smoke-test-tools.lock.yml index 955775fea8d..e115316157b 100644 --- a/.github/workflows/smoke-test-tools.lock.yml +++ b/.github/workflows/smoke-test-tools.lock.yml @@ -1797,6 +1797,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🔧 *Tool validation by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔧 Starting tool validation... [{workflow_name}]({run_url}) is checking the agent container tools...\",\"runSuccess\":\"✅ All tools validated successfully! [{workflow_name}]({run_url}) confirms agent container is ready.\",\"runFailure\":\"❌ Tool validation failed! [{workflow_name}]({run_url}) detected missing tools: {status}\"}" diff --git a/.github/workflows/smoke-update-cross-repo-pr.lock.yml b/.github/workflows/smoke-update-cross-repo-pr.lock.yml index a3aceda055a..d60042cbff1 100644 --- a/.github/workflows/smoke-update-cross-repo-pr.lock.yml +++ b/.github/workflows/smoke-update-cross-repo-pr.lock.yml @@ -1855,6 +1855,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 📜 *Cross-repo PR update smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"📜 [{workflow_name}]({run_url}) is adding the next Odyssey line to github/gh-aw-side-repo PR #1...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) successfully updated the cross-repo PR with a new Odyssey line!\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) failed to update the cross-repo PR: {status}\"}" diff --git a/.github/workflows/test-quality-sentinel.lock.yml b/.github/workflows/test-quality-sentinel.lock.yml index db81fc21217..bd2857776f0 100644 --- a/.github/workflows/test-quality-sentinel.lock.yml +++ b/.github/workflows/test-quality-sentinel.lock.yml @@ -1858,6 +1858,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: "${{ needs.activation.outputs.model_size }}" GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" diff --git a/.github/workflows/visual-regression-checker.lock.yml b/.github/workflows/visual-regression-checker.lock.yml index 0493d086196..acfa7094797 100644 --- a/.github/workflows/visual-regression-checker.lock.yml +++ b/.github/workflows/visual-regression-checker.lock.yml @@ -1705,6 +1705,7 @@ jobs: GH_AW_ENGINE_ID: "copilot" GH_AW_ENGINE_MODEL: ${{ needs.agent.outputs.model }} GH_AW_ENGINE_VERSION: "1.0.75" + GH_AW_HEAD_SHA: ${{ github.event.pull_request.head.sha }} GH_AW_PROJECT_UTC: "-08:00" GH_AW_RUNTIME_FEATURES: ${{ vars.GH_AW_RUNTIME_FEATURES }} GH_AW_THREAT_DETECTION_AIC: ${{ needs.detection.outputs.aic }} From 09914cdb1941e541e5fb1b2651afda7b93e36226 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:54:39 +0000 Subject: [PATCH 7/7] feat: support user-specified commit-id in submit-pull-request-review and create-pull-request-review-comment Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/create_pr_review_comment.cjs | 10 ++++++ actions/setup/js/pr_review_buffer.cjs | 35 +++++++++++++++++-- actions/setup/js/submit_pr_review.cjs | 10 ++++++ pkg/parser/schemas/main_workflow_schema.json | 8 +++++ pkg/workflow/create_pr_review_comment.go | 7 ++++ pkg/workflow/safe_outputs_handler_registry.go | 2 ++ pkg/workflow/submit_pr_review.go | 7 ++++ 7 files changed, 77 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index a11fca3db2b..41915f959e0 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -74,6 +74,16 @@ async function main(config = {}) { logStagedPreviewInfo("PR review comments will be previewed without being submitted"); } + const pinnedCommitId = typeof config.commit_id === "string" ? config.commit_id.trim() : ""; + if (pinnedCommitId) { + core.info(`create_pull_request_review_comment: commit-id pinned to ${pinnedCommitId}`); + if (registry && typeof registry.setDefaultPinnedCommitId === "function") { + registry.setDefaultPinnedCommitId(pinnedCommitId); + } else if (legacyBuffer && typeof legacyBuffer.setPinnedCommitId === "function") { + legacyBuffer.setPinnedCommitId(pinnedCommitId); + } + } + // Extract triggering context for footer generation const triggeringIssueNumber = context.payload?.issue?.number && !context.payload?.issue?.pull_request ? context.payload.issue.number : undefined; const triggeringPRNumber = context.payload?.pull_request?.number || (context.payload?.issue?.pull_request ? context.payload.issue.number : undefined); diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs index 8a830cfede6..56bd1a19fdb 100644 --- a/actions/setup/js/pr_review_buffer.cjs +++ b/actions/setup/js/pr_review_buffer.cjs @@ -112,6 +112,9 @@ function createReviewBuffer() { /** @type {boolean} When true, dismiss older same-workflow REQUEST_CHANGES reviews after posting a replacement review. */ let supersedeOlderReviews = false; + /** @type {string} When non-empty, pins the review to this commit SHA instead of the live PR head or GH_AW_HEAD_SHA. */ + let pinnedCommitId = ""; + /** * Best-effort execution-state capture. * When the installation token is out of quota, metadata collection should not @@ -245,6 +248,17 @@ function createReviewBuffer() { } } + /** + * Pin the review to a specific commit SHA, overriding GH_AW_HEAD_SHA and the live PR head. + * @param {string} commitId - The commit SHA to pin the review to + */ + function setPinnedCommitId(commitId) { + if (commitId && typeof commitId === "string") { + pinnedCommitId = commitId; + core.info(`PR review pinned to commit: ${commitId}`); + } + } + /** * Check if there are buffered comments to submit. * @returns {boolean} @@ -303,9 +317,12 @@ function createReviewBuffer() { // commit the agent actually reviewed, preventing attribution drift when new commits are // pushed during the run (most common under workflow_run triggers where the safe_outputs // job runs after the agent job and pulls.get() may return a newer HEAD sha). + // A user-specified pinnedCommitId (from the commit-id config option) takes highest priority. const awHeadSHA = process.env.GH_AW_HEAD_SHA || ""; - const resolvedCommitId = awHeadSHA || pullRequest.head.sha; - if (awHeadSHA && awHeadSHA !== pullRequest.head.sha) { + const resolvedCommitId = pinnedCommitId || awHeadSHA || pullRequest.head.sha; + if (pinnedCommitId && pinnedCommitId !== pullRequest.head.sha) { + core.info(`Using config-pinned commit SHA: ${pinnedCommitId} (PR head is now ${pullRequest.head.sha})`); + } else if (awHeadSHA && awHeadSHA !== pullRequest.head.sha) { core.info(`Using trigger-time head SHA: ${awHeadSHA} (PR head is now ${pullRequest.head.sha})`); } @@ -785,6 +802,7 @@ function createReviewBuffer() { setIncludeFooter: setFooterMode, // Backward compatibility alias setStaged, setSupersedeOlderReviews, + setPinnedCommitId, hasBufferedComments, hasReviewMetadata, getBufferedCount, @@ -817,6 +835,8 @@ function createPrReviewBufferRegistry() { let defaultFooterContext = null; let defaultStaged = false; let defaultSupersedeOlderReviews = false; + /** @type {string} */ + let defaultPinnedCommitId = ""; /** * Get or create the buffer for the given (repo, prNumber) pair. @@ -842,6 +862,9 @@ function createPrReviewBufferRegistry() { if (defaultSupersedeOlderReviews) { buffer.setSupersedeOlderReviews(true); } + if (defaultPinnedCommitId) { + buffer.setPinnedCommitId(defaultPinnedCommitId); + } bufferMap.set(k, buffer); insertionOrder.push({ repo, prNumber, buffer }); core.info(`PR review registry: created buffer for ${repo}#${prNumber}`); @@ -885,6 +908,13 @@ function createPrReviewBufferRegistry() { defaultSupersedeOlderReviews = value === true; } + /** @param {string} value */ + function setDefaultPinnedCommitId(value) { + if (value && typeof value === "string") { + defaultPinnedCommitId = value; + } + } + return { getOrCreate, getAllEntries, @@ -893,6 +923,7 @@ function createPrReviewBufferRegistry() { setDefaultFooterContext, setDefaultStaged, setDefaultSupersedeOlderReviews, + setDefaultPinnedCommitId, }; } diff --git a/actions/setup/js/submit_pr_review.cjs b/actions/setup/js/submit_pr_review.cjs index d9d8e8a01d8..d69ec229cd1 100644 --- a/actions/setup/js/submit_pr_review.cjs +++ b/actions/setup/js/submit_pr_review.cjs @@ -84,6 +84,16 @@ async function main(config = {}) { } } + const pinnedCommitId = typeof config.commit_id === "string" ? config.commit_id.trim() : ""; + if (pinnedCommitId) { + core.info(`submit_pull_request_review: commit-id pinned to ${pinnedCommitId}`); + if (registry) { + registry.setDefaultPinnedCommitId(pinnedCommitId); + } else if (legacyBuffer && typeof legacyBuffer.setPinnedCommitId === "function") { + legacyBuffer.setPinnedCommitId(pinnedCommitId); + } + } + let processedCount = 0; /** diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index 1790466fa5f..22bcfb1d573 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -7171,6 +7171,10 @@ "required-title-prefix": { "type": "string", "description": "The target item's title must start with this prefix for this operation to proceed" + }, + "commit-id": { + "type": "string", + "description": "Pin the review to this specific commit SHA. When set, the review is attributed to this commit instead of the current PR head. GitHub will mark the review as outdated if the PR head has since moved." } }, "additionalProperties": false @@ -7280,6 +7284,10 @@ "required-title-prefix": { "type": "string", "description": "The target item's title must start with this prefix for this operation to proceed" + }, + "commit-id": { + "type": "string", + "description": "Pin the review to this specific commit SHA. When set, the review is attributed to this commit instead of the current PR head. GitHub will mark the review as outdated if the PR head has since moved." } }, "additionalProperties": false diff --git a/pkg/workflow/create_pr_review_comment.go b/pkg/workflow/create_pr_review_comment.go index 4c281831f5c..f84a62134ce 100644 --- a/pkg/workflow/create_pr_review_comment.go +++ b/pkg/workflow/create_pr_review_comment.go @@ -14,6 +14,7 @@ type CreatePullRequestReviewCommentsConfig struct { Target string `yaml:"target,omitempty"` // Target for comments: "triggering" (default), "*" (any PR), or explicit PR number TargetRepoSlug string `yaml:"target-repo,omitempty"` // Target repository in format "owner/repo" for cross-repository PR review comments AllowedRepos []string `yaml:"allowed-repos,omitempty"` // List of additional repositories that PR review comments can be added to (additionally to the target-repo) + CommitId string `yaml:"commit-id,omitempty"` // When set, pins the review to this commit SHA instead of the current PR head. } func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any) *CreatePullRequestReviewCommentsConfig { @@ -52,6 +53,12 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any } prReviewCommentsConfig.TargetRepoSlug = targetRepoSlug + if commitId, exists := configMap["commit-id"]; exists { + if commitIdStr, ok := commitId.(string); ok { + prReviewCommentsConfig.CommitId = commitIdStr + } + } + // Parse common base fields with default max of 10 c.parseBaseSafeOutputConfig(configMap, &prReviewCommentsConfig.BaseSafeOutputConfig, 10) } else { diff --git a/pkg/workflow/safe_outputs_handler_registry.go b/pkg/workflow/safe_outputs_handler_registry.go index cba92bfb055..c9904adff6e 100644 --- a/pkg/workflow/safe_outputs_handler_registry.go +++ b/pkg/workflow/safe_outputs_handler_registry.go @@ -423,6 +423,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). AddIfNotEmpty("target-repo", c.TargetRepoSlug). AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("commit_id", c.CommitId). AddIfNotEmpty("github-token", c.GitHubToken). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). Build() @@ -441,6 +442,7 @@ var handlerRegistry = map[string]handlerBuilder{ AddIfTrue("supersede_older_reviews", c.SupersedeOlderReviews).AddStringSlice("required_labels", c.RequiredLabels). AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).AddIfNotEmpty("github-token", c.GitHubToken). AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). + AddIfNotEmpty("commit_id", c.CommitId). AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)). Build() }, diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go index 18ecdf44997..edc07bff36f 100644 --- a/pkg/workflow/submit_pr_review.go +++ b/pkg/workflow/submit_pr_review.go @@ -20,6 +20,7 @@ type SubmitPullRequestReviewConfig struct { Footer *string `yaml:"footer,omitempty"` // Controls when to show footer in PR review body: "always" (default), "none", or "if-body" (only when review has body text) AllowedEvents []string `yaml:"allowed-events,omitempty"` // Optional list of allowed review event types: APPROVE, COMMENT, REQUEST_CHANGES. If omitted, all event types are allowed. SupersedeOlderReviews bool `yaml:"supersede-older-reviews,omitempty"` // When true, dismisses older same-workflow REQUEST_CHANGES reviews after a replacement review is posted. + CommitId string `yaml:"commit-id,omitempty"` // When set, pins the review to this commit SHA instead of the current PR head. } // parseSubmitPullRequestReviewConfig handles submit-pull-request-review configuration @@ -112,6 +113,12 @@ func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) } } + if commitId, exists := configMap["commit-id"]; exists { + if commitIdStr, ok := commitId.(string); ok { + config.CommitId = commitIdStr + } + } + submitPRReviewLog.Printf("Parsed submit-pull-request-review config: max=%d, target=%s, target_repo=%s, allowed_events=%v, supersede_older_reviews=%t", templatableIntValue(config.Max), config.Target, config.TargetRepoSlug, config.AllowedEvents, config.SupersedeOlderReviews) } else { // If configData is nil or not a map, set the default max