Skip to content

feat(machine): smart prompt defaults for machine config#106

Open
nvandessel wants to merge 1 commit intomainfrom
feat/smart-prompt-defaults
Open

feat(machine): smart prompt defaults for machine config#106
nvandessel wants to merge 1 commit intomainfrom
feat/smart-prompt-defaults

Conversation

@nvandessel
Copy link
Owner

Summary

  • Add resolveDefaults() to enrich machine config prompts with auto-detected values
  • user_name and user_email default from git config --global
  • signing_key converts to select dropdown when GPG keys are detected (with "None" and "Enter manually..." options)
  • Post-processing extracts key IDs from formatted select values
  • Returns a copy — never mutates input prompts

Depends on: #PR1 (feat/ssh-keygen-wizard)

Test plan

  • make build && make lint && make test passes
  • Verify defaults populated from git config
  • Verify existing non-empty defaults are NOT overwritten
  • Verify original prompts not mutated (copy semantics)

🤖 Generated with Claude Code

@coderabbitai
Copy link

coderabbitai bot commented Feb 28, 2026

Warning

Rate limit exceeded

@nvandessel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 2 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 71e97e4 and 20a069a.

📒 Files selected for processing (2)
  • internal/machine/prompts.go
  • internal/machine/prompts_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/smart-prompt-defaults

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Feb 28, 2026

Greptile Summary

Adds smart prompt defaults for machine config by auto-detecting git user info and GPG keys. The resolveDefaults() function enriches prompts with values from git config --global and converts signing_key to a select dropdown when GPG keys are detected.

Key changes:

  • user_name and user_email auto-populate from git config if empty
  • signing_key becomes a select dropdown with detected GPG keys plus "None" and "Enter manually..." options
  • Post-processing extracts key IDs from formatted select values
  • Returns a copy with proper slice allocation for Prompts, original not mutated
  • Comprehensive test coverage for default resolution and copy semantics

Previous threads raised valid points:

  • Test should verify Options slice independence for complete copy semantics validation
  • Format "%s — %s" differs from existing FormatGPGKeyChoice() which uses "%s <%s> (%s)" - consider consistency

Confidence Score: 4/5

  • Safe to merge with minor recommendations from previous threads
  • Implementation is functionally correct with proper copy semantics for the main use case. Previous threads identified two valid but non-blocking improvements: Options slice independence test and format consistency with existing helper.
  • No files require special attention - previous thread comments address the minor improvements needed

Important Files Changed

Filename Overview
internal/machine/prompts.go Adds resolveDefaults() to auto-populate git config and GPG keys. Copy semantics are mostly correct, format differs from existing FormatGPGKeyChoice() helper.
internal/machine/prompts_test.go Adds comprehensive tests for resolveDefaults(). Previous thread correctly notes test should verify Options slice independence.

Last reviewed commit: 20a069a

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +235 to +247
func TestResolveDefaults_NoCopy(t *testing.T) {
// Verify original prompts are not mutated
original := config.MachinePrompt{
ID: "test",
Prompts: []config.PromptField{
{ID: "user_name", Default: ""},
},
}
_ = resolveDefaults(original)
if original.Prompts[0].Default != "" {
t.Error("original prompts were mutated")
}
}
Copy link

Choose a reason for hiding this comment

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

Test should also verify Options slice independence to ensure complete copy semantics. Add check like:

Suggested change
func TestResolveDefaults_NoCopy(t *testing.T) {
// Verify original prompts are not mutated
original := config.MachinePrompt{
ID: "test",
Prompts: []config.PromptField{
{ID: "user_name", Default: ""},
},
}
_ = resolveDefaults(original)
if original.Prompts[0].Default != "" {
t.Error("original prompts were mutated")
}
}
func TestResolveDefaults_NoCopy(t *testing.T) {
// Verify original prompts are not mutated
original := config.MachinePrompt{
ID: "test",
Prompts: []config.PromptField{
{ID: "user_name", Default: "", Options: []string{"a", "b"}},
},
}
result := resolveDefaults(original)
if original.Prompts[0].Default != "" {
t.Error("original prompts were mutated")
}
// Verify Options slice is independent (if modified)
if len(original.Prompts[0].Options) != 2 {
t.Error("original Options slice was modified")
}
}

p.Type = "select"
p.Options = []string{"None"}
for _, k := range keys {
p.Options = append(p.Options, fmt.Sprintf("%s — %s", k.KeyID, k.Email))
Copy link

Choose a reason for hiding this comment

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

Format differs from FormatGPGKeyChoice() in git.go (line 291-293) which uses "%s <%s> (%s)". Consider using the existing formatter for consistency, or document why different formats are needed.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@nvandessel
Copy link
Owner Author

nvandessel commented Feb 28, 2026

🌴 Frond Stack

main
└── feat/ssh-keygen-wizard  #104  [ready]
    ├── feat/gh-key-registration  #105  [blocked: ssh-keygen-wizard]
    │   └── feat/setup-verification  #107  [blocked: gh-key-registration, smart-prompt-defaults]
    └── feat/smart-prompt-defaults  #106  👈  [blocked: ssh-keygen-wizard]

Managed by frond

Base automatically changed from feat/ssh-keygen-wizard to main February 28, 2026 18:28
Add resolveDefaults() to enrich prompts with auto-detected values:
- user_name defaults from git config --global user.name
- user_email defaults from git config --global user.email
- signing_key converts to select dropdown when GPG keys detected
  (with "None" and "Enter manually..." options)
- Post-processing extracts key IDs from formatted select values
- Returns a copy, never mutates input prompts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@nvandessel nvandessel force-pushed the feat/smart-prompt-defaults branch from c8ad870 to 20a069a Compare February 28, 2026 18:30
@nvandessel
Copy link
Owner Author

@greptileai review

@codecov
Copy link

codecov bot commented Feb 28, 2026

Codecov Report

❌ Patch coverage is 52.94118% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.70%. Comparing base (71e97e4) to head (20a069a).

Files with missing lines Patch % Lines
internal/machine/prompts.go 52.94% 16 Missing ⚠️

❌ Your patch check has failed because the patch coverage (52.94%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
+ Coverage   48.68%   48.70%   +0.01%     
==========================================
  Files         108      108              
  Lines       12059    12092      +33     
==========================================
+ Hits         5871     5889      +18     
- Misses       6188     6203      +15     
Files with missing lines Coverage Δ
internal/machine/prompts.go 51.33% <52.94%> (+0.90%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant