From 9958d7aa28a264afbdc25c5e67cb007322d3df3a Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 17:28:35 -0400 Subject: [PATCH 1/5] fix(init): show reviewer credential status --- docs/init-ux-contract.md | 19 ++ internal/cmd/credentialcmd/credentialcmd.go | 3 +- .../cmd/credentialcmd/credentialcmd_test.go | 314 +++++++++++++++++- .../init_reviewer_credential_status.go | 254 ++++++++++++++ .../init_reviewer_entity_editor.go | 25 +- 5 files changed, 602 insertions(+), 13 deletions(-) create mode 100644 internal/cmd/credentialcmd/init_reviewer_credential_status.go diff --git a/docs/init-ux-contract.md b/docs/init-ux-contract.md index edf941b..ee70104 100644 --- a/docs/init-ux-contract.md +++ b/docs/init-ux-contract.md @@ -162,6 +162,25 @@ choosing **Commit staged changes and exit**, any pending secret values remain draft-only and the session returns to a no-write state. Until final commit begins, cancellation must still leave both config and keyring untouched. +Credential status shown inside a subflow must also be draft-driven. Reviewer +entity setup should show non-secret, per-key credential readiness for the +selected reviewer credential ref: + +- PAT reviewers show `git_token`. +- GitHub App reviewers show required `github_app_id` and + `github_app_private_key`, plus optional `github_app_installation_id`. +- Each key may be shown as `missing`, `existing`, `staged`, `skipped optional`, + `deferred`, or `status unavailable`. +- The destination should include the storage label and the resolved + secrets-management profile/backend when known. + +This status must never show raw secret values. Draft-local writes, defers, and +optional-key skips should be preserved when the user re-enters reviewer entity +setup, but they must be filtered out if the reviewer credential ref no longer +uses those keys. Final commit remains the only write boundary for staged secret +values, and the final readiness summary should continue to report follow-up +credential work without leaking values. + ## Draft-Local Reuse Rules LLM runtimes and reviewer entities are reusable **within the current interactive diff --git a/internal/cmd/credentialcmd/credentialcmd.go b/internal/cmd/credentialcmd/credentialcmd.go index 38b03ad..d352f4f 100644 --- a/internal/cmd/credentialcmd/credentialcmd.go +++ b/internal/cmd/credentialcmd/credentialcmd.go @@ -226,6 +226,7 @@ type initPromptContext struct { ProfileGitScopes map[string]string ReviewerEntities map[string]initReviewerEntityDraft ProfileReviewerEntities map[string]string + ReviewerCredentialStatuses []initReviewerCredentialStatus SecretsProfiles []config.EffectiveSecretsProfile ProfileSecretsProfiles map[string]string BrokenProfileSecretsProfiles map[string]string @@ -1470,7 +1471,7 @@ func editInteractiveInitReviewerEntityStep(cmd *cobra.Command, opts *root.Option if prompter == nil { prompter = newHuhInitReviewerEntityPrompter(opts) } - promptCtx := currentInteractiveInitInventoryPromptContext(session) + promptCtx := currentInteractiveInitReviewerEntityPromptContext(opts, deps, session) draft, err := prompter.EditReviewerEntity(initReviewerEntityPrompt{Context: promptCtx}) if errors.Is(err, errInitNavigateBack) { return session, false, nil diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index 142648b..3f8d47d 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -6242,7 +6242,7 @@ func TestHuhInitReviewerEntityPrompterDefaultUsesLinearReviewerFlow(t *testing.T prompter := huhInitReviewerEntityPrompter{ stderr: &stderr, editorRunner: func(editor initLinearEditor, _ io.Reader, out io.Writer) (initLinearEditorModel, error) { - model := newInitLinearEditorModel(editor, 160, 24) + model := newInitLinearEditorModel(editor, 160, 60) model = selectInitLinearFieldValue(t, model, initReviewerEntityFieldSelection, string(initReviewerEntityKindPAT)) model = focusInitLinearField(t, model, initReviewerEntityFieldAction) model = selectInitLinearFieldValue(t, model, initReviewerEntityFieldAction, initDetailActionEdit) @@ -6277,6 +6277,8 @@ func TestHuhInitReviewerEntityPrompterDefaultUsesLinearReviewerFlow(t *testing.T "Personal access token (PAT) reviewer", "Entity label", "Reviewer secret location", + "Reviewer credential status", + credentials.GitTokenKey + ": missing", "Reviewer action", "Stage reviewer settings", } { @@ -6329,6 +6331,10 @@ func TestHuhInitReviewerEntityPrompterGitHubAppLinearFlowShowsCredentialBundleCo "Reviewer secret location", "non-secret", "credential-store ref", + "Reviewer credential status", + credentials.GitHubAppIDKey + ": missing", + credentials.GitHubAppPrivateKeyKey + ": missing", + credentials.GitHubAppInstallationIDKey + ":", credentials.GitHubAppIDKey, credentials.GitHubAppPrivateKeyKey, "Optional credential key: " + credentials.GitHubAppInstallationIDKey, @@ -14049,6 +14055,190 @@ func TestInitCredentialSecretPromptTitleReviewerKeys(t *testing.T) { } } +func TestInitReviewerCredentialStatusStates(t *testing.T) { + entry := initCredentialPlanEntry{ + Ref: config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: "codereview/rianjs-bot", + Mode: string(config.GitAuthModeGitHubApp), + }, + KeySpecs: []credentials.KeySpec{ + {Key: credentials.GitHubAppIDKey, Required: true}, + {Key: credentials.GitHubAppPrivateKeyKey, Required: true}, + {Key: credentials.GitHubAppInstallationIDKey, Required: false}, + }, + } + decisions := map[initCredentialDecisionKey]initCredentialDecisionKind{} + decisions[initCredentialDecisionMapKey(entry, credentials.GitHubAppInstallationIDKey)] = initCredentialDecisionSkipOptional + + status := initReviewerCredentialStatusFromEntry( + entry, + map[string]string{credentials.GitHubAppPrivateKeyKey: "sentinel-private-key"}, + decisions, + map[string]bool{credentials.GitHubAppIDKey: true}, + "", + ) + + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppIDKey, initReviewerCredentialKeyExisting) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppPrivateKeyKey, initReviewerCredentialKeyStaged) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppInstallationIDKey, initReviewerCredentialKeySkippedOptional) + if strings.Contains(initReviewerCredentialStatusDescription(status), "sentinel-private-key") { + t.Fatalf("status description leaked secret value: %s", initReviewerCredentialStatusDescription(status)) + } +} + +func TestInitReviewerCredentialStatusDeferPreservesPartialExistingKeys(t *testing.T) { + entry := initCredentialPlanEntry{ + Ref: config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: "codereview/rianjs-bot", + Mode: string(config.GitAuthModeGitHubApp), + }, + KeySpecs: []credentials.KeySpec{ + {Key: credentials.GitHubAppIDKey, Required: true}, + {Key: credentials.GitHubAppPrivateKeyKey, Required: true}, + {Key: credentials.GitHubAppInstallationIDKey, Required: false}, + }, + } + decisions := map[initCredentialDecisionKey]initCredentialDecisionKind{} + recordInitCredentialEntryDecision(decisions, entry, initCredentialDecisionDefer) + + status := initReviewerCredentialStatusFromEntry( + entry, + nil, + decisions, + map[string]bool{credentials.GitHubAppIDKey: true}, + "", + ) + + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppIDKey, initReviewerCredentialKeyExisting) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppPrivateKeyKey, initReviewerCredentialKeyDeferred) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppInstallationIDKey, initReviewerCredentialKeyOptional) +} + +func TestInitReviewerCredentialStatusBackendUnavailableDoesNotLeakOrBlock(t *testing.T) { + entry := initCredentialPlanEntry{ + Ref: config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: "codereview/work-reviewer", + Mode: string(config.GitAuthModePAT), + }, + KeySpecs: []credentials.KeySpec{{Key: credentials.GitTokenKey, Required: true}}, + } + session := initSessionDraft{ + cfg: config.File{ + DefaultProfile: "work", + Profiles: map[string]config.Profile{"work": basicProfile("work")}, + }, + workspace: &initWorkspaceDraft{ + profileName: "work", + profile: basicProfile("work"), + credentialPlan: []initCredentialPlanEntry{entry}, + }, + writes: map[string]map[string]string{}, + credentialDecisions: map[initCredentialDecisionKey]initCredentialDecisionKind{}, + } + deps := initDeps{ + openStore: func(string, bool, config.File) (initStore, error) { + return nil, errors.New("sentinel backend unavailable") + }, + openResolvedStore: func(credentials.ResolvedSecretsProfile, string, bool, config.File) (initStore, error) { + return nil, errors.New("sentinel backend unavailable") + }, + } + + statuses := buildInteractiveInitReviewerCredentialStatuses(&root.Options{}, deps, session) + if len(statuses) != 1 { + t.Fatalf("statuses = %#v, want one status", statuses) + } + status := statuses[0] + assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyUnavailable) + description := initReviewerCredentialStatusDescription(status) + if !strings.Contains(description, "credential backend status unavailable") { + t.Fatalf("description = %q, want unavailable note", description) + } + if strings.Contains(description, "sentinel backend unavailable") { + t.Fatalf("description leaked backend error detail: %s", description) + } +} + +func TestInitReviewerCredentialStatusShowsExistingPATAndSecretsProfileDestination(t *testing.T) { + profile := basicProfile("work") + profile.SecretsProfile = "work-1password" + profile.ReviewerCredentials = &config.ReviewerCredentials{ + AuthMode: config.GitAuthModePAT, + CredentialRef: "codereview/work-reviewer", + } + cfg := config.File{ + DefaultProfile: "work", + Secrets: config.SecretsConfig{ + DefaultProfile: "work-1password", + Profiles: map[string]config.SecretsProfile{ + "work-1password": { + Label: "Work Vault", + Backend: config.SecretsProfileBackend{Kind: config.SecretsBackendKind(credstore.BackendOPDesktop)}, + }, + }, + }, + Profiles: map[string]config.Profile{"work": profile}, + } + store := newFakeInitStore(map[string]map[string]string{ + "work-reviewer": {credentials.GitTokenKey: "existing-token"}, + }) + session := initSessionDraft{ + cfg: cfg, + workspace: &initWorkspaceDraft{ + profileName: "work", + profile: profile, + }, + writes: map[string]map[string]string{}, + credentialDecisions: map[initCredentialDecisionKey]initCredentialDecisionKind{}, + } + var opened []string + deps := initDeps{ + openStore: func(string, bool, config.File) (initStore, error) { + t.Fatal("legacy openStore called for named secrets profile") + return nil, nil + }, + openResolvedStore: func(resolved credentials.ResolvedSecretsProfile, _ string, _ bool, _ config.File) (initStore, error) { + opened = append(opened, resolved.ID) + return store, nil + }, + } + + statuses := buildInteractiveInitReviewerCredentialStatuses(&root.Options{}, deps, session) + if len(statuses) != 1 { + t.Fatalf("statuses = %#v, want one reviewer status", statuses) + } + status := statuses[0] + assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyExisting) + if !reflect.DeepEqual(opened, []string{"work-1password"}) { + t.Fatalf("opened secrets profiles = %#v, want work-1password", opened) + } + description := initReviewerCredentialStatusDescription(status) + for _, want := range []string{"Destination: codereview/work-reviewer via Work Vault", string(credstore.BackendOPDesktop)} { + if !strings.Contains(description, want) { + t.Fatalf("description = %q, want %q", description, want) + } + } + if strings.Contains(description, "existing-token") { + t.Fatalf("description leaked secret value: %s", description) + } +} + +func assertReviewerCredentialKeyState(t *testing.T, status initReviewerCredentialStatus, key string, want initReviewerCredentialKeyState) { + t.Helper() + for _, row := range status.Keys { + if row.Key == key { + if row.State != want { + t.Fatalf("%s state = %q, want %q in %#v", key, row.State, want, status.Keys) + } + return + } + } + t.Fatalf("missing key %q in %#v", key, status.Keys) +} + func TestInitInteractiveMenuFocusedGitHubAppReviewerDeferEmitsReadinessAndHints(t *testing.T) { path := filepath.Join(t.TempDir(), "config.yml") cfg := config.File{ @@ -14701,6 +14891,106 @@ func TestInitInteractiveMenuReviewerSetNowBackDuringSecretStepDiscardsPartialSec } } +func TestInitInteractiveMenuReviewerCredentialStatusShowsStagedOnReentry(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.yml") + saveCredentialTestConfig(t, path, config.File{ + DefaultProfile: "work", + Profiles: map[string]config.Profile{"work": basicProfile("work")}, + }) + opts := &root.Options{ + Stdin: strings.NewReader(""), + Stdout: &bytes.Buffer{}, + Stderr: &bytes.Buffer{}, + ConfigPath: path, + } + store := newFakeInitStore(map[string]map[string]string{ + "work": {credentials.GitTokenKey: "existing-token"}, + }) + store.setBundleFunc = func(string, map[string]string, ...credstore.SetOpt) (credstore.Result, error) { + t.Fatal("SetBundle called before commit") + return credstore.Result{}, nil + } + reviewerPrompterCalls := 0 + deps := initDeps{ + menuPrompter: &fakeInitMenuPrompter{ + actions: []initMenuAction{ + initMenuActionReviewerEntities, + initMenuActionReviewerEntities, + initMenuActionExit, + }, + }, + reviewerPrompter: initReviewerEntityPrompterFunc(func(prompt initReviewerEntityPrompt) (initDraft, error) { + reviewerPrompterCalls++ + switch reviewerPrompterCalls { + case 1: + draft := seedInteractiveInitDraft(prompt.Context.RequestedProfileName, prompt.Context.ExistingProfileName, prompt.Context.DefaultProfileName, prompt.Context.ExistingProfile) + draft.ReviewerEnabled = true + draft.ReviewerAuth = string(config.GitAuthModeGitHubApp) + draft.ReviewerCredentialRef = "codereview/rianjs-bot" + return draft, nil + case 2: + if len(prompt.Context.ReviewerCredentialStatuses) == 0 { + t.Fatalf("second prompt reviewer statuses empty; existing profile name = %q; staged profile = %#v", prompt.Context.ExistingProfileName, prompt.Context.ExistingConfig.Profiles["work"].ReviewerCredentials) + } + status := findReviewerCredentialStatusForTest(t, prompt.Context.ReviewerCredentialStatuses, "codereview/rianjs-bot", string(config.GitAuthModeGitHubApp)) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppIDKey, initReviewerCredentialKeyStaged) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppPrivateKeyKey, initReviewerCredentialKeyStaged) + assertReviewerCredentialKeyState(t, status, credentials.GitHubAppInstallationIDKey, initReviewerCredentialKeySkippedOptional) + description := initReviewerCredentialStatusDescription(status) + if strings.Contains(description, "sentinel") { + t.Fatalf("status leaked secret value: %s", description) + } + return initDraft{}, errInitNavigateBack + default: + return initDraft{}, errInitNavigateBack + } + }), + secretPrompter: &fakeInitSecretPrompter{ + actions: []initCredentialSecretAction{initCredentialSecretActionSetNow}, + sources: []initSecretSource{ + initSecretSourcePaste, + initSecretSourcePaste, + initSecretSourceSkip, + }, + pastes: []string{"sentinel-app-id", "sentinel-private-key"}, + }, + openStore: func(string, bool, config.File) (initStore, error) { + return store, nil + }, + configPath: func(*root.Options) (string, error) { return path, nil }, + loadConfig: loadConfigForInit, + saveConfig: func(string, config.File) error { + t.Fatal("saveConfig called on discard") + return nil + }, + } + + if err := runInitWithDeps(&cobra.Command{}, opts, initOptions{}, deps); err != nil { + t.Fatalf("runInitWithDeps: %v", err) + } + if reviewerPrompterCalls != 2 { + t.Fatalf("reviewerPrompterCalls = %d, want staged reviewer and reentry", reviewerPrompterCalls) + } + if _, ok := store.bundles["rianjs-bot"]; ok { + t.Fatalf("reviewer bundle = %#v, want no credential write before commit", store.bundles["rianjs-bot"]) + } +} + +func findReviewerCredentialStatusForTest(t *testing.T, statuses []initReviewerCredentialStatus, ref string, mode string) initReviewerCredentialStatus { + t.Helper() + for _, status := range statuses { + if status.Ref.Ref == ref && status.Ref.Mode == mode { + return status + } + } + available := make([]string, 0, len(statuses)) + for _, status := range statuses { + available = append(available, fmt.Sprintf("%s/%s keys=%d", status.Ref.Ref, status.Ref.Mode, len(status.Keys))) + } + t.Fatalf("missing reviewer credential status ref=%q mode=%q; available: %s", ref, mode, strings.Join(available, ", ")) + return initReviewerCredentialStatus{} +} + func TestInitInteractiveMenuFocusedReviewerEntitySavePreservesCustomCredentialRef(t *testing.T) { path := filepath.Join(t.TempDir(), "config.yml") cfg := config.File{ @@ -14788,7 +15078,7 @@ func TestInitInteractiveMenuFocusedReviewerEntitySavePreservesCustomCredentialRe } } -func TestInitInteractiveMenuFocusedReviewerEntityLabelOnlySaveSkipsStore(t *testing.T) { +func TestInitInteractiveMenuFocusedReviewerEntityLabelOnlySaveSkipsCredentialWrite(t *testing.T) { path := filepath.Join(t.TempDir(), "config.yml") cfg := config.File{ DefaultProfile: "work", @@ -14820,7 +15110,19 @@ func TestInitInteractiveMenuFocusedReviewerEntityLabelOnlySaveSkipsStore(t *test ConfigPath: path, } reviewerPrompterCalls := 0 - openStoreCalls := 0 + store := newFakeInitStore(map[string]map[string]string{ + "work": { + credentials.GitTokenKey: "existing-token", + }, + "work-reviewer": { + credentials.GitHubAppIDKey: "12345", + credentials.GitHubAppPrivateKeyKey: "private-key", + }, + }) + store.setBundleFunc = func(string, map[string]string, ...credstore.SetOpt) (credstore.Result, error) { + t.Fatal("SetBundle called for reviewer label-only save") + return credstore.Result{}, nil + } deps := initDeps{ menuPrompter: &fakeInitMenuPrompter{ actions: []initMenuAction{ @@ -14843,8 +15145,7 @@ func TestInitInteractiveMenuFocusedReviewerEntityLabelOnlySaveSkipsStore(t *test return draft, nil }), openStore: func(string, bool, config.File) (initStore, error) { - openStoreCalls++ - return newFakeInitStore(nil), nil + return store, nil }, configPath: func(*root.Options) (string, error) { return path, nil }, loadConfig: loadConfigForInit, @@ -14854,9 +15155,6 @@ func TestInitInteractiveMenuFocusedReviewerEntityLabelOnlySaveSkipsStore(t *test if err := runInitWithDeps(&cobra.Command{}, opts, initOptions{}, deps); err != nil { t.Fatalf("runInitWithDeps: %v", err) } - if openStoreCalls != 0 { - t.Fatalf("openStoreCalls = %d, want 0 for reviewer label-only save", openStoreCalls) - } got, err := config.Load(path) if err != nil { t.Fatalf("Load config: %v", err) diff --git a/internal/cmd/credentialcmd/init_reviewer_credential_status.go b/internal/cmd/credentialcmd/init_reviewer_credential_status.go new file mode 100644 index 0000000..6260652 --- /dev/null +++ b/internal/cmd/credentialcmd/init_reviewer_credential_status.go @@ -0,0 +1,254 @@ +package credentialcmd + +import ( + "fmt" + "strings" + + "github.com/open-cli-collective/codereview-cli/internal/cmd/root" + "github.com/open-cli-collective/codereview-cli/internal/config" + "github.com/open-cli-collective/codereview-cli/internal/credentials" +) + +type initReviewerCredentialKeyState string + +const ( + initReviewerCredentialKeyMissing initReviewerCredentialKeyState = "missing" + initReviewerCredentialKeyExisting initReviewerCredentialKeyState = "existing" + initReviewerCredentialKeyStaged initReviewerCredentialKeyState = "staged" + initReviewerCredentialKeySkippedOptional initReviewerCredentialKeyState = "skipped optional" + initReviewerCredentialKeyDeferred initReviewerCredentialKeyState = "deferred" + initReviewerCredentialKeyOptional initReviewerCredentialKeyState = "optional" + initReviewerCredentialKeyUnavailable initReviewerCredentialKeyState = "status unavailable" +) + +type initReviewerCredentialStatus struct { + Ref config.CredentialRef + SecretsProfile credentials.ResolvedSecretsProfile + Keys []initReviewerCredentialKeyStatus + Unavailable string +} + +type initReviewerCredentialKeyStatus struct { + Key string + Required bool + State initReviewerCredentialKeyState +} + +func currentInteractiveInitReviewerEntityPromptContext(opts *root.Options, deps initDeps, session initSessionDraft) initPromptContext { + ctx := currentInteractiveInitInventoryPromptContext(session) + ctx.ReviewerCredentialStatuses = buildInteractiveInitReviewerCredentialStatuses(opts, deps, session) + return ctx +} + +func buildInteractiveInitReviewerCredentialStatuses(opts *root.Options, deps initDeps, session initSessionDraft) []initReviewerCredentialStatus { + if session.workspace == nil { + return nil + } + plannedWriteKeys := projectInitPlannedWriteKeys(session.writes) + entries := interactiveInitReviewerCredentialPlanEntries(session, plannedWriteKeys) + statuses := make([]initReviewerCredentialStatus, 0, len(entries)) + stores := map[string]initStore{} + defer func() { + for _, store := range stores { + if store != nil { + _ = store.Close() + } + } + }() + for _, entry := range entries { + if entry.Ref.Purpose != "reviewer_credentials" || entry.State == initCredentialPlanStateClearRef { + continue + } + existing := map[string]bool{} + unavailable := "" + storeKey := initCredentialStoreKey(entry.SecretsProfile) + store := stores[storeKey] + if store == nil { + if !canOpenInitStoreForReviewerStatus(deps, entry) { + unavailable = "credential backend status unavailable" + } else { + opened, err := openInitStoreForEntry(deps, opts, session.backendFlagSet, session.cfg, entry) + if err != nil || opened == nil { + unavailable = "credential backend status unavailable" + } else { + store = opened + stores[storeKey] = store + } + } + } + if store != nil { + keys, err := existingInitCredentialKeys(store, entry.Ref.Ref) + if err != nil { + unavailable = "credential backend status unavailable" + } else { + existing = keys + } + } + statuses = append(statuses, initReviewerCredentialStatusFromEntry(entry, session.writes[entry.Ref.Ref], session.credentialDecisions, existing, unavailable)) + } + return statuses +} + +func canOpenInitStoreForReviewerStatus(deps initDeps, entry initCredentialPlanEntry) bool { + if entry.SecretsProfile.IsNamed() { + return deps.openResolvedStore != nil + } + return deps.openStore != nil +} + +func interactiveInitReviewerCredentialPlanEntries(session initSessionDraft, plannedWriteKeys map[string][]string) []initCredentialPlanEntry { + profile := session.workspace.profile + if currentProfile, ok := session.cfg.Profiles[session.workspace.profileName]; ok { + profile = currentProfile + } + entries, err := planInitCredentialsWithConfig(session.cfg, session.workspace.previousProfile, profile, plannedWriteKeys) + if err == nil && hasInitReviewerCredentialPlanEntry(entries) { + return refreshInteractiveCredentialPlan(entries, plannedWriteKeys, session.satisfiedRefs) + } + return refreshInteractiveCredentialPlan(session.workspace.credentialPlan, plannedWriteKeys, session.satisfiedRefs) +} + +func hasInitReviewerCredentialPlanEntry(entries []initCredentialPlanEntry) bool { + for _, entry := range entries { + if entry.Ref.Purpose == "reviewer_credentials" { + return true + } + } + return false +} + +func initReviewerCredentialStatusFromEntry(entry initCredentialPlanEntry, planned map[string]string, decisions map[initCredentialDecisionKey]initCredentialDecisionKind, existing map[string]bool, unavailable string) initReviewerCredentialStatus { + status := initReviewerCredentialStatus{ + Ref: entry.Ref, + SecretsProfile: entry.SecretsProfile, + Unavailable: unavailable, + Keys: make([]initReviewerCredentialKeyStatus, 0, len(entry.KeySpecs)), + } + for _, spec := range entry.KeySpecs { + status.Keys = append(status.Keys, initReviewerCredentialKeyStatus{ + Key: spec.Key, + Required: spec.Required, + State: deriveInitReviewerCredentialKeyState(entry, spec, planned, decisions, existing, unavailable != ""), + }) + } + return status +} + +func deriveInitReviewerCredentialKeyState(entry initCredentialPlanEntry, spec credentials.KeySpec, planned map[string]string, decisions map[initCredentialDecisionKey]initCredentialDecisionKind, existing map[string]bool, unavailable bool) initReviewerCredentialKeyState { + if _, ok := planned[spec.Key]; ok { + return initReviewerCredentialKeyStaged + } + decision := decisions[initCredentialDecisionMapKey(entry, spec.Key)] + if decision == initCredentialDecisionSkipOptional && !spec.Required { + return initReviewerCredentialKeySkippedOptional + } + if existing[spec.Key] { + return initReviewerCredentialKeyExisting + } + if decision == initCredentialDecisionDefer && spec.Required { + return initReviewerCredentialKeyDeferred + } + if unavailable { + return initReviewerCredentialKeyUnavailable + } + if spec.Required { + return initReviewerCredentialKeyMissing + } + return initReviewerCredentialKeyOptional +} + +func initReviewerCredentialStatusForSelection(ctx initPromptContext, seed initDraft, selection string) (initReviewerCredentialStatus, bool) { + state, err := reviewerEntityEditorStateForSelection(ctx, seed, selection) + if err != nil || state.kind == initReviewerEntityKindUseGitIdentity { + return initReviewerCredentialStatus{}, false + } + ref := strings.TrimSpace(state.seed.ReviewerCredentialRef) + if ref == "" { + ref = state.standardReviewerRef + } + if ref == "" { + return initReviewerCredentialStatus{}, false + } + authMode := reviewerCredentialAuthModeForKind(state.kind) + statusRef := config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: ref, + Mode: string(authMode), + } + for _, status := range ctx.ReviewerCredentialStatuses { + if status.Ref.Purpose == statusRef.Purpose && status.Ref.Ref == statusRef.Ref && status.Ref.Mode == statusRef.Mode { + return status, true + } + } + return synthesizeReviewerCredentialStatus(ctx, statusRef), true +} + +func synthesizeReviewerCredentialStatus(ctx initPromptContext, ref config.CredentialRef) initReviewerCredentialStatus { + status := initReviewerCredentialStatus{Ref: ref} + if ctx.ExistingProfile != nil { + if resolved, err := credentials.ResolveSecretsProfileForProfile(ctx.ExistingConfig, *ctx.ExistingProfile); err == nil { + status.SecretsProfile = resolved + } else { + status.Unavailable = "credential backend status unavailable" + } + } + specs, err := credentials.KeySpecsForPurpose(ref) + if err != nil { + status.Unavailable = "credential status unavailable" + return status + } + for _, spec := range specs { + keyState := initReviewerCredentialKeyMissing + if !spec.Required { + keyState = initReviewerCredentialKeyOptional + } + status.Keys = append(status.Keys, initReviewerCredentialKeyStatus{ + Key: spec.Key, + Required: spec.Required, + State: keyState, + }) + } + return status +} + +func reviewerCredentialAuthModeForKind(kind initReviewerEntityKind) config.GitAuthMode { + switch kind { + case initReviewerEntityKindGitHubApp: + return config.GitAuthModeGitHubApp + case initReviewerEntityKindPAT, initReviewerEntityKindUseGitIdentity: + return config.GitAuthModePAT + default: + return config.GitAuthModePAT + } +} + +func initReviewerCredentialStatusDescription(status initReviewerCredentialStatus) string { + var lines []string + lines = append(lines, "Destination: "+initReviewerCredentialDestinationDescription(status)) + if strings.TrimSpace(status.Unavailable) != "" { + lines = append(lines, strings.TrimSpace(status.Unavailable)+".") + } + for _, key := range status.Keys { + lines = append(lines, fmt.Sprintf("- %s: %s", key.Key, key.State)) + } + return strings.Join(lines, "\n") +} + +func initReviewerCredentialDestinationDescription(status initReviewerCredentialStatus) string { + ref := strings.TrimSpace(status.Ref.Ref) + if ref == "" { + ref = "(standard reviewer secret location)" + } + backend := strings.TrimSpace(status.SecretsProfile.Backend) + store := strings.TrimSpace(status.SecretsProfile.DisplayName()) + switch { + case store != "" && backend != "": + return fmt.Sprintf("%s via %s (%s)", ref, store, backend) + case store != "": + return fmt.Sprintf("%s via %s", ref, store) + case backend != "": + return fmt.Sprintf("%s via %s", ref, backend) + default: + return ref + } +} diff --git a/internal/cmd/credentialcmd/init_reviewer_entity_editor.go b/internal/cmd/credentialcmd/init_reviewer_entity_editor.go index 92a28b0..83890d4 100644 --- a/internal/cmd/credentialcmd/init_reviewer_entity_editor.go +++ b/internal/cmd/credentialcmd/init_reviewer_entity_editor.go @@ -9,16 +9,18 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/huh" + "github.com/open-cli-collective/codereview-cli/internal/config" "github.com/open-cli-collective/codereview-cli/internal/credentials" ) type initReviewerEntityEditorRunner func(initLinearEditor, io.Reader, io.Writer) (initLinearEditorModel, error) const ( - initReviewerEntityFieldSelection initLinearFieldID = "reviewer_entity_selection" - initReviewerEntityFieldLabel initLinearFieldID = "reviewer_entity_label" - initReviewerEntityFieldSecretLocation initLinearFieldID = "reviewer_entity_secret_location" - initReviewerEntityFieldAction initLinearFieldID = "reviewer_entity_action" + initReviewerEntityFieldSelection initLinearFieldID = "reviewer_entity_selection" + initReviewerEntityFieldLabel initLinearFieldID = "reviewer_entity_label" + initReviewerEntityFieldSecretLocation initLinearFieldID = "reviewer_entity_secret_location" + initReviewerEntityFieldCredentialStatus initLinearFieldID = "reviewer_entity_credential_status" + initReviewerEntityFieldAction initLinearFieldID = "reviewer_entity_action" ) const ( @@ -169,6 +171,16 @@ func (s reviewerEntityEditorState) editor() initLinearEditor { reviewerSecretLocation, validateOptionalCredentialRef, ) + status := synthesizeReviewerCredentialStatus(initPromptContext{}, config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: firstNonEmpty(reviewerSecretLocation, s.standardReviewerRef), + Mode: string(reviewerCredentialAuthModeForKind(s.kind)), + }) + document.addSectionField( + initReviewerEntityFieldCredentialStatus, + "Reviewer credential status", + initReviewerCredentialStatusDescription(status), + ) } document.addEditableSelect(initReviewerEntityFieldAction, "Reviewer detail action", "", []huh.Option[string]{ huh.NewOption("Stage reviewer settings", initDetailActionEdit), @@ -257,6 +269,7 @@ func initReviewerEntityLinearEditor(ctx initPromptContext, seed initDraft) initL "", validateOptionalCredentialRef, ) + document.addSectionField(initReviewerEntityFieldCredentialStatus, "Reviewer credential status", "", initLinearFieldOptions{Hidden: true}) document.addEditableSelect(initReviewerEntityFieldAction, "Reviewer action", "", initReviewerEntityActionOptions(ctx, selection), initDetailActionEdit) editor := initLinearEditor{ Document: document, @@ -386,8 +399,12 @@ func initReviewerEntitySyncLinearFields(model *initLinearEditorModel, ctx initPr } model.setFieldHidden(initReviewerEntityFieldLabel, hideDetails) model.setFieldHidden(initReviewerEntityFieldSecretLocation, hideDetails) + model.setFieldHidden(initReviewerEntityFieldCredentialStatus, hideDetails) if !hideDetails { model.setFieldDescription(initReviewerEntityFieldSecretLocation, reviewerEntitySecretLocationDescription(state.kind)) + if status, ok := initReviewerCredentialStatusForSelection(ctx, seed, selection); ok { + model.setFieldDescription(initReviewerEntityFieldCredentialStatus, initReviewerCredentialStatusDescription(status)) + } } if resetDetails && !hideDetails { labelInput, _, _ := reviewerEntityEditorLabelSeed(initReviewerEntityDraft{ From ca96f39dbab14565a2cae7403d17c4fd0c2a36e4 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 17:38:18 -0400 Subject: [PATCH 2/5] fix(init): refresh reviewer credential status --- docs/init-ux-contract.md | 2 +- .../cmd/credentialcmd/credentialcmd_test.go | 102 ++++++++++++++++++ .../init_reviewer_credential_status.go | 60 +++++++++-- .../init_reviewer_entity_editor.go | 11 +- 4 files changed, 164 insertions(+), 11 deletions(-) diff --git a/docs/init-ux-contract.md b/docs/init-ux-contract.md index ee70104..1e7ef7a 100644 --- a/docs/init-ux-contract.md +++ b/docs/init-ux-contract.md @@ -170,7 +170,7 @@ selected reviewer credential ref: - GitHub App reviewers show required `github_app_id` and `github_app_private_key`, plus optional `github_app_installation_id`. - Each key may be shown as `missing`, `existing`, `staged`, `skipped optional`, - `deferred`, or `status unavailable`. + `deferred`, `optional`, or `status unavailable`. - The destination should include the storage label and the resolved secrets-management profile/backend when known. diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index 3f8d47d..b7aedda 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -6346,6 +6346,68 @@ func TestHuhInitReviewerEntityPrompterGitHubAppLinearFlowShowsCredentialBundleCo } } +func TestHuhInitReviewerEntityStatusUpdatesWhenSecretLocationChanges(t *testing.T) { + profile := basicProfile("work") + profile.ReviewerCredentials = &config.ReviewerCredentials{ + AuthMode: config.GitAuthModePAT, + CredentialRef: "codereview/old-reviewer", + } + cfg := config.File{ + DefaultProfile: "work", + Profiles: map[string]config.Profile{"work": profile}, + } + entities, profileEntities := buildInitReviewerEntityInventory(cfg) + profileCopy := profile + ctx := initPromptContext{ + RequestedProfileName: "work", + ExistingProfileName: "work", + DefaultProfileName: "work", + ExistingProfile: &profileCopy, + ExistingConfig: cfg, + ReviewerEntities: entities, + ProfileReviewerEntities: profileEntities, + ReviewerCredentialStatuses: []initReviewerCredentialStatus{{ + Ref: config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: "codereview/old-reviewer", + Mode: string(config.GitAuthModePAT), + }, + Keys: []initReviewerCredentialKeyStatus{{ + Key: credentials.GitTokenKey, + Required: true, + State: initReviewerCredentialKeyExisting, + }}, + }}, + } + seed := seedInteractiveInitDraft("work", "work", "work", &profile) + model := newInitLinearEditorModel(initReviewerEntityLinearEditor(ctx, seed), 180, 60) + statusIndex := model.document.fieldIndexByID(initReviewerEntityFieldCredentialStatus) + if statusIndex < 0 { + t.Fatal("reviewer credential status field missing") + } + initial := model.document[statusIndex].Description + if !strings.Contains(initial, "codereview/old-reviewer") || !strings.Contains(initial, credentials.GitTokenKey+": existing") { + t.Fatalf("initial status = %q, want old reviewer existing token", initial) + } + + locationIndex := model.document.fieldIndexByID(initReviewerEntityFieldSecretLocation) + if locationIndex < 0 { + t.Fatal("reviewer secret location field missing") + } + model.document[locationIndex].Value = "codereview/new-reviewer" + model.afterFieldChange(locationIndex) + + updated := model.document[statusIndex].Description + for _, want := range []string{"Destination: codereview/new-reviewer", credentials.GitTokenKey + ": missing"} { + if !strings.Contains(updated, want) { + t.Fatalf("updated status = %q, want %q", updated, want) + } + } + if strings.Contains(updated, "codereview/old-reviewer") || strings.Contains(updated, credentials.GitTokenKey+": existing") { + t.Fatalf("updated status = %q, want old ref existing state cleared", updated) + } +} + func TestHuhInitReviewerEntityDetailsBackDoesNotMutateDraft(t *testing.T) { t.Setenv("TERM", "dumb") draft := seedInteractiveInitDraft("work", "work", "work", nil) @@ -14226,6 +14288,46 @@ func TestInitReviewerCredentialStatusShowsExistingPATAndSecretsProfileDestinatio } } +func TestInitReviewerCredentialStatusIncludesSelectableReviewerEntities(t *testing.T) { + work := basicProfile("work") + bot := basicProfile("bot") + bot.ReviewerCredentials = &config.ReviewerCredentials{ + AuthMode: config.GitAuthModePAT, + CredentialRef: "codereview/shared-reviewer", + } + cfg := config.File{ + DefaultProfile: "work", + Profiles: map[string]config.Profile{ + "work": work, + "bot": bot, + }, + } + store := newFakeInitStore(map[string]map[string]string{ + "shared-reviewer": {credentials.GitTokenKey: "existing-token"}, + }) + session := initSessionDraft{ + cfg: cfg, + workspace: &initWorkspaceDraft{ + profileName: "work", + profile: work, + }, + writes: map[string]map[string]string{}, + credentialDecisions: map[initCredentialDecisionKey]initCredentialDecisionKind{}, + } + deps := initDeps{ + openStore: func(string, bool, config.File) (initStore, error) { + return store, nil + }, + } + + statuses := buildInteractiveInitReviewerCredentialStatuses(&root.Options{}, deps, session) + status := findReviewerCredentialStatusForTest(t, statuses, "codereview/shared-reviewer", string(config.GitAuthModePAT)) + assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyExisting) + if strings.Contains(initReviewerCredentialStatusDescription(status), "existing-token") { + t.Fatalf("status description leaked secret value: %s", initReviewerCredentialStatusDescription(status)) + } +} + func assertReviewerCredentialKeyState(t *testing.T, status initReviewerCredentialStatus, key string, want initReviewerCredentialKeyState) { t.Helper() for _, row := range status.Keys { diff --git a/internal/cmd/credentialcmd/init_reviewer_credential_status.go b/internal/cmd/credentialcmd/init_reviewer_credential_status.go index 6260652..e3f3613 100644 --- a/internal/cmd/credentialcmd/init_reviewer_credential_status.go +++ b/internal/cmd/credentialcmd/init_reviewer_credential_status.go @@ -102,10 +102,11 @@ func interactiveInitReviewerCredentialPlanEntries(session initSessionDraft, plan profile = currentProfile } entries, err := planInitCredentialsWithConfig(session.cfg, session.workspace.previousProfile, profile, plannedWriteKeys) - if err == nil && hasInitReviewerCredentialPlanEntry(entries) { - return refreshInteractiveCredentialPlan(entries, plannedWriteKeys, session.satisfiedRefs) + if err != nil || !hasInitReviewerCredentialPlanEntry(entries) { + entries = append([]initCredentialPlanEntry(nil), session.workspace.credentialPlan...) } - return refreshInteractiveCredentialPlan(session.workspace.credentialPlan, plannedWriteKeys, session.satisfiedRefs) + entries = appendSelectableReviewerCredentialPlanEntries(session, profile, plannedWriteKeys, entries) + return refreshInteractiveCredentialPlan(entries, plannedWriteKeys, session.satisfiedRefs) } func hasInitReviewerCredentialPlanEntry(entries []initCredentialPlanEntry) bool { @@ -117,6 +118,50 @@ func hasInitReviewerCredentialPlanEntry(entries []initCredentialPlanEntry) bool return false } +func appendSelectableReviewerCredentialPlanEntries(session initSessionDraft, profile config.Profile, plannedWriteKeys map[string][]string, entries []initCredentialPlanEntry) []initCredentialPlanEntry { + resolved, err := credentials.ResolveSecretsProfileForProfile(session.cfg, profile) + if err != nil { + return entries + } + seen := make(map[string]struct{}, len(entries)) + for _, entry := range entries { + seen[initCredentialEntryKey(entry.Ref)] = struct{}{} + } + entities, _ := buildInitReviewerEntityInventory(session.cfg) + for _, entity := range entities { + if entity.Kind == initReviewerEntityKindUseGitIdentity { + continue + } + ref := config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: strings.TrimSpace(entity.CredentialRef), + Mode: string(entity.AuthMode), + } + if ref.Ref == "" { + continue + } + key := initCredentialEntryKey(ref) + if _, ok := seen[key]; ok { + continue + } + specs, err := credentials.KeySpecsForPurpose(ref) + if err != nil { + continue + } + entry := initCredentialPlanEntry{ + Ref: ref, + SecretsProfile: resolved, + KeySpecs: append([]credentials.KeySpec(nil), specs...), + PlannedWriteKeys: append([]string(nil), plannedWriteKeys[ref.Ref]...), + } + entry.MissingRequiredKeys = missingRequiredInitCredentialKeys(entry.KeySpecs, entry.PlannedWriteKeys) + entry.State = classifyInitCredentialPlanEntry(entry) + entries = append(entries, entry) + seen[key] = struct{}{} + } + return entries +} + func initReviewerCredentialStatusFromEntry(entry initCredentialPlanEntry, planned map[string]string, decisions map[initCredentialDecisionKey]initCredentialDecisionKind, existing map[string]bool, unavailable string) initReviewerCredentialStatus { status := initReviewerCredentialStatus{ Ref: entry.Ref, @@ -157,14 +202,17 @@ func deriveInitReviewerCredentialKeyState(entry initCredentialPlanEntry, spec cr return initReviewerCredentialKeyOptional } -func initReviewerCredentialStatusForSelection(ctx initPromptContext, seed initDraft, selection string) (initReviewerCredentialStatus, bool) { +func initReviewerCredentialStatusForSelectionRef(ctx initPromptContext, seed initDraft, selection string, reviewerSecretLocation string) (initReviewerCredentialStatus, bool) { state, err := reviewerEntityEditorStateForSelection(ctx, seed, selection) if err != nil || state.kind == initReviewerEntityKindUseGitIdentity { return initReviewerCredentialStatus{}, false } - ref := strings.TrimSpace(state.seed.ReviewerCredentialRef) + ref := strings.TrimSpace(reviewerSecretLocation) if ref == "" { - ref = state.standardReviewerRef + ref = strings.TrimSpace(state.seed.ReviewerCredentialRef) + if ref == "" { + ref = state.standardReviewerRef + } } if ref == "" { return initReviewerCredentialStatus{}, false diff --git a/internal/cmd/credentialcmd/init_reviewer_entity_editor.go b/internal/cmd/credentialcmd/init_reviewer_entity_editor.go index 83890d4..2163d5f 100644 --- a/internal/cmd/credentialcmd/init_reviewer_entity_editor.go +++ b/internal/cmd/credentialcmd/init_reviewer_entity_editor.go @@ -282,7 +282,7 @@ func initReviewerEntityLinearEditor(ctx initPromptContext, seed initDraft) initL initReviewerEntitySyncLinearFields(model, ctx, seed, true) return } - if id == initReviewerEntityFieldAction { + if id == initReviewerEntityFieldAction || id == initReviewerEntityFieldSecretLocation { initReviewerEntitySyncLinearFields(model, ctx, seed, false) } }, @@ -402,9 +402,6 @@ func initReviewerEntitySyncLinearFields(model *initLinearEditorModel, ctx initPr model.setFieldHidden(initReviewerEntityFieldCredentialStatus, hideDetails) if !hideDetails { model.setFieldDescription(initReviewerEntityFieldSecretLocation, reviewerEntitySecretLocationDescription(state.kind)) - if status, ok := initReviewerCredentialStatusForSelection(ctx, seed, selection); ok { - model.setFieldDescription(initReviewerEntityFieldCredentialStatus, initReviewerCredentialStatusDescription(status)) - } } if resetDetails && !hideDetails { labelInput, _, _ := reviewerEntityEditorLabelSeed(initReviewerEntityDraft{ @@ -429,6 +426,12 @@ func initReviewerEntitySyncLinearFields(model *initLinearEditorModel, ctx initPr model.setFieldValue(initReviewerEntityFieldLabel, labelInput) model.setFieldValue(initReviewerEntityFieldSecretLocation, reviewerSecretLocation) } + if !hideDetails { + reviewerSecretLocation := model.document.fieldValue(initReviewerEntityFieldSecretLocation) + if status, ok := initReviewerCredentialStatusForSelectionRef(ctx, seed, selection, reviewerSecretLocation); ok { + model.setFieldDescription(initReviewerEntityFieldCredentialStatus, initReviewerCredentialStatusDescription(status)) + } + } } func initReviewerEntitySetSelectionOptions(model *initLinearEditorModel, ctx initPromptContext, selected string) { From a140b5556ace59eb8bf8aaa99799a2abc9fc6dc1 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 17:43:03 -0400 Subject: [PATCH 3/5] fix(init): inspect reviewer template status --- .../cmd/credentialcmd/credentialcmd_test.go | 71 +++++++++++++++---- .../init_reviewer_credential_status.go | 65 ++++++++++++----- 2 files changed, 103 insertions(+), 33 deletions(-) diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index b7aedda..81b80bc 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -6398,7 +6398,7 @@ func TestHuhInitReviewerEntityStatusUpdatesWhenSecretLocationChanges(t *testing. model.afterFieldChange(locationIndex) updated := model.document[statusIndex].Description - for _, want := range []string{"Destination: codereview/new-reviewer", credentials.GitTokenKey + ": missing"} { + for _, want := range []string{"Destination: codereview/new-reviewer", "credential backend status unavailable", credentials.GitTokenKey + ": status unavailable"} { if !strings.Contains(updated, want) { t.Fatalf("updated status = %q, want %q", updated, want) } @@ -13909,6 +13909,11 @@ func TestInitInteractiveMenuFocusedBackKeepsSessionUntouched(t *testing.T) { Stderr: &bytes.Buffer{}, ConfigPath: path, } + store := newFakeInitStore(nil) + store.setBundleFunc = func(string, map[string]string, ...credstore.SetOpt) (credstore.Result, error) { + t.Fatal("SetBundle should not run after focused Back navigation") + return credstore.Result{}, nil + } deps := initDeps{ menuPrompter: &fakeInitMenuPrompter{ actions: []initMenuAction{ @@ -13924,8 +13929,7 @@ func TestInitInteractiveMenuFocusedBackKeepsSessionUntouched(t *testing.T) { return initDraft{}, errInitNavigateBack }), openStore: func(string, bool, config.File) (initStore, error) { - t.Fatal("openStore should not run for focused Back navigation") - return nil, nil + return store, nil }, configPath: func(*root.Options) (string, error) { return path, nil }, loadConfig: loadConfigForInit, @@ -14210,10 +14214,7 @@ func TestInitReviewerCredentialStatusBackendUnavailableDoesNotLeakOrBlock(t *tes } statuses := buildInteractiveInitReviewerCredentialStatuses(&root.Options{}, deps, session) - if len(statuses) != 1 { - t.Fatalf("statuses = %#v, want one status", statuses) - } - status := statuses[0] + status := findReviewerCredentialStatusForTest(t, statuses, "codereview/work-reviewer", string(config.GitAuthModePAT)) assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyUnavailable) description := initReviewerCredentialStatusDescription(status) if !strings.Contains(description, "credential backend status unavailable") { @@ -14269,10 +14270,7 @@ func TestInitReviewerCredentialStatusShowsExistingPATAndSecretsProfileDestinatio } statuses := buildInteractiveInitReviewerCredentialStatuses(&root.Options{}, deps, session) - if len(statuses) != 1 { - t.Fatalf("statuses = %#v, want one reviewer status", statuses) - } - status := statuses[0] + status := findReviewerCredentialStatusForTest(t, statuses, "codereview/work-reviewer", string(config.GitAuthModePAT)) assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyExisting) if !reflect.DeepEqual(opened, []string{"work-1password"}) { t.Fatalf("opened secrets profiles = %#v, want work-1password", opened) @@ -14328,6 +14326,41 @@ func TestInitReviewerCredentialStatusIncludesSelectableReviewerEntities(t *testi } } +func TestInitReviewerCredentialStatusIncludesStandardTemplateRefs(t *testing.T) { + work := basicProfile("work") + cfg := config.File{ + DefaultProfile: "work", + Profiles: map[string]config.Profile{"work": work}, + } + store := newFakeInitStore(map[string]map[string]string{ + "work-reviewer": {credentials.GitTokenKey: "existing-token"}, + }) + session := initSessionDraft{ + cfg: cfg, + workspace: &initWorkspaceDraft{ + profileName: "work", + profile: work, + }, + writes: map[string]map[string]string{}, + credentialDecisions: map[initCredentialDecisionKey]initCredentialDecisionKind{}, + } + deps := initDeps{ + openStore: func(string, bool, config.File) (initStore, error) { + return store, nil + }, + } + + ctx := currentInteractiveInitReviewerEntityPromptContext(&root.Options{}, deps, session) + status, ok := initReviewerCredentialStatusForSelectionRef(ctx, seedInteractiveInitDraft("work", "work", "work", &work), string(initReviewerEntityKindPAT), "") + if !ok { + t.Fatal("PAT template status missing") + } + assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyExisting) + if strings.Contains(initReviewerCredentialStatusDescription(status), "existing-token") { + t.Fatalf("status description leaked secret value: %s", initReviewerCredentialStatusDescription(status)) + } +} + func assertReviewerCredentialKeyState(t *testing.T, status initReviewerCredentialStatus, key string, want initReviewerCredentialKeyState) { t.Helper() for _, row := range status.Keys { @@ -15372,7 +15405,7 @@ func TestInitInteractiveMenuFocusedReviewerEntitySaveRestoresDefaultCredentialRe } } -func TestInitInteractiveMenuFocusedReviewerEntityDoesNotOpenStoreForPromptContext(t *testing.T) { +func TestInitInteractiveMenuFocusedReviewerEntityPromptContextIncludesCredentialStatus(t *testing.T) { path := filepath.Join(t.TempDir(), "config.yml") saveCredentialTestConfig(t, path, config.File{ DefaultProfile: "work", @@ -15384,6 +15417,13 @@ func TestInitInteractiveMenuFocusedReviewerEntityDoesNotOpenStoreForPromptContex Stderr: &bytes.Buffer{}, ConfigPath: path, } + store := newFakeInitStore(map[string]map[string]string{ + "work-reviewer": {credentials.GitTokenKey: "existing-token"}, + }) + store.setBundleFunc = func(string, map[string]string, ...credstore.SetOpt) (credstore.Result, error) { + t.Fatal("SetBundle should not run while building focused reviewer prompt context") + return credstore.Result{}, nil + } deps := initDeps{ menuPrompter: &fakeInitMenuPrompter{ actions: []initMenuAction{ @@ -15391,12 +15431,13 @@ func TestInitInteractiveMenuFocusedReviewerEntityDoesNotOpenStoreForPromptContex initMenuActionExit, }, }, - reviewerPrompter: initReviewerEntityPrompterFunc(func(_ initReviewerEntityPrompt) (initDraft, error) { + reviewerPrompter: initReviewerEntityPrompterFunc(func(prompt initReviewerEntityPrompt) (initDraft, error) { + status := findReviewerCredentialStatusForTest(t, prompt.Context.ReviewerCredentialStatuses, "codereview/work-reviewer", string(config.GitAuthModePAT)) + assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyExisting) return initDraft{}, errInitNavigateBack }), openStore: func(string, bool, config.File) (initStore, error) { - t.Fatal("openStore should not run for focused reviewer prompt context") - return nil, nil + return store, nil }, configPath: func(*root.Options) (string, error) { return path, nil }, loadConfig: loadConfigForInit, diff --git a/internal/cmd/credentialcmd/init_reviewer_credential_status.go b/internal/cmd/credentialcmd/init_reviewer_credential_status.go index e3f3613..b649785 100644 --- a/internal/cmd/credentialcmd/init_reviewer_credential_status.go +++ b/internal/cmd/credentialcmd/init_reviewer_credential_status.go @@ -127,6 +127,18 @@ func appendSelectableReviewerCredentialPlanEntries(session initSessionDraft, pro for _, entry := range entries { seen[initCredentialEntryKey(entry.Ref)] = struct{}{} } + if standardRef, err := credentials.FormatRef(session.workspace.profileName + "-reviewer"); err == nil { + entries = appendReviewerCredentialPlanEntry(entries, seen, resolved, plannedWriteKeys, config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: standardRef, + Mode: string(config.GitAuthModePAT), + }) + entries = appendReviewerCredentialPlanEntry(entries, seen, resolved, plannedWriteKeys, config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: standardRef, + Mode: string(config.GitAuthModeGitHubApp), + }) + } entities, _ := buildInitReviewerEntityInventory(session.cfg) for _, entity := range entities { if entity.Kind == initReviewerEntityKindUseGitIdentity { @@ -140,28 +152,33 @@ func appendSelectableReviewerCredentialPlanEntries(session initSessionDraft, pro if ref.Ref == "" { continue } - key := initCredentialEntryKey(ref) - if _, ok := seen[key]; ok { - continue - } - specs, err := credentials.KeySpecsForPurpose(ref) - if err != nil { - continue - } - entry := initCredentialPlanEntry{ - Ref: ref, - SecretsProfile: resolved, - KeySpecs: append([]credentials.KeySpec(nil), specs...), - PlannedWriteKeys: append([]string(nil), plannedWriteKeys[ref.Ref]...), - } - entry.MissingRequiredKeys = missingRequiredInitCredentialKeys(entry.KeySpecs, entry.PlannedWriteKeys) - entry.State = classifyInitCredentialPlanEntry(entry) - entries = append(entries, entry) - seen[key] = struct{}{} + entries = appendReviewerCredentialPlanEntry(entries, seen, resolved, plannedWriteKeys, ref) } return entries } +func appendReviewerCredentialPlanEntry(entries []initCredentialPlanEntry, seen map[string]struct{}, resolved credentials.ResolvedSecretsProfile, plannedWriteKeys map[string][]string, ref config.CredentialRef) []initCredentialPlanEntry { + key := initCredentialEntryKey(ref) + if _, ok := seen[key]; ok { + return entries + } + specs, err := credentials.KeySpecsForPurpose(ref) + if err != nil { + return entries + } + entry := initCredentialPlanEntry{ + Ref: ref, + SecretsProfile: resolved, + KeySpecs: append([]credentials.KeySpec(nil), specs...), + PlannedWriteKeys: append([]string(nil), plannedWriteKeys[ref.Ref]...), + } + entry.MissingRequiredKeys = missingRequiredInitCredentialKeys(entry.KeySpecs, entry.PlannedWriteKeys) + entry.State = classifyInitCredentialPlanEntry(entry) + entries = append(entries, entry) + seen[key] = struct{}{} + return entries +} + func initReviewerCredentialStatusFromEntry(entry initCredentialPlanEntry, planned map[string]string, decisions map[initCredentialDecisionKey]initCredentialDecisionKind, existing map[string]bool, unavailable string) initReviewerCredentialStatus { status := initReviewerCredentialStatus{ Ref: entry.Ref, @@ -228,6 +245,9 @@ func initReviewerCredentialStatusForSelectionRef(ctx initPromptContext, seed ini return status, true } } + if ctx.ReviewerCredentialStatuses != nil { + return synthesizeUnavailableReviewerCredentialStatus(ctx, statusRef), true + } return synthesizeReviewerCredentialStatus(ctx, statusRef), true } @@ -259,6 +279,15 @@ func synthesizeReviewerCredentialStatus(ctx initPromptContext, ref config.Creden return status } +func synthesizeUnavailableReviewerCredentialStatus(ctx initPromptContext, ref config.CredentialRef) initReviewerCredentialStatus { + status := synthesizeReviewerCredentialStatus(ctx, ref) + status.Unavailable = "credential backend status unavailable" + for i := range status.Keys { + status.Keys[i].State = initReviewerCredentialKeyUnavailable + } + return status +} + func reviewerCredentialAuthModeForKind(kind initReviewerEntityKind) config.GitAuthMode { switch kind { case initReviewerEntityKindGitHubApp: From d96f21cfb1add3ad6e1b8dec23d668d28b0e4f93 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 17:49:15 -0400 Subject: [PATCH 4/5] test(init): tighten reviewer credential status coverage --- .../cmd/credentialcmd/credentialcmd_test.go | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/internal/cmd/credentialcmd/credentialcmd_test.go b/internal/cmd/credentialcmd/credentialcmd_test.go index 81b80bc..b09dd1f 100644 --- a/internal/cmd/credentialcmd/credentialcmd_test.go +++ b/internal/cmd/credentialcmd/credentialcmd_test.go @@ -6286,7 +6286,7 @@ func TestHuhInitReviewerEntityPrompterDefaultUsesLinearReviewerFlow(t *testing.T t.Fatalf("stderr missing %q:\n%s", want, out) } } - assertContentOrder(t, out, "Reviewer entity", "Reviewer details", "Entity label", "Reviewer secret location", "Reviewer action") + assertContentOrder(t, out, "Reviewer entity", "Reviewer details", "Entity label", "Reviewer secret location", "Reviewer credential status", "Reviewer action") if strings.Contains(out, "Back to main menu") { t.Fatalf("stderr = %q, want action-local Back without staging instead of inventory Back", out) } @@ -6344,6 +6344,7 @@ func TestHuhInitReviewerEntityPrompterGitHubAppLinearFlowShowsCredentialBundleCo t.Fatalf("stderr missing %q:\n%s", want, out) } } + assertContentOrder(t, out, "Reviewer secret location", "Reviewer credential status", "Reviewer action") } func TestHuhInitReviewerEntityStatusUpdatesWhenSecretLocationChanges(t *testing.T) { @@ -14182,6 +14183,47 @@ func TestInitReviewerCredentialStatusDeferPreservesPartialExistingKeys(t *testin assertReviewerCredentialKeyState(t, status, credentials.GitHubAppInstallationIDKey, initReviewerCredentialKeyOptional) } +func TestInitReviewerCredentialStatusSelectionFiltersByAuthMode(t *testing.T) { + ctx := initPromptContext{ + RequestedProfileName: "work", + ExistingProfileName: "work", + DefaultProfileName: "work", + ExistingConfig: config.File{Profiles: map[string]config.Profile{"work": basicProfile("work")}}, + ReviewerCredentialStatuses: []initReviewerCredentialStatus{ + { + Ref: config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: "codereview/work-reviewer", + Mode: string(config.GitAuthModeGitHubApp), + }, + Keys: []initReviewerCredentialKeyStatus{ + {Key: credentials.GitHubAppIDKey, Required: true, State: initReviewerCredentialKeyStaged}, + {Key: credentials.GitHubAppPrivateKeyKey, Required: true, State: initReviewerCredentialKeyStaged}, + }, + }, + { + Ref: config.CredentialRef{ + Purpose: "reviewer_credentials", + Ref: "codereview/work-reviewer", + Mode: string(config.GitAuthModePAT), + }, + Keys: []initReviewerCredentialKeyStatus{ + {Key: credentials.GitTokenKey, Required: true, State: initReviewerCredentialKeyMissing}, + }, + }, + }, + } + seed := seedInteractiveInitDraft("work", "work", "work", nil) + + status, ok := initReviewerCredentialStatusForSelectionRef(ctx, seed, string(initReviewerEntityKindPAT), "codereview/work-reviewer") + if !ok { + t.Fatal("PAT status missing") + } + assertReviewerCredentialKeyState(t, status, credentials.GitTokenKey, initReviewerCredentialKeyMissing) + assertReviewerCredentialKeyAbsent(t, status, credentials.GitHubAppIDKey) + assertReviewerCredentialKeyAbsent(t, status, credentials.GitHubAppPrivateKeyKey) +} + func TestInitReviewerCredentialStatusBackendUnavailableDoesNotLeakOrBlock(t *testing.T) { entry := initCredentialPlanEntry{ Ref: config.CredentialRef{ @@ -14374,6 +14416,15 @@ func assertReviewerCredentialKeyState(t *testing.T, status initReviewerCredentia t.Fatalf("missing key %q in %#v", key, status.Keys) } +func assertReviewerCredentialKeyAbsent(t *testing.T, status initReviewerCredentialStatus, key string) { + t.Helper() + for _, row := range status.Keys { + if row.Key == key { + t.Fatalf("unexpected key %q in %#v", key, status.Keys) + } + } +} + func TestInitInteractiveMenuFocusedGitHubAppReviewerDeferEmitsReadinessAndHints(t *testing.T) { path := filepath.Join(t.TempDir(), "config.yml") cfg := config.File{ From 5448c0b436ed8ae94febbc83c4a3d5765646042f Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Thu, 18 Jun 2026 17:54:56 -0400 Subject: [PATCH 5/5] chore(init): clarify reviewer credential status contract --- docs/init-ux-contract.md | 10 +++++++ .../init_reviewer_credential_status.go | 27 ++++++++++--------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/docs/init-ux-contract.md b/docs/init-ux-contract.md index 1e7ef7a..b3afda6 100644 --- a/docs/init-ux-contract.md +++ b/docs/init-ux-contract.md @@ -171,6 +171,16 @@ selected reviewer credential ref: `github_app_private_key`, plus optional `github_app_installation_id`. - Each key may be shown as `missing`, `existing`, `staged`, `skipped optional`, `deferred`, `optional`, or `status unavailable`. +- `missing` means the backend was consulted and no staged or existing value was + found for a required key. +- `existing` means the backend reports a stored value for the key. +- `staged` means a draft-local value will be written at final commit. +- `skipped optional` means the user explicitly skipped an optional key in the + current draft. +- `deferred` means the user deferred a required key in the current draft. +- `optional` means an optional key has no staged, skipped, or existing value. +- `status unavailable` means the backend or key contract could not be inspected, + so the UI must not claim a key is missing. - The destination should include the storage label and the resolved secrets-management profile/backend when known. diff --git a/internal/cmd/credentialcmd/init_reviewer_credential_status.go b/internal/cmd/credentialcmd/init_reviewer_credential_status.go index b649785..36dfc00 100644 --- a/internal/cmd/credentialcmd/init_reviewer_credential_status.go +++ b/internal/cmd/credentialcmd/init_reviewer_credential_status.go @@ -224,20 +224,20 @@ func initReviewerCredentialStatusForSelectionRef(ctx initPromptContext, seed ini if err != nil || state.kind == initReviewerEntityKindUseGitIdentity { return initReviewerCredentialStatus{}, false } - ref := strings.TrimSpace(reviewerSecretLocation) - if ref == "" { - ref = strings.TrimSpace(state.seed.ReviewerCredentialRef) - if ref == "" { - ref = state.standardReviewerRef + effectiveCredentialRef := strings.TrimSpace(reviewerSecretLocation) + if effectiveCredentialRef == "" { + effectiveCredentialRef = strings.TrimSpace(state.seed.ReviewerCredentialRef) + if effectiveCredentialRef == "" { + effectiveCredentialRef = state.standardReviewerRef } } - if ref == "" { + if effectiveCredentialRef == "" { return initReviewerCredentialStatus{}, false } authMode := reviewerCredentialAuthModeForKind(state.kind) statusRef := config.CredentialRef{ Purpose: "reviewer_credentials", - Ref: ref, + Ref: effectiveCredentialRef, Mode: string(authMode), } for _, status := range ctx.ReviewerCredentialStatuses { @@ -245,7 +245,8 @@ func initReviewerCredentialStatusForSelectionRef(ctx initPromptContext, seed ini return status, true } } - if ctx.ReviewerCredentialStatuses != nil { + backendStatusesWereComputed := ctx.ReviewerCredentialStatuses != nil + if backendStatusesWereComputed { return synthesizeUnavailableReviewerCredentialStatus(ctx, statusRef), true } return synthesizeReviewerCredentialStatus(ctx, statusRef), true @@ -317,12 +318,12 @@ func initReviewerCredentialDestinationDescription(status initReviewerCredentialS ref = "(standard reviewer secret location)" } backend := strings.TrimSpace(status.SecretsProfile.Backend) - store := strings.TrimSpace(status.SecretsProfile.DisplayName()) + storeLabel := strings.TrimSpace(status.SecretsProfile.DisplayName()) switch { - case store != "" && backend != "": - return fmt.Sprintf("%s via %s (%s)", ref, store, backend) - case store != "": - return fmt.Sprintf("%s via %s", ref, store) + case storeLabel != "" && backend != "": + return fmt.Sprintf("%s via %s (%s)", ref, storeLabel, backend) + case storeLabel != "": + return fmt.Sprintf("%s via %s", ref, storeLabel) case backend != "": return fmt.Sprintf("%s via %s", ref, backend) default: