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 2 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.

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
4 changes: 4 additions & 0 deletions actions/setup/js/dispatch_workflow.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ async function main(config = {}) {
core.info(`✓ Successfully dispatched workflow: ${workflowFile} (run ID: ${runId})`);
} else {
core.info(`✓ Successfully dispatched workflow: ${workflowFile}`);
if (typeof message.temporary_id === "string") {

@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] Missing test for dispatch_workflow when temporary_id is provided but the API returns no workflow_run_id — the warning branch fires but there's no assertion that temporaryId is absent from the response.

💡 Suggested test 
it("should not include temporaryId when API returns no workflow_run_id", async () => {
  github.rest.actions.createWorkflowDispatch.mockResolvedValueOnce({ data: {} }); // no workflow_run_id
  const handler = await main({ workflows: ["test-workflow"], workflow_files: { "test-workflow": ".lock.yml" } });
  const result = await handler({ type: "dispatch_workflow", workflow_name: "test-workflow", temporary_id: "aw_run123", inputs: {} }, {});
  expect(result.success).toBe(true);
  expect(result.temporaryId).toBeUndefined();
  expect(core.warning).toHaveBeenCalledWith(expect.stringContaining("aw_run123"));
});

Without this test the warning path is exercised only manually.

@copilot please address this.

core.warning(`Unable to register temporary ID '${message.temporary_id}' because the GitHub API did not return a workflow run ID.`);
}
}

// Record the time of this dispatch for rate limiting
Expand All @@ -335,6 +338,7 @@ async function main(config = {}) {
workflow_name: workflowName,
inputs: inputs,
run_id: runId,
...(typeof message.temporary_id === "string" && runId ? { temporaryId: message.temporary_id, repo: resolvedRepoSlug, number: runId } : {}),
};
} catch (error) {
const errorMessage = getErrorMessage(error);
Expand Down
20 changes: 20 additions & 0 deletions actions/setup/js/dispatch_workflow.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,6 +839,26 @@ describe("dispatch_workflow handler factory", () => {
expect(core.info).toHaveBeenCalledWith(expect.stringContaining("run ID: 987654"));
});

it("should return temporary ID mapping for a dispatched workflow run", async () => {
github.rest.actions.createWorkflowDispatch.mockResolvedValueOnce({
data: { workflow_run_id: 987654 },
});
const handler = await main({
workflows: ["test-workflow"],
workflow_files: { "test-workflow": ".lock.yml" },
});

const result = await handler({ type: "dispatch_workflow", workflow_name: "test-workflow", temporary_id: "aw_run123", inputs: {} }, {});

expect(result).toMatchObject({
success: true,
run_id: 987654,
temporaryId: "aw_run123",
repo: "test-owner/test-repo",
number: 987654,
});
});

it("should succeed without run_id when API returns no workflow_run_id", async () => {
github.rest.actions.createWorkflowDispatch.mockResolvedValueOnce({ data: {} });

Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/safe_outputs_tools.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1916,14 +1916,14 @@
},
{
"name": "approve_workflow_run",
"description": "Approve a GitHub Actions workflow run awaiting required approval. Supply the positive run ID from the workflow run URL only when the run belongs to the triggering pull request or an explicitly allowed pull request. The handler approves only pull request runs with status waiting; it rejects other runs.",
"description": "Approve a GitHub Actions workflow run awaiting required approval. Supply the positive run ID from the workflow run URL or a temporary ID returned by dispatch_workflow, only when the run belongs to the triggering pull request or an explicitly allowed pull request. The handler approves only pull request runs with status waiting; it rejects other runs.",
"inputSchema": {
"type": "object",
"required": ["run_id"],
"properties": {
"run_id": {
"type": ["number", "string"],
"description": "Positive integer workflow run ID to approve (for example, 123456789 from /actions/runs/123456789).",
"description": "Positive integer workflow run ID to approve, or the temporary ID returned by a dispatch_workflow call (for example, 123456789 from /actions/runs/123456789 or aw_workflow_run).",
"x-synonyms": ["runId", "workflow_run_id"]
},
"secrecy": {
Expand Down
3 changes: 2 additions & 1 deletion actions/setup/js/safe_outputs_tools_loader.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,11 @@ function attachHandlers(tools, handlers, logger) {
// Create a custom handler that wraps args in inputs and adds workflow_name
const workflowName = tool._workflow_name.trim();
tool.handler = args => {
const { ref, ...inputs } = args ?? {};
const { ref, temporary_id, ...inputs } = args ?? {};
// Wrap workflow inputs in inputs and pass dispatch ref as top-level field
return handlers.defaultHandler("dispatch_workflow")({
...(ref && { ref }),
...(temporary_id && { temporary_id }),
...(args !== undefined && { inputs }),
workflow_name: workflowName,
});
Expand Down
15 changes: 15 additions & 0 deletions actions/setup/js/safe_outputs_tools_loader.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,21 @@ describe("safe_outputs_tools_loader", () => {
});
});

it("should pass temporary_id separately from dispatch workflow inputs", () => {
const tools = [{ name: "ci_workflow", description: "CI workflow", _workflow_name: "ci" }];
const mockHandlerFunction = vi.fn();
const handlers = { defaultHandler: vi.fn(() => mockHandlerFunction) };

const result = attachHandlers(tools, handlers);
result[0].handler({ environment: "staging", temporary_id: "aw_run123" });

@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] The safe_outputs_tools_loader.cjs test added for temporary_id passes the value through attachHandlers, but there is no test verifying that temporary_id is absent from the inputs object forwarded to the handler — confirming the destructuring split is correct.

💡 Suggested assertion to add 
expect(mockHandlerFunction).toHaveBeenCalledWith(
  expect.objectContaining({
    temporary_id: "aw_run123",
    inputs: expect.not.objectContaining({ temporary_id: expect.anything() }),
  })
);

Without this, a regression where temporary_id leaks into inputs would go undetected.

@copilot please address this.


expect(mockHandlerFunction).toHaveBeenCalledWith({
workflow_name: "ci",
temporary_id: "aw_run123",
inputs: { environment: "staging" },
});
});

it("should pass ref as top-level field for dispatch_workflow handler", () => {
const tools = [{ name: "ci_workflow", description: "CI workflow", _workflow_name: "ci" }];
const mockHandlerFunction = vi.fn();
Expand Down
4 changes: 2 additions & 2 deletions pkg/workflow/js/safe_outputs_tools.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1916,14 +1916,14 @@
},
{
"name": "approve_workflow_run",
"description": "Approve a GitHub Actions workflow run awaiting required approval. Supply the positive run ID from the workflow run URL only when the run belongs to the triggering pull request or an explicitly allowed pull request. The handler approves only pull request runs with status waiting; it rejects other runs.",
"description": "Approve a GitHub Actions workflow run awaiting required approval. Supply the positive run ID from the workflow run URL or a temporary ID returned by dispatch_workflow, only when the run belongs to the triggering pull request or an explicitly allowed pull request. The handler approves only pull request runs with status waiting; it rejects other runs.",
"inputSchema": {
"type": "object",
"required": ["run_id"],
"properties": {
"run_id": {
"type": ["number", "string"],
"description": "Positive integer workflow run ID to approve (for example, 123456789 from /actions/runs/123456789).",
"description": "Positive integer workflow run ID to approve, or the temporary ID returned by a dispatch_workflow call (for example, 123456789 from /actions/runs/123456789 or aw_workflow_run).",
"x-synonyms": ["runId", "workflow_run_id"]
},
"secrecy": {
Expand Down
8 changes: 4 additions & 4 deletions pkg/workflow/safe_output_validation_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func TestApproveWorkflowRunValidationConfig(t *testing.T) {
if config.DefaultMax != 1 {
t.Errorf("approve_workflow_run DefaultMax = %d, want 1", config.DefaultMax)
}
if runID := config.Fields["run_id"]; !runID.Required || !runID.PositiveInteger {
t.Errorf("approve_workflow_run run_id = %+v, want required positive integer", runID)
if runID := config.Fields["run_id"]; !runID.Required || !runID.IssueNumberOrTemporaryID {
t.Errorf("approve_workflow_run run_id = %+v, want required positive integer or temporary ID", runID)
}

jsonStr, err := GetValidationConfigJSONWithDataSchema([]string{"approve_workflow_run"}, nil, false, nil)
Expand All @@ -96,8 +96,8 @@ func TestApproveWorkflowRunValidationConfig(t *testing.T) {
if len(parsed) != 1 || !ok || parsedConfig.DefaultMax != 1 {
t.Errorf("approve_workflow_run validation config = %#v, want defaultMax 1", parsedConfig)
}
if runID := parsedConfig.Fields["run_id"]; !runID.Required || !runID.PositiveInteger {
t.Errorf("approve_workflow_run generated run_id = %+v, want required positive integer", runID)
if runID := parsedConfig.Fields["run_id"]; !runID.Required || !runID.IssueNumberOrTemporaryID {
t.Errorf("approve_workflow_run generated run_id = %+v, want required positive integer or temporary ID", runID)
}
}

Expand Down
Loading
Loading