Skip to content

Improve OPTIONS responses across endpoints#1038

Merged
jviotti merged 3 commits into
mainfrom
better-options
Jun 8, 2026
Merged

Improve OPTIONS responses across endpoints#1038
jviotti merged 3 commits into
mainfrom
better-options

Conversation

@jviotti

@jviotti jviotti commented Jun 8, 2026

Copy link
Copy Markdown
Member

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@augmentcode

augmentcode Bot commented Jun 8, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR standardizes how the server answers OPTIONS (CORS preflight) requests across multiple read-only endpoints, returning an RFC/Fetch-friendly 204 response shape.

Changes:

  • Added a reusable cors_preflight() helper to emit 204 + Access-Control-Allow-* + Allow headers.
  • Updated several read-only actions to short-circuit OPTIONS via cors_preflight() (default, health, schema search, dependency tree, schema serving, directory listing, artifact/static serving).
  • Expanded Allow headers in 405 responses to include OPTIONS where these endpoints now support it.
  • Adjusted existing Hurl E2E suites to expect 204 preflight responses (instead of 404/405) and updated asserted headers accordingly.
  • Added dedicated preflight.all.hurl test files (headless/html/path) to lock in the preflight response contract per surface.

Technical Notes: Preflight responses are uncacheable (Cache-Control: no-store) and include explicit per-surface allowed methods/headers to keep each action’s CORS contract explicit.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread src/router/artifact.cc
"sourcemeta:one/method-not-allowed",
"This HTTP method is invalid for this URL", error_schema,
enable_cors ? "*" : "", "GET, HEAD");
enable_cors ? "*" : "", "GET, HEAD, OPTIONS");

@augmentcode augmentcode Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RouterAction::artifact_serve still treats options as method-not-allowed (if method != get && != head), but the 405 response now advertises Allow: GET, HEAD, OPTIONS. If an OPTIONS request ever reaches this helper (e.g., a caller forgets the early preflight branch), the response would be internally inconsistent (405 while claiming OPTIONS is allowed).

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 43 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/router/artifact.cc
Comment thread src/actions/action_serve_schema_artifact_v1.h Outdated
jviotti added 2 commits June 8, 2026 13:13
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

@github-actions github-actions Bot left a comment

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.

Benchmark Index (community)

Details
Benchmark suite Current: fe1a200 Previous: ee8d793 Ratio
Add one schema (0 existing) 388 ms 401 ms 0.97
Add one schema (100 existing) 34 ms 30 ms 1.13
Add one schema (1000 existing) 99 ms 96 ms 1.03
Add one schema (10000 existing) 773 ms 759 ms 1.02
Update one schema (1 existing) 24 ms 22 ms 1.09
Update one schema (101 existing) 36 ms 31 ms 1.16
Update one schema (1001 existing) 99 ms 96 ms 1.03
Update one schema (10001 existing) 815 ms 790 ms 1.03
Cached rebuild (1 existing) 7 ms 7 ms 1
Cached rebuild (101 existing) 10 ms 9 ms 1.11
Cached rebuild (1001 existing) 34 ms 33 ms 1.03
Cached rebuild (10001 existing) 291 ms 279 ms 1.04
Index 100 schemas 569 ms 486 ms 1.17
Index 1000 schemas 1595 ms 1458 ms 1.09
Index 10000 schemas 13932 ms 14012 ms 0.99
Index 10000 schemas (custom meta-schema) 134520 ms 136047 ms 0.99

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions github-actions Bot left a comment

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.

Benchmark Index (enterprise)

Details
Benchmark suite Current: fe1a200 Previous: ee8d793 Ratio
Add one schema (0 existing) 381 ms 376 ms 1.01
Add one schema (100 existing) 33 ms 33 ms 1
Add one schema (1000 existing) 88 ms 94 ms 0.94
Add one schema (10000 existing) 958 ms 850 ms 1.13
Update one schema (1 existing) 25 ms 25 ms 1
Update one schema (101 existing) 31 ms 33 ms 0.94
Update one schema (1001 existing) 90 ms 94 ms 0.96
Update one schema (10001 existing) 710 ms 763 ms 0.93
Cached rebuild (1 existing) 7 ms 8 ms 0.88
Cached rebuild (101 existing) 10 ms 11 ms 0.91
Cached rebuild (1001 existing) 30 ms 34 ms 0.88
Cached rebuild (10001 existing) 244 ms 277 ms 0.88
Index 100 schemas 664 ms 676 ms 0.98
Index 1000 schemas 1608 ms 1547 ms 1.04
Index 10000 schemas 13476 ms 13667 ms 0.99
Index 10000 schemas (custom meta-schema) 137395 ms 132022 ms 1.04

This comment was automatically generated by workflow using github-action-benchmark.

@jviotti jviotti merged commit 1374bf6 into main Jun 8, 2026
5 checks passed
@jviotti jviotti deleted the better-options branch June 8, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant