feat(tui): stop auto-init in projects + dual global/project health pills#19
Merged
cooper (czxtm) merged 3 commits intomainfrom May 10, 2026
Merged
feat(tui): stop auto-init in projects + dual global/project health pills#19cooper (czxtm) merged 3 commits intomainfrom
cooper (czxtm) merged 3 commits intomainfrom
Conversation
added 2 commits
May 9, 2026 19:56
Selecting `key_provider: macos-keychain` (via `himitsu init` or the TUI
wizard) used to be cosmetic — the wizard offered the choice, the config
flag persisted, but every read/write code path went through
`<data_dir>/key` regardless. The keychain stayed empty and the secret
sat on disk in the same place as the disk provider.
Wiring:
- New `crypto::keystore` module owns provider-aware persistence:
`store_new_key`, `load_identity`, `migrate_disk_to_keychain`, plus a
pubkey-file probe (`is_initialized`) that's the canonical "has the
user run init" check regardless of provider.
- `Context` gains a `key_provider` field, populated once at dispatcher
boot (and at TUI launch) by reading the user config. New
`Context::load_identity()` chokepoints every command that decrypts —
13 call sites migrated from `age::read_identity(&ctx.key_path())`.
- `init.rs` reorders so the active provider is settled BEFORE keypair
generation. With keychain selected:
1. The secret goes to a `security add-generic-password` entry
under `io.darkmatter.himitsu.agekey.byfp.v1`, keyed by the
pubkey's fingerprint.
2. The pubkey file is still written to disk (provider-agnostic).
3. No `<data_dir>/key` is created.
- Migration: switching to keychain on an already-initialized machine
moves the existing on-disk secret into the keychain and deletes the
disk file (only after the keychain write succeeds — safe to retry).
- "Is initialized" probes (`cli::mod::Cli::run`, `tui::mod::should_launch_init_flow`)
now check `key.pub` instead of `key`, so post-migration himitsu
doesn't loop into auto-init on every command.
- README explains the per-provider semantics and migration behavior.
Bonus: the envs view picks up Ctrl+W as a save alias alongside Ctrl+S
(stashed local polish that was sitting in the working tree).
Verified manually:
- Fresh init with `--key-provider macos-keychain`: pubkey on disk, no
secret on disk, keychain entry under .byfp.v1.
- Decrypt against a real store via the keychain succeeds (`himitsu get`,
`ls`, `search` all work without the on-disk secret).
- 704 tests pass, clippy clean.
Three related fixes to the TUI launch flow: 1. Don't auto-fire the init wizard from projects without their own store. The previous \`should_launch_init_flow\` returned true whenever the resolved active store was empty — so cd-ing into any repo that wasn't itself wired to a himitsu store bounced the user into setup. Now the wizard fires only when himitsu isn't initialized at all OR the user has zero registered stores under \`stores_dir\`. A project with no store of its own just lands on the dashboard with the project pill grayed out. 2. Replace the single store-health indicator in the dashboard header with two pills: \`global: <status>\` and \`project: <status>\`. The project pill renders muted gray "n/a" when the current repo has no project store wired up (no \`himitsu.yaml\` / no \`default_store\` / slug not registered) — explicit "not configured" instead of a missing chip. The global pill always reflects the active store's git health (synced / behind / dirty / not-recipient / etc.) using the same logic that previously drove the single pill. 3. Lock the wizard's project-slug suggestion to the captured git_root instead of looking up \`current_dir()\` again from a free function. The auto-init bootstrap can run from any cwd and free helpers that re-derive the repo can pick up the wrong origin (or the global default by accident). New \`init::suggested_project_slug_for(&Path)\` takes an explicit root; the wizard passes its stored \`git_root\` so the suggestion is always \`<this repo's origin org>/secrets\`. Tests updated: \`should_launch_init_flow\` now has three cases — no-key, has-key-but-no-stores-registered, and the new "any-store-registered" case (which doesn't fire the wizard even when the active store is empty). 705 tests pass, clippy clean.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bea7323f49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| timestamp: &str, | ||
| ) -> Result<()> { | ||
| std::fs::create_dir_all(data_dir)?; | ||
| std::fs::write(pubkey_path(data_dir), format!("{pubkey}\n"))?; |
There was a problem hiding this comment.
When the provider-specific secret write fails (for example --key-provider macos-keychain on a non-macOS host, or a denied/failing security add-generic-password on macOS), this writes key.pub before the private key is actually stored. Because is_initialized() now treats the presence of key.pub as the initialization probe, subsequent runs skip init and then fail to load a non-existent private key. Store the private key first and only materialize key.pub after success, or remove key.pub on provider errors.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| store_new_key( | ||
| &ProviderChoice::Disk, | ||
| dir.path(), | ||
| "AGE-SECRET-KEY-1ABCDEF", |
There was a problem hiding this comment.
Pull request overview
This PR refines himitsu’s TUI launch + dashboard UX by preventing the init wizard from auto-firing in unrelated repos, splitting “store health” into separate global vs project indicators, and making key loading provider-aware (disk vs macOS Keychain) via a new keystore module.
Changes:
- Adjusts TUI auto-init gating to trigger only when not initialized or when zero stores are registered globally.
- Updates the search/dashboard header to render two health indicators (“global” and “project”), including an explicit “n/a” project state.
- Introduces
crypto::keystoreand threadsContext::key_provider+Context::load_identity()through CLI/TUI decrypt call sites; locks init wizard project-slug suggestion to the captured git root.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/src/tui/views/secret_viewer.rs | Switches decrypt path to Context::load_identity(); updates test context fixture with key_provider. |
| rust/src/tui/views/search.rs | Adds global/project health pills and supporting helpers; switches decrypt to load_identity(); threads key_provider into owned context. |
| rust/src/tui/views/remote_add.rs | Updates test context fixture with key_provider. |
| rust/src/tui/views/new_secret.rs | Updates test context fixture with key_provider. |
| rust/src/tui/views/init_wizard.rs | Prefills project slug using captured git_root via suggested_project_slug_for. |
| rust/src/tui/views/envs.rs | Adds Ctrl+W save chord; ensures cloned contexts copy key_provider; updates help + tests. |
| rust/src/tui/mod.rs | Updates init-flow boot context to carry key_provider; changes should_launch_init_flow logic to use keystore init probe + registered-stores scan. |
| rust/src/tui/harness.rs | Updates test context fixture with key_provider. |
| rust/src/tui/app.rs | Ensures cloned contexts copy key_provider. |
| rust/src/crypto/mod.rs | Exposes new crypto::keystore module. |
| rust/src/crypto/keystore.rs | New provider-aware key persistence + initialization probe + migration helpers. |
| rust/src/cli/tag.rs | Switches decrypt path to Context::load_identity(). |
| rust/src/cli/sync.rs | Ensures per-store sync contexts preserve key_provider. |
| rust/src/cli/search.rs | Uses Context::load_identity().ok() for optional metadata decrypt. |
| rust/src/cli/schema.rs | Updates test context fixture with key_provider. |
| rust/src/cli/rekey.rs | Switches decrypt path to Context::load_identity(). |
| rust/src/cli/recipient.rs | Updates test context fixture with key_provider. |
| rust/src/cli/mod.rs | Adds Context.key_provider + load_identity(); updates “initialized” probe to keystore pubkey check; loads provider from config for dispatcher paths. |
| rust/src/cli/ls.rs | Uses Context::load_identity().ok() when tag metadata requires identity. |
| rust/src/cli/join.rs | Updates test context fixture with key_provider. |
| rust/src/cli/init.rs | Reorders init to settle key provider before writing key material; adds disk→keychain migration; adds suggested_project_slug_for(&Path). |
| rust/src/cli/import.rs | Updates test context fixtures with key_provider. |
| rust/src/cli/get.rs | Switches decrypt path to Context::load_identity(). |
| rust/src/cli/generate.rs | Switches decrypt path to Context::load_identity(). |
| rust/src/cli/export.rs | Switches decrypt path to Context::load_identity(). |
| rust/src/cli/exec.rs | Switches decrypt path to Context::load_identity(); removes now-unused age import. |
| rust/src/cli/codegen.rs | Updates test context fixtures with key_provider. |
| README.md | Expands key_provider documentation with keychain semantics + migration behavior. |
| .beads/issues.jsonl | Updates local issue tracker entries (metadata/content changes only). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1206
to
+1213
| fn check_store_health_pair(ctx: &Context) -> (StoreHealth, Option<StoreHealth>) { | ||
| let global_health = check_store_health(ctx); | ||
|
|
||
| let project_health = match resolve_project_store(ctx) { | ||
| Some(project_store) => { | ||
| let mut project_ctx = ctx.clone(); | ||
| project_ctx.store = project_store; | ||
| Some(check_store_health(&project_ctx)) |
Comment on lines
+1273
to
+1275
| fn span_width(spans: &[Span<'_>]) -> usize { | ||
| spans.iter().map(|s| s.content.chars().count()).sum() | ||
| } |
Comment on lines
+60
to
67
| /// Path to the age private key file. Only valid for the | ||
| /// [`Disk`](crate::config::KeyProvider::Disk) provider — with the | ||
| /// keychain provider this path doesn't exist, so callers should | ||
| /// reach the secret through [`Self::load_identity`] instead of | ||
| /// reading the path directly. | ||
| pub fn key_path(&self) -> PathBuf { | ||
| self.data_dir.join("key") | ||
| crate::crypto::keystore::disk_secret_path(&self.data_dir) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related fixes to the TUI launch flow:
1. Stop auto-firing init in every project
Before: `should_launch_init_flow` returned true whenever the resolved active store was empty — so `cd`-ing into any repo without its own himitsu store bounced you into the setup wizard every single time you ran `himitsu`.
After: the wizard fires only when himitsu isn't initialized at all OR the user has zero registered stores under `stores_dir`. A project without a store of its own now just lands on the dashboard with the project pill grayed out.
2. Two health pills in the dashboard header
Before: one pill for "the active store's health" — single source, conflated global vs project context.
After: `global: ` + `project: `, side-by-side. The project pill is muted gray `project: n/a` when the current repo has no `himitsu.yaml` / no `default_store` / the slug isn't registered. Explicit "not configured" instead of an absent chip.
3. Lock project-slug prefill to the captured git_root
Before: `init::suggested_project_slug()` re-derived the repo from `current_dir()` every time. The auto-init bootstrap can run from any cwd and pick up the wrong origin — in your case, the wizard's project field prefilled with the global default slug instead of the project repo's slug.
After: new `init::suggested_project_slug_for( & Path)` takes an explicit git_root. The wizard captures the repo at construction time and passes its stored `git_root` to the helper, so the suggestion is locked to `<this repo's origin org>/secrets` and can't drift.
Test plan
Depends on
This branch builds on #18 (keychain wiring) — it uses `Context.key_provider` in test fixtures. Should rebase cleanly once #18 lands.
Related issues
Filed during the same gap-scan that produced this work: hm-zhm, hm-b50, hm-c0h, hm-3rc, hm-1dh, hm-avf, hm-c0w, hm-pdo, hm-1lm, hm-vcx, hm-vho, hm-5xm. None of those are addressed here — this PR is scoped to the init/dashboard flow only.