test: cover routing for a provider literally named "oauth"#119
Open
heskew wants to merge 1 commit into
Open
Conversation
Add an integration test asserting /oauth/oauth/login dispatches to a provider whose configured name is literally "oauth" (302 to its authorizationUrl with its client_id). Harper strips the mount segment, so parseRoute sees target.id = "oauth/login" and must resolve providerName "oauth"; any change that detects-and-omits a leading "oauth" path segment would instead resolve "login" and make the provider unreachable. The existing oac-oauth-tenant case doesn't exercise this (its first path segment isn't "oauth"), so this new assertion is the one that fails under such a change. Reuses the existing path-segment-routing-app fixture; no fixture changes needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Reviewed; no blockers found. |
Contributor
|
Reviewed; no blockers found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an integration test to
integrationTests/path-segment-routing.test.tscovering a routing case the existing suite missed: a provider whose configured name is literallyoauthmust be reachable at/oauth/oauth/login.Why
Harper strips the resource mount segment before the OAuth resource runs, so
parseRoutereceivestarget.id = "oauth/login"and must resolveproviderName = "oauth". The existing suite only assertsoac-oauth-tenant(whose first path segment isn'toauth), so it does not exercise this case.This came out of reviewing #101 ("detect and omit oauth prefix from path parts"), which strips a leading
oauthsegment inparseRoute. Verified empirically against real Harper:GET /oauth/oauth/login→302to theoauth-named provider'sauthorizationUrl✅GET /oauth/oauth/login→200, provider unreachable ❌ (providerNamebecomeslogin)So the prefix-strip isn't a harmless no-op — it breaks any provider legitimately named
oauth. This test is the regression guard for that invariant: green onmain, red under such a change.Test
Reuses the existing
path-segment-routing-appfixture (which already configures anoauth-named decoy provider) — no fixture changes. Boots real Harper and asserts the302+authorizationUrlorigin/path +client_id.🤖 Generated with Claude Code