fix: detect stale PEM when GitHub App is deleted and recreated

internal/appsetup/appsetup.go

@@ -19,6 +19,7 @@ import (
 	"os/exec"
 	"regexp"
 	"runtime"
+	"strconv"
 	"strings"
 	"time"
 
@@ -129,6 +130,7 @@ type Setup struct {
 	permErrors   []string
 	publicApps   bool
 	appSet       string
+	storedAppIDs map[string]string // org/role → app_id from ROLE_APP_IDS
 }
 
 // NewSetup creates a new Setup instance.
@@ -170,6 +172,13 @@ func (s *Setup) WithPublicApps(public bool) *Setup {
 	return s
 }
 
+// WithStoredAppIDs sets the stored ROLE_APP_IDS mapping (org/role → app_id)
+// used to detect stale credentials when an app is deleted and recreated.
+func (s *Setup) WithStoredAppIDs(ids map[string]string) *Setup {
+	s.storedAppIDs = ids
+	return s
+}
+
 // appSetPattern validates app set slugs: lowercase alphanumeric with hyphens,
 // must start with a letter or digit, no leading/trailing/consecutive hyphens.
 var appSetPattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`)
@@ -431,13 +440,27 @@ func (s *Setup) recoverPEM(ctx context.Context, org, slug, role string) (string,
 	return pemStr, nil
 }
 
+// isAppIDStale checks whether the live installation's app ID differs from the
+// stored ROLE_APP_IDS value, indicating the app was deleted and recreated.
+func (s *Setup) isAppIDStale(org, role string, liveID int) bool {
+	if s.storedAppIDs == nil {
+		return false
+	}
+	storedID, ok := s.storedAppIDs[org+"/"+role]
+	if !ok {
+		return false
+	}
+	return storedID != strconv.Itoa(liveID)
+}
+
 // handleExistingApp reuses an existing app if its credentials are still
 // available, or reports that the private key is lost.
 //
 // GitHub App PEM private keys are only available at creation time — the
 // manifest code exchange (POST /app-manifests/{code}/conversions) is the
 // one and only time the PEM is returned. If the secret wasn't stored or
 // was deleted, the key is lost and the app must be deleted and recreated.
+//
 // The secretExists callback checks the appropriate backend (Secret Manager
 // in OIDC mint mode, GitHub repo secrets otherwise).
 //
@@ -457,7 +480,9 @@ func (s *Setup) handleExistingApp(ctx context.Context, inst *forge.Installation,
 			return nil, fmt.Errorf("checking secret for role %s: %w", role, err)
 		}
 
-		if exists {
+		stale := s.isAppIDStale(org, role, inst.AppID)
+
+		if exists && !stale {
 			s.checkPermissions(inst, org, role)
 			s.ui.StepDone(fmt.Sprintf("Reusing existing app %s (credentials present)", inst.AppSlug))
 			return &AppCredentials{
@@ -469,12 +494,26 @@ func (s *Setup) handleExistingApp(ctx context.Context, inst *forge.Installation,
 			}, nil
 		}
 
-		// Secret doesn't exist — try to recover by generating a new key.
+		if exists && stale {
+			s.ui.StepWarn(fmt.Sprintf(
+				"App %s was recreated (ID changed) — stored key is invalid",
+				inst.AppSlug))
+		}
+
+		// Secret doesn't exist or is stale — try to recover by generating a new key.
 		pemStr, recoverErr := s.recoverPEM(ctx, org, inst.AppSlug, role)
 		if recoverErr != nil {
 			return nil, fmt.Errorf("recovering PEM for %s: %w", inst.AppSlug, recoverErr)
 		}
 		if pemStr == "" {
+			if stale {
+				return nil, fmt.Errorf(
+					"app %s was recreated (ID changed) and needs a new private key; "+
+						"generate one at https://github.com/apps/%s "+
+						"or run 'fullsend admin uninstall' and re-run install",
+					inst.AppSlug, inst.AppSlug,
+				)
+			}
 			return nil, fmt.Errorf(
 				"app %s exists but its private key secret is missing; "+
 					"run 'fullsend admin uninstall' first, then delete the app at "+

internal/appsetup/appsetup_test.go

@@ -676,6 +676,136 @@ func TestSetup_ExistingApp_PEMRecovery_FileNotFound(t *testing.T) {
 	assert.Contains(t, err.Error(), "checking PEM file")
 }
 
+func TestSetup_ExistingApp_StaleAppID_TriggersRecovery(t *testing.T) {
+	pemData := generateTestPEM(t)
+	pemPath := writeTempPEM(t, pemData)
+
+	client := &forge.FakeClient{
+		Installations: []forge.Installation{
+			{ID: 100, AppID: 20, AppSlug: "fullsend-fullsend"},
+		},
+		AppClientIDs: map[string]string{
+			"fullsend-fullsend": "Iv1.fullsend123",
+		},
+	}
+	prompter := &fakePrompter{confirmResult: true, readLineResult: pemPath}
+	printer := ui.New(&discardWriter{})
+
+	var storedPEM string
+	s := NewSetup(client, prompter, newFakeBrowser(), printer).
+		WithAppSet("fullsend").
+		WithSecretExists(func(_ string) (bool, error) { return true, nil }).
+		WithStoredAppIDs(map[string]string{"myorg/fullsend": "10"}).
+		WithStoreSecret(func(_ context.Context, _, p string) error {
+			storedPEM = p
+			return nil
+		})
+
+	creds, err := s.Run(context.Background(), "myorg", "fullsend")
+	require.NoError(t, err)
+	assert.Equal(t, 20, creds.AppID)
+	assert.NotEmpty(t, creds.PEM, "should have new PEM from recovery")
+	assert.NotEmpty(t, storedPEM, "should have stored new PEM")
+	assert.True(t, prompter.confirmCalled, "should prompt for PEM recovery")
+}
+
+func TestSetup_ExistingApp_MatchingAppID_Reuses(t *testing.T) {
+	client := &forge.FakeClient{
+		Installations: []forge.Installation{
+			{ID: 100, AppID: 10, AppSlug: "fullsend-fullsend"},
+		},
+		AppClientIDs: map[string]string{
+			"fullsend-fullsend": "Iv1.fullsend123",
+		},
+	}
+	prompter := &fakePrompter{}
+	printer := ui.New(&discardWriter{})
+
+	s := NewSetup(client, prompter, newFakeBrowser(), printer).
+		WithAppSet("fullsend").
+		WithSecretExists(func(_ string) (bool, error) { return true, nil }).
+		WithStoredAppIDs(map[string]string{"myorg/fullsend": "10"})
+
+	creds, err := s.Run(context.Background(), "myorg", "fullsend")
+	require.NoError(t, err)
+	assert.Equal(t, 10, creds.AppID)
+	assert.Empty(t, creds.PEM, "PEM should be empty to signal reuse")
+	assert.False(t, prompter.confirmCalled, "should not prompt — IDs match")
+}
+
+func TestSetup_ExistingApp_NoStoredIDs_Reuses(t *testing.T) {
+	client := &forge.FakeClient{
+		Installations: []forge.Installation{
+			{ID: 100, AppID: 10, AppSlug: "fullsend-fullsend"},
+		},
+		AppClientIDs: map[string]string{
+			"fullsend-fullsend": "Iv1.fullsend123",
+		},
+	}
+	prompter := &fakePrompter{}
+	printer := ui.New(&discardWriter{})
+
+	// No WithStoredAppIDs — backwards compatible behavior.
+	s := NewSetup(client, prompter, newFakeBrowser(), printer).
+		WithAppSet("fullsend").
+		WithSecretExists(func(_ string) (bool, error) { return true, nil })
+
+	creds, err := s.Run(context.Background(), "myorg", "fullsend")
+	require.NoError(t, err)
+	assert.Equal(t, 10, creds.AppID)
+	assert.Empty(t, creds.PEM, "PEM should be empty to signal reuse")
+	assert.False(t, prompter.confirmCalled, "should not prompt — no stored IDs to compare")
+}
+
+func TestIsAppIDStale(t *testing.T) {
+	s := &Setup{}
+
+	t.Run("nil map returns false", func(t *testing.T) {
+		assert.False(t, s.isAppIDStale("org", "role", 10))
+	})
+
+	s.storedAppIDs = map[string]string{
+		"myorg/fullsend":   "10",
+		"myorg/prioritize": "20",
+	}
+
+	t.Run("matching ID returns false", func(t *testing.T) {
+		assert.False(t, s.isAppIDStale("myorg", "fullsend", 10))
+	})
+
+	t.Run("mismatched ID returns true", func(t *testing.T) {
+		assert.True(t, s.isAppIDStale("myorg", "fullsend", 99))
+	})
+
+	t.Run("unknown key returns false", func(t *testing.T) {
+		assert.False(t, s.isAppIDStale("otherog", "fullsend", 10))
+	})
+}
+
+func TestSetup_ExistingApp_StaleAppID_UserDeclines(t *testing.T) {
+	client := &forge.FakeClient{
+		Installations: []forge.Installation{
+			{ID: 100, AppID: 20, AppSlug: "fullsend-fullsend"},
+		},
+		AppClientIDs: map[string]string{
+			"fullsend-fullsend": "Iv1.fullsend123",
+		},
+	}
+	prompter := &fakePrompter{confirmResult: false}
+	printer := ui.New(&discardWriter{})
+
+	s := NewSetup(client, prompter, newFakeBrowser(), printer).
+		WithAppSet("fullsend").
+		WithSecretExists(func(_ string) (bool, error) { return true, nil }).
+		WithStoredAppIDs(map[string]string{"myorg/fullsend": "10"})
+
+	_, err := s.Run(context.Background(), "myorg", "fullsend")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "was recreated")
+	assert.True(t, prompter.confirmCalled, "should prompt for PEM recovery")
+	assert.False(t, prompter.readLineCalled, "should not ask for file path after decline")
+}
+
 func TestValidateRSAPEM_Valid(t *testing.T) {
 	assert.NoError(t, ValidateRSAPEM(generateTestPEM(t)))
 }

internal/cli/admin.go

@@ -487,12 +487,14 @@ Inference authentication:
 
 			// Pre-copy PEM secrets for shared public apps before app setup.
 			var sharedSlugs map[string]string
+			var perOrgStoredIDs map[string]string
 			if mintProject != "" && !skipAppSetup && !skipMintCheck {
-				slugs, err := copySharedAppPEMs(ctx, client, printer, org, roles, mintProject, mintRegion)
+				slugs, storedIDs, err := copySharedAppPEMs(ctx, client, printer, org, roles, mintProject, mintRegion)
 				if err != nil {
 					return err
 				}
 				sharedSlugs = slugs
+				perOrgStoredIDs = storedIDs
 			}
 
 			// Collect agent credentials via app setup.
@@ -501,7 +503,7 @@ Inference authentication:
 				if err := ensureConfigRepoExists(ctx, client, printer, org); err != nil {
 					return err
 				}
-				creds, err := runAppSetup(ctx, client, printer, org, roles, mintProject, publicApps, sharedSlugs, appSet)
+				creds, err := runAppSetup(ctx, client, printer, org, roles, mintProject, publicApps, sharedSlugs, appSet, perOrgStoredIDs)
 				if err != nil {
 					return err
 				}
@@ -832,14 +834,17 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error {
 	if needAppSetup {
 		var sharedSlugs map[string]string
 		if mintProject != "" {
-			slugs, slugErr := copySharedAppPEMs(ctx, client, printer, owner, roles, mintProject, mintRegion)
+			slugs, storedIDs, slugErr := copySharedAppPEMs(ctx, client, printer, owner, roles, mintProject, mintRegion)
 			if slugErr != nil {
 				return slugErr
 			}
 			sharedSlugs = slugs
+			if existingIDs == nil {
+				existingIDs = storedIDs
+			}
 		}
 
-		creds, credErr := runAppSetup(ctx, client, printer, owner, roles, mintProject, publicApps, sharedSlugs, c.AppSet)
+		creds, credErr := runAppSetup(ctx, client, printer, owner, roles, mintProject, publicApps, sharedSlugs, c.AppSet, existingIDs)
 		if credErr != nil {
 			return credErr
 		}
@@ -1230,23 +1235,28 @@ func resolveSharedRoleAppIDs(ctx context.Context, client forge.Client, existingI
 // their PEM secrets to the target org's naming convention. This runs before
 // app setup so that handleExistingApp finds the PEM and returns credentials
 // without trying to generate a new key.
-// Returns a role → app-slug mapping for detected shared apps so callers
-// can pass them as known slugs to app setup.
-func copySharedAppPEMs(ctx context.Context, client forge.Client, printer *ui.Printer, org string, roles []string, mintProject, mintRegion string) (map[string]string, error) {
+// Returns a role → app-slug mapping for detected shared apps and the full
+// ROLE_APP_IDS map (org/role → app_id) so callers can pass it to app setup
+// without a redundant GCP API call.
+func copySharedAppPEMs(ctx context.Context, client forge.Client, printer *ui.Printer, org string, roles []string, mintProject, mintRegion string) (map[string]string, map[string]string, error) {
 	prov := gcf.NewProvisioner(gcf.Config{
 		ProjectID:  mintProject,
 		Region:     mintRegion,
 		GitHubOrgs: []string{org},
 	}, gcf.NewLiveGCFClient())
 
 	existingIDs, err := prov.GetExistingRoleAppIDs(ctx)
-	if err != nil || len(existingIDs) == 0 {
-		return nil, nil
+	if err != nil {
+		printer.StepWarn(fmt.Sprintf("Could not read ROLE_APP_IDS: %v", err))
+		return nil, nil, nil
+	}
+	if len(existingIDs) == 0 {
+		return nil, nil, nil
 	}
 
 	installations, err := client.ListOrgInstallations(ctx, org)
 	if err != nil {
-		return nil, nil
+		return nil, existingIDs, nil
 	}
 
 	roleSet := make(map[string]bool, len(roles))
@@ -1279,25 +1289,26 @@ func copySharedAppPEMs(ctx context.Context, client forge.Client, printer *ui.Pri
 
 			printer.StepStart(fmt.Sprintf("Shared app detected: %s (app %d) — copying PEM from %s", role, inst.AppID, srcOrg))
 			if err := prov.CopyAgentPEM(ctx, srcOrg, org, role); err != nil {
-				return nil, fmt.Errorf("copying shared PEM for %s: %w", role, err)
+				return nil, nil, fmt.Errorf("copying shared PEM for %s: %w", role, err)
 			}
 			printer.StepDone(fmt.Sprintf("Copied shared %s PEM", role))
 			break
 		}
 	}
-	return sharedSlugs, nil
+	return sharedSlugs, existingIDs, nil
 }
 
 // runAppSetup creates or reuses GitHub Apps for each role. When mintProject is
 // non-empty, PEMs are also stored in GCP Secret Manager during app creation so
 // they survive partial provisioning failures.
-func runAppSetup(ctx context.Context, client forge.Client, printer *ui.Printer, org string, roles []string, mintProject string, publicApps bool, sharedSlugs map[string]string, appSet string) ([]layers.AgentCredentials, error) {
+func runAppSetup(ctx context.Context, client forge.Client, printer *ui.Printer, org string, roles []string, mintProject string, publicApps bool, sharedSlugs map[string]string, appSet string, storedAppIDs map[string]string) ([]layers.AgentCredentials, error) {
 	printer.Header("Setting up GitHub Apps")
 	printer.Blank()
 
 	setup := appsetup.NewSetup(client, appsetup.StdinPrompter{}, appsetup.DefaultBrowser{}, printer).
 		WithPublicApps(publicApps).
-		WithAppSet(appSet)
+		WithAppSet(appSet).
+		WithStoredAppIDs(storedAppIDs)
 
 	// Merge known slugs: config-based first, then shared app overrides.
 	knownSlugs := loadKnownSlugs(ctx, client, org)