Fix upstream Github response status change for creating a workflow dispatch event

[alecrajeev]

Issue

Github recently updated their response code detailed here #317:

Fetched Workflow ID: 118145225
Error: dispatchWorkflow: An unexpected error has occurred: Failed to dispatch action, expected 204 but received 200
Error: Failed: An unhandled error has occurred: Failed to dispatch action, expected 204 but received 200
Error: Failed: An unhandled error has occurred: Failed to dispatch action, expected 204 but received 200
Error: Error: failed to run script step: command terminated with non-zero exit code: error executing command [sh -e /__w/_temp/8ad9f6b0-dc75-11f0-8216-d9d1b2e5bc1a.sh], exit code 1
Error: Process completed with exit code 1.
Error: Executing the custom container implementation failed. Please contact your self hosted runner administrator.

Fix

Update the allowed response codes from github.

I tested this with an internal repo and it was successful.

Summary by CodeRabbit

• Bug Fixes

  • Broadened workflow dispatch success validation to accept multiple HTTP success responses, reducing false failure reports so some previously flagged dispatches now pass.

• Tests

  • Updated and parameterized tests and snapshots to reflect the accepted success statuses and adjusted success logging expectations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Relaxed WorkflowDispatch response validation to treat HTTP 200 or 204 as success; tests and error message updated to accept and reflect either status.

Changes

Cohort / File(s)	Summary
Workflow Dispatch handling src/api.ts, src/api.spec.ts	src/api.ts: treat HTTP 200 or 204 as successful WorkflowDispatch responses and update the error message to mention both codes. src/api.spec.ts: update tests, parameterize status cases (200, 204), and adjust snapshots/log assertions to match the revised behavior and messages.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Check the conditional in src/api.ts correctly accepts 200 and 204 and that the error message text matches.
• Verify src/api.spec.ts parameterized tests correctly mock both statuses and assert logging/snapshots for both outcomes.

Poem

🐰 I hopped in code with nimble paws,
Opened the gate for two small laws.
200 nods, 204 cheers,
Together they whisper — dispatch clears. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: relaxing the workflow dispatch response check to accept HTTP 200 in addition to 204, which aligns with GitHub's upstream response status change.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad7463a and 4799d52.

📒 Files selected for processing (1)

• src/api.spec.ts (2 hunks)

🔇 Additional comments (3)

src/api.spec.ts (3)

148-148: Test name accurately reflects the updated API behavior.

The test name correctly indicates that both 200 and 204 are acceptable status codes for successful workflow dispatch.

---

163-163: Error messages correctly updated for dual status code acceptance.

The error message and inline snapshot properly reflect that both HTTP 200 and 204 are acceptable responses from the GitHub API for workflow dispatch events.

Also applies to: 170-170

---

175-209: Excellent use of parameterized testing for dual status code support.

The conversion to it.each([200, 204]) systematically verifies that both HTTP status codes are handled correctly. The change from toMatchInlineSnapshot to toMatch (lines 200-207) is appropriate for parameterized tests since both status codes produce identical log output.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[alecrajeev]

@Codex- I think I have a fix in-place for the bug

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.72%. Comparing base (f080a7b) to head (4799d52).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #318   +/-   ##
=======================================
  Coverage   91.72%   91.72%           
=======================================
  Files           7        7           
  Lines         266      266           
  Branches       66       66           
=======================================
  Hits          244      244           
  Misses          3        3           
  Partials       19       19           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/api.spec.ts (1)

148-173: Update test name to reflect dual status code acceptance.

The error messages on lines 163 and 170 correctly reflect that both 200 and 204 are now valid responses. However, the test name "should throw if a non-204 status is returned" is misleading since both 200 and 204 are now accepted.

🔎 Suggested fix for test name

-    it("should throw if a non-204 status is returned", async () => {
+    it("should throw if a non-200/204 status is returned", async () => {

🧹 Nitpick comments (1)

src/api.spec.ts (1)

120-146: Add test coverage for HTTP 200 success response.

Since the PR addresses GitHub's change to return HTTP 200 instead of 204, consider adding a test case that verifies HTTP 200 is accepted as a successful response alongside the existing 204 test.

🔎 Suggested additional test case

Add this test after the existing "should resolve after a successful dispatch" test:

it("should resolve after a successful dispatch with 200 status", async () => {
  vi.spyOn(
    mockOctokit.rest.actions,
    "createWorkflowDispatch",
  ).mockReturnValue(
    Promise.resolve({
      data: undefined,
      status: 200,
      headers: {},
    }),
  );

  // Behaviour
  await expect(dispatchWorkflow("")).resolves.not.toThrow();

  // Logging
  assertOnlyCalled(coreInfoLogMock);
  expect(coreInfoLogMock).toHaveBeenCalledOnce();
  expect(coreInfoLogMock.mock.calls[0]?.[0]).toMatchInlineSnapshot(`
    "Successfully dispatched workflow:
      Repository: owner/repo
      Branch: ref
      Workflow: workflow
      Workflow Inputs: {"testInput":"test"}
      Distinct ID: "
  `);
});

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7a798ab and b910788.

📒 Files selected for processing (1)

• src/api.spec.ts (1 hunks)

[Codex-]

Thanks!