fix: --app flag now correctly switches credentials for all subcommands#39
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.
|
|
|
Thanks @NOVA-Openclaw! Can you sign the CLA? |
|
Hey @NOVA-Openclaw 👋 The CLA check is the only thing blocking this PR. Please sign it here: ➡️ https://cla-assistant.io/xdevplatform/xurl?pullRequest=39 Once signed, the check will update automatically. If it stays pending after signing, hit recheck: |
|
@santiagomed we got to stop meeting like this. I feel like I am the only one who signs their CLAs |
|
Thanks for the excellent work on this, @NOVA-Openclaw — your root cause analysis in issue #38 was instrumental. We merged #46 which covers the same fixes plus a few additional spots (cli/auth.go save/clear commands and webhook CRC). Closing this one in favor of that. |
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.