Skip to content

Support temporary IDs for dispatched workflow runs #53572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 3 commits into from
Closed

Support temporary IDs for dispatched workflow runs #53572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46a002f
Support temporary IDs for dispatched workflow runs
Copilot Aug 18, 2026
4b54459
Support temporary IDs for dispatched workflow runs
Copilot Aug 18, 2026
4ccd726
Plan: address blocking PR feedback
Copilot Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .github/workflows/daily-team-evolution-insights.lock.yml
View file
Open in desktop

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

9 changes: 7 additions & 2 deletions .github/workflows/mcp-inspector.lock.yml
View file
Open in desktop

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

9 changes: 9 additions & 0 deletions .github/workflows/smoke-copilot-aoai-apikey.lock.yml
View file
Open in desktop

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

9 changes: 9 additions & 0 deletions .github/workflows/smoke-copilot-aoai-entra.lock.yml
View file
Open in desktop

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

9 changes: 9 additions & 0 deletions .github/workflows/smoke-copilot-arm.lock.yml
View file
Open in desktop

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

9 changes: 9 additions & 0 deletions .github/workflows/smoke-copilot.lock.yml
View file
Open in desktop

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

9 changes: 9 additions & 0 deletions .github/workflows/squad-implement-worker.lock.yml
View file
Open in desktop

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

9 changes: 9 additions & 0 deletions .github/workflows/squad.lock.yml
View file
Open in desktop

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

14 changes: 11 additions & 3 deletions actions/setup/js/approve_workflow_run.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const path = require("node:path");
const { isStagedMode } = require("./safe_output_helpers.cjs");
const { logStagedPreviewInfo } = require("./staged_preview.cjs");
const { checkFileProtectionPostApply } = require("./manifest_file_helpers.cjs");
const { loadTemporaryIdMapFromResolved, resolveIssueNumber } = require("./temporary_id.cjs");

/** @type {string} Safe output type handled by this module */
const HANDLER_TYPE = "approve_workflow_run";
Expand Down Expand Up @@ -129,10 +130,17 @@ async function main(config = {}) {

const githubClient = isStaged ? null : await createAuthenticatedGitHubClient(config);

return async function handleApproveWorkflowRun(message) {
const runId = parsePositiveInt(message.run_id);
return async function handleApproveWorkflowRun(message, resolvedTemporaryIds = {}) {
const resolvedRunId = resolveIssueNumber(message.run_id, loadTemporaryIdMapFromResolved(resolvedTemporaryIds));
const runId = resolvedRunId.wasTemporaryId ? (resolvedRunId.resolved?.number ?? undefined) : parsePositiveInt(message.run_id);

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The new approve_workflow_run path trusts resolvedTemporaryIds[...].number without re-validating it as a positive integer, so a malformed or fractional mapping can now flow straight into getWorkflowRun/approval instead of being rejected like direct run_id input is.

💡 Why this needs fixing

resolveIssueNumber() converts number with Number(...), but this call site never re-applies parsePositiveInt() when the source was a temporary ID:

const runId = resolvedRunId.wasTemporaryId
  ? (resolvedRunId.resolved?.number ?? undefined)
  : parsePositiveInt(message.run_id);

That means values like 1.5, 0, -1, or NaN coming from the resolved map can bypass the existing integer validation path and get sent to the Actions API. At best this becomes a flaky runtime failure; at worst it weakens the contract that approve_workflow_run only accepts positive integer run IDs.

Please normalize the resolved temporary-ID value through the same positive-integer validator before using it, and add a test that rejects a temporary ID mapped to a non-integer/non-positive number.

Comment on lines +134 to +135
if (!runId) {
const error = "run_id must be a positive integer";
const error = (resolvedRunId.wasTemporaryId ? resolvedRunId.errorMessage : null) || "run_id must be a positive integer or resolved temporary ID";
core.warning(error);
return { success: false, error };
}
const resolvedRepo = resolvedRunId.resolved?.repo;

@github-actions github-actions Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/tdd] resolvedRepo can be undefined when a stored temporary ID entry has number but no repo field — the cross-repo guard would produce a misleading error like "belongs to undefined".

💡 Suggested fix + regression test

Guard before comparing:

const resolvedRepo = resolvedRunId.resolved?.repo;
if (resolvedRunId.wasTemporaryId) {
  if (!resolvedRepo) {
    const error = `Temporary workflow run ID '${message.run_id}' does not contain a repository reference`;
    core.warning(error);
    return { success: false, error };
  }
  if (resolvedRepo !== `${context.repo.owner}/${context.repo.repo}`) {
    const error = `Temporary workflow run ID '${message.run_id}' belongs to ${resolvedRepo}, not ${context.repo.owner}/${context.repo.repo}`;
    core.warning(error);
    return { success: false, error };
  }
}

And add a test:

it("rejects a temporary ID with missing repo field", async () => {
  const handler = await main(externalTokenConfig);
  const result = await handler({ run_id: "aw_run123" }, { aw_run123: { number: 123 } });
  expect(result.success).toBe(false);
  expect(result.error).toContain("does not contain a repository reference");
});

@copilot please address this.

if (resolvedRunId.wasTemporaryId && resolvedRepo !== `${context.repo.owner}/${context.repo.repo}`) {
const error = `Temporary workflow run ID '${message.run_id}' belongs to ${resolvedRepo}, not ${context.repo.owner}/${context.repo.repo}`;
core.warning(error);
return { success: false, error };
}
Expand Down
21 changes: 21 additions & 0 deletions actions/setup/js/approve_workflow_run.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,27 @@ describe("approve_workflow_run", () => {
expect(mockGetWorkflowRun).toHaveBeenCalledWith(expect.objectContaining({ run_id: 123 }));
});

it("resolves a temporary workflow run ID", async () => {
const { main } = require("./approve_workflow_run.cjs");
const handler = await main(externalTokenConfig);

const result = await handler({ run_id: "aw_run123" }, { aw_run123: { repo: "test-owner/test-repo", number: 123 } });

expect(result.success).toBe(true);
expect(mockGetWorkflowRun).toHaveBeenCalledWith(expect.objectContaining({ run_id: 123 }));
});

it("rejects a temporary workflow run ID from another repository", async () => {
const { main } = require("./approve_workflow_run.cjs");
const handler = await main(externalTokenConfig);

const result = await handler({ run_id: "aw_run123" }, { aw_run123: { repo: "other-owner/other-repo", number: 123 } });

expect(result.success).toBe(false);
expect(result.error).toContain("other-owner/other-repo");
expect(mockGetWorkflowRun).not.toHaveBeenCalled();
});

it.each([undefined, "", 0, -1, 1.5, "abc", "12abc"])("rejects invalid run ID %j", async runId => {
const { main } = require("./approve_workflow_run.cjs");
const handler = await main(externalTokenConfig);
Expand Down
Loading