Skip to content

docs(api): say what Config.Fallback receives after unmatchedAPIRoute - #673

Merged
ThomasK33 merged 3 commits into
mainfrom
issue352-fallback-comment
Nov 4, 2026
Merged

ThomasK33 merged 3 commits into
mainfrom
issue352-fallback-comment

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

The api.Config.Fallback comment now says what the field receives: every request outside /api/. This is a follow-up from the merge advisor on #672.

Why

The comment said Fallback "receives every request not matching an /api/v1 route" and that "the serve command mounts the git smart-HTTP handler there". Both statements are wrong:

  1. Since feat(api)!: send route_not_found and authentication_unavailable #672, unmatchedAPIRoute is registered for /api/ (internal/api/api.go), so an unmatched /api/ path gets its problem body and never reaches Fallback.
  2. The serve command does not set Fallback. It mounts the API handler at /api/ in its own mux, next to the git smart-HTTP handler (cmd/openstorage/main.go).

Change

Comment only, no behavior change.

Validation

go build ./internal/api, golangci-lint run ./internal/api/... (0 issues), and go test ./internal/api -run 'Fallback|Unmatched' pass.

Refs #352


Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

The #672 advisor found the Fallback comment stale: since #672, an
unmatched /api/ path gets unmatchedAPIRoute's problem body, so Fallback
sees only paths outside /api/. The serve command never set Fallback.

_Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high_

Change-Id: Ib929048a31e6ab93da7c7983da9da5165069c78f
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T22:06:37.521224Z 49ba0bc Manual request
🔒 Security Review ✅ Completed 2026-11-04T22:07:01.535495Z 49ba0bc Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c96395c671

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/api/api.go Outdated
Comment on lines +35 to +36
// Fallback (optional) receives every request outside /api/: an unmatched
// /api/ path gets unmatchedAPIRoute's problem body. The serve command does

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Account for ServeMux redirects in the fallback contract

When a caller sets Fallback and requests /api (or an unclean path such as /x/../api/foo), http.ServeMux performs its trailing-slash or path-cleaning redirect before selecting the / handler, so Fallback never receives that request. The repository already identifies /api as this redirect case in cmd/openstorage/metrics_surface_test.go:20; therefore “every request outside /api/” remains inaccurate and should be limited to clean, non-redirected paths.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: c96395c671

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Codex round 1 on #673: ServeMux redirects /api and unclean paths before
it picks the / handler, so Fallback never sees them.

Change-Id: I81dd6477fb76312cd249b5e92c950935b59206ce
Signed-off-by: Thomas Kosiewski <tk@coder.com>
Change-Id: I847fec0f8c1aa34e8f9c864d1deffcfcfbdf2205
Signed-off-by: Thomas Kosiewski <tk@coder.com>
@ThomasK33

Copy link
Copy Markdown
Member Author

Codex round 1 had one P2 finding (medium severity): ServeMux redirects /api and unclean paths before it picks the / handler, so Fallback never sees them. 83db336 says so in the comment, and 49ba0bc rewraps it (new commits, no force-push). Comment only; go build ./internal/api and gofmt pass.

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 49ba0bc4d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 49ba0bc4d9

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@ThomasK33
ThomasK33 added this pull request to the merge queue Nov 4, 2026
Merged via the queue into main with commit 32ca3ea Nov 4, 2026
9 checks passed
@ThomasK33
ThomasK33 deleted the issue352-fallback-comment branch November 4, 2026 22:52
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