Skip to content

fix(init): show reviewer credential status #351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rianjs merged 5 commits into from
Jun 18, 2026
Merged

fix(init): show reviewer credential status #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9958d7a
fix(init): show reviewer credential status
rianjs Jun 18, 2026
ca96f39
fix(init): refresh reviewer credential status
rianjs Jun 18, 2026
a140b55
fix(init): inspect reviewer template status
rianjs Jun 18, 2026
d96f21c
test(init): tighten reviewer credential status coverage
rianjs Jun 18, 2026
5448c0b
chore(init): clarify reviewer credential status contract
rianjs Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions docs/init-ux-contract.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,35 @@ 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

@monit-reviewer monit-reviewer Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low (harness-engineering:harness-knowledge-reviewer): The credential status section enumerates the seven possible key states but does not define their preconditions. The PR discussion established a critical invariant: missing must only be reported after consulting the backend (meaning not present in backend AND not staged), and status unavailable is the fallback when the backend is unreachable. Without these definitions, a future implementer adding a new status path could regress to falsely claiming missing without a backend check — the exact architectural gap this PR fixes. One sentence per state defining its precondition would make the contract self-contained.

Reply to this thread when addressed.

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`, `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.

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
Expand Down
3 changes: 2 additions & 1 deletion internal/cmd/credentialcmd/credentialcmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading