fix(invoke): prevent crash in getTokenInfo/getAuthorizedUser functions

[JDetmar]

Summary

• Fix invoke functions (getTokenInfo, getAuthorizedUser) crashing with gRPC connection closing error
• Add safeGetConfigToken() helper that wraps infer.GetConfig with recover() to handle panics
• Refactor GetHTTPClient() to check environment variable first (safe path), then fall back to config
• Add 7 unit tests covering the fix

Root Cause

The infer.GetConfig[*Config](ctx) function panics (not returns nil) when provider config is not available in the context. For invoke functions, which may be called before Configure() completes asynchronously, the config might not be injected into the context yet.

The previous code incorrectly assumed GetConfig returns nil when config is unavailable:

config := infer.GetConfig[*Config](ctx)  // PANICS!
if config != nil { ... }  // Never reached

Test plan

• All 7 new unit tests pass
• All 418 existing provider tests pass
• Linter passes with 0 issues
• Codegen verified (no schema changes needed - internal refactor)
• Manual test: Run getTokenInfo invoke function with WEBFLOW_API_TOKEN env var set

🤖 Generated with Claude Code

[github-actions]

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

[Copilot]

Pull request overview

This PR fixes a crash in the getTokenInfo and getAuthorizedUser invoke functions by adding panic recovery when accessing provider configuration. The issue occurred because infer.GetConfig panics (rather than returning an error) when called in contexts where provider configuration hasn't been initialized yet, such as during invoke function calls before Configure() completes asynchronously.

Changes:

• Added safeGetConfigToken() helper function with panic recovery to safely access provider config
• Refactored GetHTTPClient() to check environment variable first (safe path), then fall back to config with panic recovery
• Added 7 comprehensive unit tests covering the fix and various edge cases

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
provider/config.go	Core fix: adds safeGetConfigToken() with panic recovery and refactors GetHTTPClient() to prioritize env var over config
provider/config_test.go	New test file with 7 unit tests validating the fix, including empty context, nil context, env var handling, and panic prevention
ISSUES-TO-FIX.md	Updates issue #3 status from "NOT YET INVESTIGATED" to "FIXED" with root cause and solution documentation

[Copilot]

This change reverses the precedence order of token sources. Previously, provider config took precedence over the environment variable. Now the environment variable takes precedence over provider config.

This is a breaking change in behavior: if a user has both pulumi config set webflow:apiToken AND WEBFLOW_API_TOKEN environment variable set, the environment variable will now be used instead of the config value.

While this change makes sense for fixing the panic issue and aligns with common practice (env vars often take precedence), it should be documented as a breaking change or the precedence should be maintained by swapping the order back (checking config first with safe recovery, then falling back to env var).

[Copilot]

This test is named "EnvVarTakesPrecedence" but doesn't actually test precedence when both config and env var are present. It only tests that env var works when config is unavailable (context.Background()).

To properly test precedence, you would need to:

1. Create a context with provider config containing one token value
2. Set WEBFLOW_API_TOKEN environment variable to a different value
3. Verify that the environment variable value is used (i.e., takes precedence)

The current test is still valuable for verifying the panic fix, but the name is misleading. Consider renaming it to something like "TestGetHTTPClient_EnvVarWorksWithoutConfig" or adding a proper precedence test.

[Copilot]

The panic recovery here will catch ALL panics that occur within this function, not just panics from infer.GetConfig. This means if there's a programming error elsewhere in this function (like a nil pointer dereference), it will be silently converted to an empty token return, making debugging harder.

Consider adding logging or being more specific about what panics are expected. For example, you could check the recovered value and only handle expected panic types, re-panicking for unexpected ones.

[Copilot]

Passing nil context violates the Go context best practices. While the test includes a nolint directive, testing with nil context may not accurately represent real-world behavior. The comment suggests this is an edge case that "shouldn't happen in practice" - if it truly shouldn't happen, consider removing this test case entirely, as it adds test complexity without value.

If you want to keep it for defensive programming, the current implementation is adequate with the staticcheck suppression.

Suggested change

	func TestSafeGetConfigToken_NilContext(t *testing.T) {
	// Test with nil context - should not panic
	// Note: This is an edge case that shouldn't happen in practice
	defer func() {
	if r := recover(); r != nil {
	t.Errorf("safeGetConfigToken panicked with nil context: %v", r)
	}
	}()
	//nolint:staticcheck // SA1012: intentionally passing nil context to test panic recovery
	token := safeGetConfigToken(nil)
	assert.Equal(t, "", token)
	}