[terminalchai]

Problem

Fixes #1200.

RFC 9110 §15.5.6 requires that a 405 Method Not Allowed response MUST include an Allow header field listing the methods that are permitted for the target resource.

The current assertMethod implementation throws a bare HTTPError({ status: 405 }) with no Allow header, making the response technically non-compliant and unhelpful to clients.

Fix

Normalize the expected argument to an array, add "HEAD" if allowHead is true, then pass the resulting method list as the Allow header to HTTPError:

// Before
throw new HTTPError({ status: 405 });

// After
const allowedMethods = new Set(Array.isArray(expected) ? expected : [expected]);
if (allowHead) {
  allowedMethods.add("HEAD");
}
throw new HTTPError({
  status: 405,
  headers: { Allow: [...allowedMethods].join(", ") },
});

This directly follows the approach suggested by @sandros94 in the issue thread.

Tests

• Updated the existing assertMethod test to assert Allow: POST, HEAD on the 405 response.
• Added a new test for the array-of-methods case (["GET", "POST"]) asserting Allow: GET, POST.

Verification

• vitest run — 943/943 tests ✅
• oxlint — 0 warnings, 0 errors ✅
• oxfmt --check — clean ✅

Summary by CodeRabbit

• Bug Fixes
  • HTTP 405 (Method Not Allowed) responses now include an Allow header listing all permitted methods; HEAD is included when applicable.
• Tests
  • Tests updated to assert 405 responses include the exact set of allowed methods for single and multi-method scenarios.
• Documentation
  • Docs updated to note 405 responses now include an Allow header per RFC 9110.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d20ec214-9c97-4b05-8fa2-0bc4456ae674

📥 Commits

Reviewing files that changed from the base of the PR and between 77ede82 and 9711854.

📒 Files selected for processing (2)

• docs/2.utils/1.request.md
• src/utils/request.ts

---

📝 Walkthrough

Walkthrough

assertMethod now builds a 405 HTTPError that includes an Allow response header listing permitted methods (adds HEAD when allowHead is truthy). Tests and docs were updated to assert and document the new Allow header behavior.

Changes

Cohort / File(s)	Summary
Allow header behavior src/utils/request.ts	When isMethod fails, construct a 405 HTTPError with an Allow response header enumerating normalized expected methods and optionally HEAD. No public API signature changes.
Tests updated test/utils.test.ts	Updated assertMethod tests to capture 405 responses and assert the Allow header contains the exact permitted methods for single (POST) and multi (GET, POST) cases.
Docs docs/2.utils/1.request.md	Documentation updated to state that 405 responses include an Allow header listing permitted methods per RFC 9110.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(request): include Allow header in 405 response #1314 — Updates assertMethod to include an Allow header on 405 responses and adjusts tests accordingly.
• fix(cors): preserve CORS headers on error responses #1352 — Ensures error responses preserve/merge headers (relevant for propagating Allow on thrown HTTPError).

Suggested reviewers

• pi0

Poem

I nibble bits of HTTP hay,
I add an Allow to point the way,
405s now plainly say,
Which methods may come play,
Hop, patch, ship — hooray! 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The PR fully addresses issue #1200 by implementing Allow header inclusion on assertMethod 405 responses with proper method enumeration logic and RFC 9110 compliance.
Out of Scope Changes check	✅ Passed	All changes directly support the core objective: updating assertMethod implementation and adding comprehensive test coverage for the Allow header functionality.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title accurately and specifically describes the main change: adding an Allow header to 405 responses from the assertMethod function.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[coderabbitai]

🧹 Nitpick comments (1)

test/utils.test.ts (1)

304-304: Avoid hard-coding Allow header ordering in the assertions.

Allow is order-insensitive, so exact string matches here will fail on harmless internal changes like sorting or deduping. Comparing the parsed method set would keep the coverage while making the tests less brittle.

♻️ Suggested test update

-      expect(res405.headers.get("Allow")).toBe("POST, HEAD");
+      expect(new Set(res405.headers.get("Allow")?.split(/\s*,\s*/))).toEqual(
+        new Set(["POST", "HEAD"]),
+      );
...
-      expect(res405.headers.get("Allow")).toBe("GET, POST");
+      expect(new Set(res405.headers.get("Allow")?.split(/\s*,\s*/))).toEqual(
+        new Set(["GET", "POST"]),
+      );

Also applies to: 316-316

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/utils.test.ts` at line 304, The assertion currently hard-codes the exact
string ordering for the Allow header
(expect(res405.headers.get("Allow")).toBe("POST, HEAD")), which is brittle;
instead retrieve the Allow header from res405 (and the other similar assertion),
split on commas, trim each method, and compare as a set/array-insensitive
collection to the expected methods (e.g., compare to a Set or sort both arrays)
so the test asserts membership rather than ordering; update the assertions
referencing res405.headers.get("Allow") accordingly in test/utils.test.ts.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/utils.test.ts`:
- Line 304: The assertion currently hard-codes the exact string ordering for the
Allow header (expect(res405.headers.get("Allow")).toBe("POST, HEAD")), which is
brittle; instead retrieve the Allow header from res405 (and the other similar
assertion), split on commas, trim each method, and compare as a
set/array-insensitive collection to the expected methods (e.g., compare to a Set
or sort both arrays) so the test asserts membership rather than ordering; update
the assertions referencing res405.headers.get("Allow") accordingly in
test/utils.test.ts.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0fd93c2d-4a5e-4141-b8a9-a9ff5cb287ae

📥 Commits

Reviewing files that changed from the base of the PR and between c28356e and bd1c97f.

📒 Files selected for processing (2)

• src/utils/request.ts
• test/utils.test.ts

[pi0]

thnx!