🔑 Fix double key set in interactive API key collection

[dsyme]

Summary

• Prevent duplicate secret uploads by updating existingSecrets after key upload
• Added error handling for CheckAndCollectEngineSecrets function
• Log secret update to improve tracking of secret management

Details

The changes ensure that when an API key is collected interactively, the existingSecrets map is updated to reflect the new secret. This prevents potential duplicate secret uploads in subsequent steps of the process.

[Copilot]

Pull request overview

Updates the interactive “add” flow to avoid re-uploading engine API-key secrets by recording newly ensured secrets in the in-memory existingSecrets map after interactive secret collection.

Changes:

• Adds explicit error handling around CheckAndCollectEngineSecrets in the interactive engine key collection path.
• Updates existingSecrets after secret collection to prevent duplicate secret uploads later in the interactive flow.
• Adds debug logging for the existingSecrets update.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

existingSecrets is documented/used as “secrets that already exist in the repository”, but this code unconditionally sets opt.SecretName to true after CheckAndCollectEngineSecrets returns nil. CheckAndCollectEngineSecrets can also succeed because an alternative secret already exists in the repo (e.g., CLAUDE_CODE_OAUTH_TOKEN), in which case opt.SecretName may still not exist—this mutates existingSecrets into an incorrect state and makes the “after upload” comment/log misleading. Consider only setting opt.SecretName when it was actually uploaded/ensured in-repo (e.g., skip the update when any opt.AlternativeSecrets are already present in c.existingSecrets, or re-check repo secrets after the operation), and adjust the comment/log wording accordingly.

Suggested change

	// Update existingSecrets to reflect that the secret was uploaded
	// This prevents duplicate secret uploads in applyChanges later
	opt := constants.GetEngineOption(engine)
	if opt != nil {
	c.existingSecrets[opt.SecretName] = true
	addInteractiveLog.Printf("Updated existingSecrets to include %s after upload", opt.SecretName)
	// Update existingSecrets to reflect that the engine's secret is ensured in the repository
	// This helps prevent duplicate secret uploads in applyChanges later
	opt := constants.GetEngineOption(engine)
	if opt != nil {
	// If the primary secret is already marked as existing (for example, by CheckAndCollectEngineSecrets),
	// we don't need to change anything.
	if c.existingSecrets[opt.SecretName] {
	addInteractiveLog.Printf("Secret %s already marked as existing; no update needed", opt.SecretName)
	} else {
	// If any alternative secrets are already present, success may have been due to them.
	// In that case, do not incorrectly mark the primary secret as existing.
	alternativePresent := false
	for _, alt := range opt.AlternativeSecrets {
	if c.existingSecrets[alt] {
	alternativePresent = true
	break
	}
	}
	if alternativePresent {
	addInteractiveLog.Printf("Not marking %s as existing because an alternative secret is already present", opt.SecretName)
	} else {
	// No existing primary or alternative secrets recorded; assume we just ensured the primary secret.
	c.existingSecrets[opt.SecretName] = true
	addInteractiveLog.Printf("Updated existingSecrets to include %s after ensuring secret in repository", opt.SecretName)
	}
	}