Fix centralized workflow dispatch API usage and status comment attribution

[Copilot]

Centralized slash-command routing was dispatching workflows via an API path that emits deprecation warnings and did not reliably propagate dispatched run metadata into status comments. This change updates dispatch behavior so status comments link to the dispatched run and display the dispatched workflow name (instead of Agentic Commands).

• Dispatch API + response handling

  • Switch centralized dispatch calls to the workflow dispatch endpoint request form.
  • Request return_run_details when supported, with compatibility fallback when not.
  • Normalize run metadata from response payload variants (workflow_run_id / workflow_run.*).

• Status comment metadata propagation

  • After dispatch, enrich forwarded aw_context with:
    • dispatched_workflow_name
    • dispatched_run_url
  • Reuse-status-comment update path reads these fields and prefers them when rebuilding comment content.

• Workflow name correction in comment body

  • Extend comment body rendering with an explicit workflow-name override.
  • Ensure reused status comments render the dispatched workflow name rather than inheriting router workflow name.

• Targeted test coverage updates

  • Update slash-command routing tests to assert dispatch endpoint usage and status-comment patch behavior.
  • Add comment rendering/reuse assertions for dispatched workflow/run override fields.

await github.request("POST /repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches", {
  owner, repo, workflow_id, ref, inputs,
  return_run_details: true,
});

aw_context = {
  ...aw_context,
  dispatched_workflow_name: workflowId.replace(/\.lock\.yml$/, ""),
  dispatched_run_url: runUrl,
};

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #42077 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100).

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

[Copilot]

Pull request overview

This PR updates centralized slash-command routing to dispatch workflows via the workflow dispatch REST endpoint (with optional return_run_details handling) and propagates dispatched run/workflow metadata into reusable status comments so the comment links to the dispatched run and shows the dispatched workflow name.

Changes:

• Switch dispatch logic to POST /repos/{owner}/{repo}/actions/workflows/{workflow_id}/dispatches, retrying without return_run_details when unsupported, and normalizing run metadata from response variants.
• Enrich forwarded aw_context with dispatched workflow/run metadata and prefer these fields when rebuilding reusable status comments.
• Extend status comment rendering to support an explicit workflow-name override and add targeted tests for dispatch + comment updates.

Show a summary per file

File	Description
actions/setup/js/route_slash_command.cjs	Updates dispatch to use the workflow dispatch endpoint, parses run metadata, and patches the immediate status comment with dispatched run/workflow info.
actions/setup/js/route_slash_command.test.cjs	Updates routing tests to mock the new dispatch endpoint behavior and assert status comment patching with dispatched run links.
actions/setup/js/add_workflow_run_comment.cjs	Reads dispatched run/workflow overrides from aw_context when updating reusable status comments; adds workflow-name override support to comment body builder.
actions/setup/js/add_workflow_run_comment.test.cjs	Adds tests verifying dispatched metadata overrides and workflow-name override precedence in comment rendering.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Low

[github-actions]

Review: Fix centralized workflow dispatch API usage and status comment attribution

The changes are well-structured and address the stated goals clearly. No blocking issues found.

Summary

• ✅ Dispatch API migration — switching from github.rest.actions.createWorkflowDispatch to github.request with return_run_details is correct, and the GHES compatibility fallback is sensibly guarded.
• ✅ Response normalization — handling both flat and nested workflow_run response shapes is robust; Number.isFinite guards against NaN/0 correctly.
• ✅ Comment attribution — readAwContextString, dispatchedWorkflowName || undefined passthrough, and buildCommentBody override are clean.
• ✅ Test coverage — main paths (dispatch endpoint switch, status comment metadata, workflow name override, disabled workflow) are all exercised.

Non-blocking suggestions

Two inline comments are posted (details there):

1. Missing test for the return_run_details retry path — the GHES compatibility fallback has no unit test; a simple mock covering the 422 → retry → success flow would close that gap.
2. Multi-route last-write-wins behavior — when multiple routes have status_comment: true, sequential updateStatusCommentWithDispatch calls overwrite each other; worth documenting or acknowledging as intentional.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 48.7 AIC · ⌖ 8.96 AIC · ⊞ 4.8K

[github-actions]

The return_run_details compatibility fallback (retrying without that parameter) has no dedicated test coverage. Consider adding a test that mocks github.request to throw a 422 error whose message includes "return_run_details" on the first call, then succeeds on the second — verifying the dispatch still completes and dispatched.dispatched is true.

@copilot please address this.

[github-actions]

When multiple routes in the same slash command have status_comment: true, this loop will call updateStatusCommentWithDispatch once per matching route. Each call overwrites the comment with the latest dispatched workflow's name/URL, so only the last dispatched workflow ends up displayed. This is probably fine for single-route configurations (the common case), but if multi-workflow slash commands with status_comment: true are expected, the behavior should be documented or the last-write-wins semantic should be made explicit.

@copilot please address this.

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 95/100 — Excellent

Analyzed 6 test(s) across 2 JavaScript .test.cjs files: 6 design tests, 0 implementation tests, 0 guideline violations.

📊 Metrics & Test Classification (6 tests analyzed)

Metric	Value
New/modified tests analyzed	6
✅ Design tests (behavioral contracts)	6 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	5 (83%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	0 (Go mock libraries / missing build tags / no assertion messages)

Test	File	Classification	Issues Detected
"uses dispatched workflow metadata from aw_context when provided"	add_workflow_run_comment.test.cjs	✅ Design	Happy-path only (no edge case for partial metadata)
"should prefer explicit workflow name override over environment defaults"	add_workflow_run_comment.test.cjs	✅ Design	—
Existing dispatch test + new statusUpdateCalls assertions	route_slash_command.test.cjs	✅ Design	—
"skips slash routes when target workflow is disabled" (mock updated)	route_slash_command.test.cjs	✅ Design	—
"skips label routes when target workflow is disabled" (mock updated)	route_slash_command.test.cjs	✅ Design	—
"ignores disabled workflow_dispatch failures for disabled label routes" (mock updated)	route_slash_command.test.cjs	✅ Design	—

Go: 0; JavaScript: 6 (*.test.cjs). No other languages detected.

Test inflation check:

• add_workflow_run_comment.test.cjs +40 lines vs add_workflow_run_comment.cjs +22 lines → ratio 1.82:1 ✅
• route_slash_command.test.cjs +36 lines vs route_slash_command.cjs +95 lines → ratio 0.38:1 ✅

Verdict

✅ Check passed. 0% implementation tests (threshold: 30%). All new and modified tests verify observable behavioral contracts — dispatch API output handling, status comment body content, workflow name override priority, and disabled-workflow error paths. The three modified tests in route_slash_command.test.cjs are accurate mock-infrastructure updates matching the production switch from github.rest.actions.createWorkflowDispatch to github.request; their existing behavioral assertions are preserved intact.

Minor observation: "uses dispatched workflow metadata from aw_context when provided" covers only the happy path (both dispatched_workflow_name and dispatched_run_url present in aw_context). A test for partial metadata (e.g. dispatched_run_url absent) would improve resilience coverage, but this is not a blocker given the straightforward fallback logic in the production code.

🧪 Test quality analysis by Test Quality Sentinel · 81.5 AIC · ⌖ 12.2 AIC · ⊞ 8.2K · ◷

[github-actions]

✅ Test Quality Sentinel: 95/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%).

[github-actions]

Skills-Based Review 🧠

Applied /diagnose, /tdd, and /grill-with-docs — requesting changes primarily on test coverage gaps for the GHES fallback path.

📋 Key Themes & Highlights

Key Themes

• GHES fallback untested: The return_run_details retry logic is the most novel runtime behaviour in this PR, but it has zero test coverage. A future change to the error-string check would silently break GHES compatibility.
• Core new behaviour not verified in tests: Tests mock dispatch and check the response is handled, but never assert return_run_details: true was actually sent, making the key intent unverifiable.
• Graceful-degradation path needs a test: When dispatch returns no run_url (GHES without run-details support), the status-comment update is intentionally skipped — but there is no test asserting this.
• Fragile error-string matching: The fallback hinge on a lowercase .includes("return_run_details") match. A GHES version that rewords the error message would silently regress.

Positive Highlights

• ✅ Clean dispatchWorkflow return-type upgrade from boolean to a typed object — well-structured for future extension.
• ✅ readAwContextString helper is a nice defensive primitive; consistent null/type guard pattern.
• ✅ updateStatusCommentWithDispatch swallows errors to core.warning — good non-fatal treatment.
• ✅ Test for dispatched workflow name override in comment body is exactly the right kind of assertion.
• ✅ Two-shape response normalisation (workflow_run_id flat vs workflow_run nested) is clearly documented.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 103.7 AIC · ⌖ 7.75 AIC · ⊞ 6.6K

Comments that could not be inline-anchored

actions/setup/js/route_slash_command.cjs:344

[/diagnose] No test covers the return_run_details GHES compatibility fallback — this is the highest-risk unverified path in the PR.

<details>
<summary>💡 Suggested test skeleton</summary>

it("retries dispatch without return_run_details when GHES rejects it", async () => {
  let callCount = 0;
  globals.github.request = vi.fn(async (route, params) => {
    if (String(route).includes("/dispatches")) {
      callCount++;
      if (callCount === 1) {
        const err = Object.assign(…

</details>

<details><summary>actions/setup/js/route_slash_command.test.cjs:197</summary>

**[/tdd]** The test asserts the API version header is sent, but doesn&#39;t verify `return_run_details: true` is included in the dispatch request — the central new behaviour of this PR.

&lt;details&gt;
&lt;summary&gt;💡 Add the assertion&lt;/summary&gt;

```js
expect(dispatchCalls[0].return_run_details).toBe(true);

Without this, the return_run_details flag could be silently dropped in a refactor and no test would catch it.

</details>

@copilot please address this.

actions/setup/js/route_slash_command.cjs:345

[/diagnose] The GHES fallback trigger relies on message.includes("return_run_details") — a brittle string-match that will silently stop working if GitHub GHES changes its error wording.

<details>
<summary>💡 Consider a more defensive approach</summary>

The current check:

const mentionsReturnRunDetails = typeof message === 'string' && message.toLowerCase().includes('return_run_details');

A minor phrasing change in a GHES release (e.g. "unrecognized parameter" vs *"unknown pa…

actions/setup/js/route_slash_command.test.cjs:251

[/tdd] The test verifies the status comment is updated when run_url is present, but there is no complementary test asserting the update is skipped when dispatched.run_url is undefined — the expected degraded-mode behaviour on GHES without run-details support.

<details>
<summary>💡 What to add</summary>

Add a test where the dispatch mock returns { data: {} } (no run details) and assert that no PATCH /repos/.../issues/comments/... call is made. This guards `updateStatusCommentWi…

actions/setup/js/route_slash_command.cjs:383

[/grill-with-docs] The parameter is named eventName but it receives identifier (the output of eventIdentifier()), which can be "pull_request_comment" — a gh-aw–specific value, not a raw GitHub event name. The two concepts are different and the naming may confuse future maintainers.

<details>
<summary>💡 Suggestion</summary>

Rename the parameter to eventIdentifier to match the call-site variable and the project's own terminology, and update the JSDoc @param accordingly:

…

</details>

<details><summary>actions/setup/js/add_workflow_run_comment.cjs:360</summary>

**[/diagnose]** Double-trim: `readAwContextString` already calls `.trim()` on the returned value, so `workflowNameOverride?.trim()` here is redundant. Harmless, but creates a subtle expectation mismatch — callers passing other (non-aw_context) values still need to ensure they pass trimmed strings, yet the trim in `readAwContextString` only helps aw_context callers.

&lt;details&gt;
&lt;summary&gt;💡 Options&lt;/summary&gt;

1. Remove the `.trim()` in `buildCommentBody` and document that callers are responsible f…

</details>

<details><summary>actions/setup/js/route_slash_command.cjs:302</summary>

**[/tdd]** `firstNonEmptyString` is a small but non-trivial helper with edge-case sensitivity (it treats whitespace-only strings as empty). It has no direct unit tests — it&#39;s exercised only through the end-to-end dispatch path.

&lt;details&gt;
&lt;summary&gt;💡 Suggested unit tests&lt;/summary&gt;

```js
describe(&#39;firstNonEmptyString&#39;, () =&gt; {
  it(&#39;returns the first non-empty string&#39;, () =&gt; {
    expect(firstNonEmptyString([null, undefined, &#39;&#39;, &#39;  &#39;, &#39;valid&#39;])).toBe(&#39;valid&#39;);
  });
  it(&#39;returns empty string w…

</details>

[github-actions]

REQUEST_CHANGES — The dispatch and metadata-propagation logic has correctness gaps that should be addressed before merging.

### Blocking issues (5)

High

• Fan-out loop overwrites status comment (route_slash_command.cjs:760): updateStatusCommentWithDispatch is called inside the route loop; when multiple routes both maintain a status comment, each dispatch stomps the previous one and only the last dispatch's metadata survives.

Medium

• Fragile return_run_details fallback (route_slash_command.cjs:344): The GHES compatibility retry only fires when the error message literally contains "return_run_details". Alternative error phrasings from different GHES versions convert the fallback into a hard failure. Widening the retry condition to any 400/422 is safer.
• Silent no-op on old GHES (route_slash_command.cjs:387): When the server returns no run URL (GHES fallback path), updateStatusCommentWithDispatch returns early without any core.info/core.warning. The status comment is silently left unpatched with no diagnostic signal.
• Workflow-ID marker/name desync (add_workflow_run_comment.cjs:361): workflowName is overridden to the dispatched workflow's name for user-visible text, but generateWorkflowIdMarker still uses process.env.GITHUB_WORKFLOW (the router). The dispatched workflow's own add_workflow_run_comment step will fail to find the existing comment by marker and create a duplicate.
• No test for the return_run_details fallback retry (route_slash_command.test.cjs:251): The inner try/catch and retry-without-parameter path is entirely untested. A regression would go undetected until GHES production failures.

🔎 Code quality review by PR Code Quality Reviewer · 108.7 AIC · ⌖ 7.66 AIC · ⊞ 5.2K

[github-actions]

Status comment stomped on each loop iteration in fan-out scenarios: updateStatusCommentWithDispatch is called inside the routes loop, so when multiple routes both maintainsStatusComment, every successful dispatch overwrites the same comment — the last writer wins and all earlier dispatch metadata is silently lost.

💡 Impact and suggested fix

In the existing test creates an immediate status comment once and forwards it in aw_context, two routes match /archie (both status_comment: true). Both dispatches succeed, so updateStatusCommentWithDispatch is called twice with the same status_comment_id. The test only asserts statusUpdateCalls[0], so the second overwrite goes unobserved. In production, the comment would show only the last dispatched workflow's run link.

Options:

1. Collect { workflowId, runUrl } per-route and perform a single comment update after the loop.
2. Skip the router-side update for multi-route fans and let each dispatched workflow's own add_workflow_run_comment step handle attribution.
3. At minimum, tighten the assertion: verify the final comment state after both dispatches, not just statusUpdateCalls[0].

[github-actions]

return_run_details detection relies on a fragile substring match that will silently hard-fail dispatch on GHES with different error phrasing: the inner catch only retries when the error is both a 400/422 status AND its message contains the literal string "return_run_details". If a GHES version rejects the parameter with a different message (e.g., "unexpected_input", "invalid field", etc.), mentionsReturnRunDetails is false, the inner catch re-throws, and the outer catch turns it into a hard Failed to dispatch workflow failure instead of the intended silent retry.

💡 Suggested fix

Retry on all 400/422 errors — the worst case from a false positive is an unnecessary second request. Since return_run_details is the only added field and valid GHES endpoints won't reject a vanilla dispatch, retrying without it on any 4xx is safe:

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.

[github-actions]

Status comment is silently left stale when the server returns no run URL (old GHES): when return_run_details is rejected and the retry response contains no run metadata, dispatched.run_url is undefined, and this guard returns early with no core.warning. The status comment created by addImmediateStatusComment() is never updated with the dispatched workflow name — the PR's primary goal — and there is no observable signal that the enrichment was skipped.

💡 Suggested improvement

Add 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 runUrl is unavailable, rather than skipping the update entirely.

[github-actions]

Visible workflow name and hidden workflow-ID marker are now out of sync: workflowName (user-visible) correctly uses workflowNameOverride, but the generateWorkflowIdMarker call a few lines below still uses process.env.GITHUB_WORKFLOW (the router's name). When the dispatched workflow archie.lock.yml later runs its own add_workflow_run_comment step, it searches for an existing status comment tagged with marker archie — 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:

const workflowId = workflowNameOverride || process.env.GH_AW_WORKFLOW_NAME || process.env.GITHUB_WORKFLOW || "";
if (workflowId) {
  body += `\n\n${generateWorkflowIdMarker(workflowId)}`;
}

Alternatively, keep the router marker intentionally and document that the reuse contract relies on status_comment_id rather than marker matching — but the current code makes this implicit and fragile.

[github-actions]

No test covers the return_run_details fallback retry path: the new GHES compatibility code (inner try/catch, retry without the parameter) has zero coverage. This is the critical path for older GHES compatibility and it's entirely untested.

💡 Missing test case

Add a test that:

1. Mocks github.request to throw { status: 422, response: { data: { message: "Unsupported parameter return_run_details" } } } on the first dispatch call.
2. Returns { data: {} } on the second dispatch call.
3. Asserts that dispatchCalls has exactly 2 entries, the second of which has no return_run_details field.
4. Asserts that the dispatch is still reported as successful (no setFailed).
5. Asserts that updateStatusCommentWithDispatch is NOT called (since the retry response has no run URL).

Without this test, a refactor that breaks the fallback will go undetected until a production GHES deployment fails.

Done

[pelikhan]

@copilot run pr-finisher skill

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.82.0