fix(init): clarify github app reviewer credentials

[rianjs]

Summary

• Add GitHub App-specific reviewer credential copy to both reviewer-entity editor paths.
• Show required and optional GitHub App credential keys in readiness text.
• Keep deferred follow-up commands required-only, omitting optional github_app_installation_id.

Closes #339

Verification

• go test ./internal/cmd/credentialcmd ./internal/credentials ./internal/config
• make check
• tmux 120x40, isolated HOME/config, configured rianjs-bot GitHub App reviewer and deferred reviewer credentials.

Reviewer details capture included:

Reviewer secret location
... non-secret credential-store ref such as codereview/work-reviewer ...
... required secrets github_app_id and github_app_private_key ... github_app_installation_id is optional.

Final readiness/hints capture included:

- work: needs follow-up (reviewer deferred (required: github_app_id, github_app_private_key; optional: github_app_installation_id))
Next: cr set-credential --ref codereview/rianjs-bot --key github_app_id --stdin
Next: cr set-credential --ref codereview/rianjs-bot --key github_app_private_key --stdin

[rianjs]

Findings

Minor: initCredentialReadinessNote now appends key summaries for every deferred / needs-setup credential entry, not just GitHub App or multi-key entries. That is a visible UX copy change for single-key PAT/LLM/profile Git credentials outside #339’s stated scope, and it may add noise ahead of the wording-cleanup ticket. Please either gate the summary to multi-key/GitHub App entries or add explicit tests/PR rationale showing the broader readiness copy is intentional.

Minor: The new readiness test only covers the defer branch. The code also has a distinct overwrite_ref / missing_required path that formats MissingRequiredKeys plus optional keys. Add a partial GitHub App bundle test for that branch so github_app_installation_id stays labeled optional and never appears as missing required.

Minor: The tmux evidence shows the reviewer details copy and final command hints, but not the final readiness text even though readiness wording changed. Please capture or mention the readiness line from the real TUI/finalize flow so the user-facing readiness behavior is empirically verified, not only unit-tested.

[rianjs]

Findings

Minor: The code-level concerns from the prior review are addressed, but the supplied verification evidence still only shows the reviewer details copy and final command hints. This diff changes initCredentialReadinessNote, so the PR should also include the real tmux/finalize readiness capture showing the required/optional GitHub App keys. The new unit tests cover the behavior, but the batch requirement is empirical TUI verification for user-facing UX changes.

[rianjs]

No findings.

The new initProfileReadinessLine keeps finalize-screen and post-save readiness formatting aligned, and the updated verification now covers the real TUI readiness output plus required-only follow-up hints. The implementation remains scoped to reviewer GitHub App copy, multi-key readiness precision, and required-key command hints.

[rianjs]

Coverage Gap

• [Minor] The new GitHub App checks are still mostly helper-level smoke tests. credentialcmd_test.go and credentialcmd_test.go prove the formatting helpers render the intended copy, but they do not run the full init path that actually emits readiness text and cr set-credential follow-up hints. A regression in runInitWithDeps/applyInteractiveInitSessionPlan wiring for deferred GitHub App reviewers could still pass these tests. One end-to-end init test with a deferred GitHub App reviewer would close that gap.

[monit-reviewer]

Automated PR Review

Reviewed commit: 3d0c2c9

Approved with 2 non-blocking suggestions below. Address at your discretion.

Summary

Reviewer	Findings
harness-engineering:harness-self-documenting-code-reviewer	2

harness-engineering:harness-self-documenting-code-reviewer (2 findings)

💡 Suggestion - internal/cmd/credentialcmd/credentialcmd.go:6157

initCredentialScopedKeySummary: the word 'Scoped' does not communicate intent. A reader comparing this to initCredentialKeySummary cannot tell from names alone what distinguishes them. A name like initCredentialKeySummaryIfMultiKey would make the guard condition legible without reading the body.

💡 Suggestion - internal/cmd/credentialcmd/init_reviewer_entity_editor.go:447

reviewerEntityKindDetailDescription (GitHub App branch) and reviewerEntitySecretLocationDescription (line 455) both assemble strings via chained + concatenation of string literals and credential key constants. The sentence structure is interrupted by the constant splices, making the prose hard to read at a glance. Using fmt.Sprintf with the full sentence as a format string in both functions would make the copy structure immediately legible.

1 info-level observations excluded. Run with --verbose to include.

4 PR discussion threads considered.

---

Completed in 2m 05s | $1.55 | sonnet | daemon 0.2.132 | Glorfindel

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	2m 05s wall · 2m 01s compute (Reviewers: 1m 23s · Synthesis: 38s)
Cost	$1.55 (estimated)
Tokens	305.1k in / 12.4k out
Turns	12

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	38.0k	2.1k	13.8k	24.2k (1h)	$0.18
harness-engineering:harness-architecture-reviewer	sonnet	51.8k	810	13.8k	38.0k (1h)	$0.24
harness-engineering:harness-enforcement-reviewer	sonnet	53.9k	2.5k	13.8k	40.1k (1h)	$0.28
harness-engineering:harness-knowledge-reviewer	sonnet	52.4k	1.2k	13.8k	38.7k (1h)	$0.25
harness-engineering:harness-self-documenting-code-reviewer	sonnet	56.8k	4.8k	13.8k	43.0k (1h)	$0.33
security:security-code-auditor	sonnet	52.1k	1.1k	13.8k	38.4k (1h)	$0.25

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

[monit-reviewer]

🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): initCredentialScopedKeySummary: the word 'Scoped' does not communicate intent. A reader comparing this to initCredentialKeySummary cannot tell from names alone what distinguishes them. A name like initCredentialKeySummaryIfMultiKey would make the guard condition legible without reading the body.

Reply to this thread when addressed.

[monit-reviewer]

🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): reviewerEntityKindDetailDescription (GitHub App branch) and reviewerEntitySecretLocationDescription (line 455) both assemble strings via chained + concatenation of string literals and credential key constants. The sentence structure is interrupted by the constant splices, making the prose hard to read at a glance. Using fmt.Sprintf with the full sentence as a format string in both functions would make the copy structure immediately legible.

Reply to this thread when addressed.