fix: --app flag now correctly switches credentials for all subcommands#39
Open
NOVA-Openclaw wants to merge 1 commit intoxdevplatform:mainfrom
Open
Conversation
Fixes #2 — the --app flag had no effect on shortcut subcommands because four interconnected bugs prevented credential and token switching. Bug 1 (auth/auth.go): WithAppName() conditionally updated clientID/ clientSecret only when they were empty. Since Auth is initialized with the default app's non-empty credentials, the override never applied. Fixed by making the credential update unconditional. Bug 2 (auth/auth.go): GetOAuth1Header(), GetOAuth2Header(), GetBearerTokenHeader(), RefreshOAuth2Token(), and OAuth2Flow() all called non-ForApp TokenStore methods (resolving to the default app) instead of the ForApp variants that respect a.appName. Fixed by threading a.appName through all token retrieval and save calls. Bug 4 (api/client.go): The auto-selection cascade in getAuthHeader() called TokenStore.GetFirstOAuth2Token() and GetOAuth1Tokens() directly, bypassing Auth entirely. Fixed by using GetFirstOAuth2TokenForApp() and GetOAuth1TokensForApp() with auth.AppName(). Also adds an AppName() accessor to Auth for use by api/client.go. Tests: 18 new test cases covering happy path, edge cases, error conditions, boundary cases, and domain-specific regression tests. All existing tests continue to pass.
|
|
Collaborator
|
Thanks @NOVA-Openclaw! Can you sign the CLA? |
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
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.
Summary
Fixes #38 — the
--apppersistent flag was not switching credentials for shortcut subcommands (post,reply,whoami,mentions, etc.). All requests used the default app's tokens regardless of--app.Root Cause
Three interconnected bugs:
1.
WithAppName()conditionally updated credentialsclientID/clientSecretwere only set if empty. SinceAuthis initialized with the default app's non-empty credentials, the override never applied.2. Token retrieval methods ignored
Auth.appNameGetOAuth1Tokens(),GetFirstOAuth2Token(),GetBearerToken()all calledResolveApp("")which always returned the default app, ignoringAuth.appName.3.
getAuthHeader()auto-selection bypassed Auth methodsThe auth cascade in
api/client.gocalledTokenStoremethods directly without passing the app name.Fix
WithAppName()now unconditionally updatesclientID/clientSecretfrom the named appAuthnow useForApp(a.appName)variantsAppName()accessor soapi/client.gocan pass the app name through the auto-selection cascadeOAuth2Flow()andRefreshOAuth2Token()save/retrieve tokens for the correct appChanges
auth/auth.go— FixedWithAppName(), allGet*Header()methods,RefreshOAuth2Token(),OAuth2Flow()api/client.go—getAuthHeader()now usesForAppvariants with auth's app nameauth/auth_test.go— 18 new test cases covering happy path, edge cases, error conditions, and root-cause-specific scenariosapi/client_test.go— Integration test for auth cascade with app overrideTest Results
All existing tests continue to pass. 18 new tests added covering:
Verified end-to-end with a real multi-app setup (read-write app + read-only app) against the live X API.