Fix centralized workflow dispatch API usage and status comment attribution #42077
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,16 +295,29 @@ async function addImmediateStatusComment() { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * @param {Array<unknown>} values | ||
| * @returns {string} | ||
| */ | ||
| function firstNonEmptyString(values) { | ||
| for (const value of values) { | ||
| if (typeof value === "string" && value.trim()) { | ||
| return value; | ||
| } | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| /** | ||
| * Dispatches a workflow with the API version header required by GitHub REST. | ||
| * @param {string} workflowId | ||
| * @param {string} ref | ||
| * @param {Record<string, string>} inputs | ||
| * @returns {Promise<{ dispatched: boolean, run_id?: number, run_url?: string }>} | ||
| */ | ||
| async function dispatchWorkflow(workflowId, ref, inputs) { | ||
| try { | ||
| const dispatchArgs = { | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: workflowId, | ||
|
|
@@ -313,17 +326,91 @@ async function dispatchWorkflow(workflowId, ref, inputs) { | |
| headers: { | ||
| "X-GitHub-Api-Version": GITHUB_API_VERSION, | ||
| }, | ||
| }; | ||
|
|
||
| /** @type {{ data?: any }} */ | ||
| let response; | ||
| try { | ||
| response = await github.request("POST /repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches", { | ||
| ...dispatchArgs, | ||
| return_run_details: true, | ||
| }); | ||
| } catch (dispatchError) { | ||
| /** @type {any} */ | ||
| const err = dispatchError; | ||
| const status = err && typeof err === "object" ? err.status : undefined; | ||
| const message = typeof err?.response?.data?.message === "string" ? err.response.data.message : String(dispatchError); | ||
| const isValidationStatus = status === 400 || status === 422; | ||
| const mentionsReturnRunDetails = typeof message === "string" && message.toLowerCase().includes("return_run_details"); | ||
|
Contributor
There was a problem hiding this comment.
💡 Suggested fixRetry on all 400/422 errors — the worst case from a false positive is an unnecessary second request. Since const isValidationStatus = status === 400 || status === 422;
if (!isValidationStatus) {
throw err;
}
core.info("Workflow dispatch endpoint returned 4xx; retrying without return_run_details (GHES compatibility fallback).");
response = await github.request(...);This is strictly more robust and no more dangerous than the current implementation. |
||
| if (!(isValidationStatus && mentionsReturnRunDetails)) { | ||
| throw err; | ||
| } | ||
| core.info("Workflow dispatch endpoint rejected 'return_run_details' (common on older GHES versions); retrying without it."); | ||
| response = await github.request("POST /repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches", dispatchArgs); | ||
|
Contributor
There was a problem hiding this comment. The @copilot please address this. |
||
| } | ||
|
|
||
| const responseData = response?.data || {}; | ||
| // GitHub may return run metadata in either shape: | ||
| // - flat: { workflow_run_id, workflow_run_url } (workflow_run_url is a REST API URL) | ||
| // - nested: { workflow_run: { id, html_url, url } } (url is a REST API URL; html_url is the Actions page URL) | ||
| const parsedRunId = Number(responseData?.workflow_run_id ?? responseData?.workflow_run?.id); | ||
| const runId = Number.isFinite(parsedRunId) && parsedRunId > 0 ? parsedRunId : undefined; | ||
| const serverUrl = context.serverUrl || process.env.GITHUB_SERVER_URL || "https://github.com"; | ||
| // Prefer the constructed HTML URL when runId is available — it is always the Actions run page URL. | ||
| // Avoid workflow_run_url and workflow_run.url which are REST API URLs, not Actions run page URLs. | ||
| const runUrlFromRunId = runId ? `${serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId}` : undefined; | ||
| const runUrlFromResponse = firstNonEmptyString([responseData?.workflow_run?.html_url]); | ||
| const runUrl = runUrlFromRunId || runUrlFromResponse; | ||
| return { | ||
| dispatched: true, | ||
| ...(runId ? { run_id: runId } : {}), | ||
| ...(runUrl ? { run_url: runUrl } : {}), | ||
| }; | ||
| } catch (error) { | ||
| if (isDisabledWorkflowDispatchError(error)) { | ||
| core.info(`Skipping workflow '${workflowId}' because it is disabled.`); | ||
| return { dispatched: false }; | ||
| } | ||
| throw new Error(`Failed to dispatch workflow '${workflowId}' on ref '${ref}': ${String(error)}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Update the shared status comment with dispatched workflow run metadata. | ||
| * @param {{ status_comment_id: string, status_comment_url?: string, status_comment_repo?: string }} statusCommentContext | ||
| * @param {string} eventName | ||
| * @param {string} workflowId | ||
| * @param {string|undefined} runUrl | ||
| */ | ||
| async function updateStatusCommentWithDispatch(statusCommentContext, eventName, workflowId, runUrl) { | ||
| if (!statusCommentContext?.status_comment_id) { | ||
| return; | ||
| } | ||
| if (!runUrl) { | ||
|
Contributor
There was a problem hiding this comment. Status comment is silently left stale when the server returns no run URL (old GHES): when 💡 Suggested improvementAdd a log so that operators can diagnose missing enrichment in GHES environments: if (!runUrl) {
core.info(`Skipping status comment enrichment for '${workflowId}': dispatch response contained no run URL (older GHES may not support return_run_details).`);
return;
}If workflow-name attribution is considered important enough to justify the effort, you could also patch the comment with just the dispatched workflow name even when |
||
| return; | ||
| } | ||
| try { | ||
| await createOrReuseStatusComment({ | ||
| ...context, | ||
| eventName: "workflow_dispatch", | ||
| payload: { | ||
| inputs: { | ||
| event_name: eventName, | ||
| event_payload: JSON.stringify(context.payload || {}), | ||
| aw_context: JSON.stringify({ | ||
| ...statusCommentContext, | ||
| dispatched_workflow_name: workflowId.replace(/\.lock\.yml$/, ""), | ||
| dispatched_run_url: runUrl, | ||
| }), | ||
| }, | ||
| }, | ||
| nonFatalStatusCommentErrors: true, | ||
| }); | ||
| } catch (error) { | ||
| core.warning(`Failed to update immediate status comment with dispatched run details: ${String(error)}`); | ||
| } | ||
| } | ||
|
|
||
| function isBuiltinHelpEnabled() { | ||
| const raw = (process.env.GH_AW_HELP_COMMAND_ENABLED || "").trim().toLowerCase(); | ||
| if (!raw || raw === "true") { | ||
|
|
@@ -602,7 +689,7 @@ async function main() { | |
| const dispatched = await dispatchWorkflow(workflowID, ref, { | ||
| aw_context: JSON.stringify(awContext), | ||
| }); | ||
| if (dispatched.dispatched) { | ||
| core.info(`Dispatched '${workflowID}' for label '${labelName}'`); | ||
| } | ||
| } | ||
|
|
@@ -669,8 +756,11 @@ async function main() { | |
| const dispatched = await dispatchWorkflow(workflowID, ref, { | ||
| aw_context: JSON.stringify(awContext), | ||
| }); | ||
| if (dispatched.dispatched) { | ||
| core.info(`Dispatched '${workflowID}' for '/${commandName}'`); | ||
| if (maintainsStatusComment(route) && statusCommentContext) { | ||
| await updateStatusCommentWithDispatch(statusCommentContext, identifier, workflowID, dispatched.run_url); | ||
|
Contributor
There was a problem hiding this comment. When multiple routes in the same slash command have @copilot please address this.
Contributor
There was a problem hiding this comment. Status comment stomped on each loop iteration in fan-out scenarios: 💡 Impact and suggested fixIn the existing test Options:
|
||
| } | ||
| } | ||
| } | ||
| core.info(`Completed centralized routing for '/${commandName}'.`); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Visible workflow name and hidden workflow-ID marker are now out of sync:
workflowName(user-visible) correctly usesworkflowNameOverride, but thegenerateWorkflowIdMarkercall a few lines below still usesprocess.env.GITHUB_WORKFLOW(the router's name). When the dispatched workflowarchie.lock.ymllater runs its ownadd_workflow_run_commentstep, it searches for an existing status comment tagged with markerarchie— but the comment was tagged with the router's marker (e.g.,agentic-commands). The match fails, a second status comment is created, and the original router-enriched comment is orphaned.💡 Suggested fix
Pass the workflow name override through to the marker as well:
Alternatively, keep the router marker intentionally and document that the reuse contract relies on
status_comment_idrather than marker matching — but the current code makes this implicit and fragile.