Skip to content

fix(cli): print new-vault banner before first passphrase prompt#529

Merged
ALRubinger merged 1 commit intomainfrom
fix/issue-528-banner-and-comment
May 7, 2026
Merged

fix(cli): print new-vault banner before first passphrase prompt#529
ALRubinger merged 1 commit intomainfrom
fix/issue-528-banner-and-comment

Conversation

@ALRubinger
Copy link
Copy Markdown
Owner

Summary

Resolves findings 1 and 2 from #528 (manual acceptance testing of #454).

  • Finding 1 — banner ordering: the "Creating a new Aileron vault" warning was printing after the user typed a passphrase, defeating the warning's purpose. Moved it ahead of the first prompt in both promptCreateAndUnlock (auto-spawn first-run) and runVaultInit (aileron vault init).
  • Finding 2 — stale comment: sessionLogPath's comment claimed the session log lives "alongside the audit log in .aileron/", but the audit log moved to ~/.aileron/audit/ per Umbrella: Local Daemon Architecture (ADR-0012) #454 step 5. Updated to describe what the function actually does.

Approach

Added willPromptInteractively(passphraseFile string) bool next to readVaultPassphrase in cmd/aileron/vault.go. It mirrors the dispatch order (file > env > interactive) so callers can decide whether to print user-facing context before the prompt fires, instead of inferring interactivity from the post-hoc passphraseSource return value.

Both promptCreateAndUnlock and runVaultInit now print the banner conditionally on willPromptInteractively, then read the passphrase, then (if interactive) read the confirmation. File/env-sourced passphrases continue to skip the banner — non-interactive callers (CI, scripts) don't need the warning.

Test plan

  • New regression test TestEnsureVaultUnlocked_StoppedMissing_BannerPrintsBeforeFirstPrompt asserts banner index < first-prompt index in stderr (mirrors the user-observed output from Umbrella: Local Daemon Architecture (ADR-0012) #454 Test 1).
  • New regression test TestRunVaultInit_BannerPrintsBeforeFirstPrompt covers the vault init path with the same ordering check.
  • Verified both new tests fail on the pre-fix code (banner@21, prompt@0) and pass after the fix.
  • go test ./cmd/aileron/... ./internal/... — all green.
  • Coverage: cmd/aileron 85.8%, internal/launch 87.4%.

🤖 Generated with Claude Code

The first-run banner ("Creating a new Aileron vault. The passphrase
you choose protects all secrets...") was firing AFTER the user had
already typed a passphrase, defeating the warning's purpose. Move
it ahead of the first prompt in both promptCreateAndUnlock (the
auto-spawn first-run path) and runVaultInit (`aileron vault init`).

Add willPromptInteractively helper so callers can decide whether to
print user-facing context BEFORE the prompt fires, instead of inferring
interactivity from readVaultPassphrase's post-hoc source return value.

Also drop the stale audit-log reference in sessionLogPath's comment;
the audit log moved to user-scope (~/.aileron/audit/) but the session
log stayed per-project, so they're no longer "alongside" each other.

Regression tests pin the banner-before-prompt ordering for both code
paths.

Refs #528 (findings 1 and 2).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 7, 2026

🚅 Deployed to the aileron-pr-529 environment in aileron

1 service not affected by this PR
  • docs

@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.41%. Comparing base (970400c) to head (1b2ade5).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #529      +/-   ##
==========================================
- Coverage   82.34%   81.41%   -0.94%     
==========================================
  Files         221      221              
  Lines       21908    21912       +4     
==========================================
- Hits        18041    17840     -201     
- Misses       2758     2981     +223     
+ Partials     1109     1091      -18     
Flag Coverage Δ
integration 9.47% <ø> (-8.08%) ⬇️
unit 77.82% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ALRubinger ALRubinger merged commit 851f38c into main May 7, 2026
10 checks passed
@ALRubinger ALRubinger deleted the fix/issue-528-banner-and-comment branch May 7, 2026 19:42
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