refactor: deepen seams from the 2026-06-09 architecture review by czxtm · Pull Request #20 · darkmatter/himitsu

cooper (czxtm) · 2026-06-10T15:15:51Z

What changed

Implements all seven beads from the 2026-06-09 architecture review (design decisions recorded in CONTEXT.md; each bead's close note carries the full spec):

hm-x9r — Context owns project-config resolution. ctx.project_config() is lazy + memoized (clones share the memo; the dispatcher seeds it in project mode). Fixes the latent --project bug class at five sites that still cwd-walked: generate, codegen ×2, export, check, and the TUI health sidebar. The raw loaders are now pub(crate) internals; migrate's multi-root scan is the one sanctioned direct caller.
hm-i9k — OutputResolver (rust/src/cli/output_resolver.rs). One module owns project outputs: map → candidates-with-decrypted-tags → selector/alias resolution → decoded values. open() is the single decrypt-once scan (zero I/O when no outputs are defined); tri-state env_map() replaces exec's ~105-line hand-rolled pipeline; decode() serves generate/codegen, which stop decrypting the store twice. Deletes resolver_candidates_with_tags and SecretResolver::resolve_candidates.
hm-y36 — exec accepts multiple refs. himitsu exec tag:pci prod/db -- cmd injects the union; every ref must match; the same env var resolving to different values across refs is a hard error. The -- separator is now required (all docs/tests already used it). The zsh completion generator is updated for the new positional shape — the existing test caught that the rename would have silently broken fuzzy completion.
hm-x7z — ADR-0001 (docs/adr/). Keep last-wins duplicate-key collapse in the file generators (aliases resolve after selectors, so last-wins = "the alias pins the binding"); codegen gains generate's warning instead of silently clobbering; exec stays strict.
hm-cpc — StoreOps (rust/src/cli/store_ops.rs). The mutation chain (append-only commit on success and failure, push, completions-cache refresh) now lives in one module with two callers: the CLI dispatcher (one finalize per command, so batches stay one commit) and silent TUI mutation cores. Fixes real drift: TUI delete/rekey left the store dirty (no commit at all), join println!'d into ratatui, and TUI sets skipped env-key validation, effective-store routing, and the cache refresh.
hm-dwr — KeyRegistry. One exhaustive-match row per KeyAction (a missing row is a compile error) drives dispatch, help screens (rendered from the live keymap, so rebinds show up), and palette shortcuts. The three hardcoded search chords (Ctrl+Space / Ctrl+T / Ctrl+O) became rebindable actions. The envs keymap field is renamed outputs with a serde alias for existing configs.
hm-a16 — search-view graduation. PathFolding and ResultSort are now pure, terminal-free modules (rust/src/tui/model/); StoreHealth is a drawable widget (rust/src/tui/widgets/store_health.rs). Behavior-identical extraction; all pre-existing view tests pass through the moved code unchanged.

Bonus fixes (found by the new tests)

Completions cache invalidation used seconds-granularity max-mtime and missed deletions landing in the same second as the last write — the fingerprint now hashes nanosecond mtime + entry count (self-healing for existing caches).
Ctrl+Space chords serialized as a literal space and could never roundtrip the whitespace-separated chord config format — now renders as space.
The secret viewer's single-secret rekey ran against the ambient store instead of the secret's own store, and uncommitted.

Reviewer notes

Deliberate behavior deltas: exec without -- (undocumented form) now errors with a clap usage message; codegen warns on duplicate keys (was silent); local undecryptable entries in generate/codegen now produce SecretResolver's rich "no matching key" diagnostic instead of the raw age error; TUI mutations now commit/push/refresh like their CLI equivalents.
Documented deviation: the review originally planned for the Outputs view to adopt PathFolding/ResultSort in this change, but its presumed duplication turned out to be two one-line alphabetical sorts — adoption would have added a hypothetical seam, so it was evaluated and skipped (recorded in CONTEXT.md and the bead close).
demo/demo-vhs.gif was modified by something outside this session and is intentionally not included in this PR.
Gates at HEAD, run twice: 691 lib + 155 integration tests green, clippy 0 warnings.

🤖 Generated with Claude Code

Land the 2026-06-09 review (hm-x9r, hm-i9k, hm-y36, hm-x7z, hm-cpc, hm-dwr, hm-a16): - Context owns project-config resolution (ctx.project_config(), memoized) — fixes five sites that ignored --project (generate, codegen x2, export, check, TUI health) - OutputResolver: one decrypt-once scan behind open(); tri-state env_map makes exec's label path a one-liner; generate/codegen stop decrypting the store twice - exec accepts multiple refs (union; conflicting values hard-error) - ADR-0001: keep last-wins collapse in file generators; codegen gains generate's duplicate-key warning - StoreOps owns the mutation chain (commit/push/completions refresh) for both the CLI dispatcher and TUI cores — TUI delete/rekey no longer leave the store dirty; join no longer prints into ratatui - KeyRegistry: exhaustive-match rows derive dispatch, live help, and palette shortcuts; envs config key renamed outputs (serde alias) - PathFolding/ResultSort/StoreHealth graduate out of the search view as terminal-free modules Also fixes: completions cache missed same-second deletions (mtime fingerprint now nanos + entry count); Ctrl+Space chords could not roundtrip the config format; viewer rekey targeted the ambient store instead of the secret's own. Gates: 691 lib + 155 integration tests, clippy clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

CI gates on `cargo fmt -- --check`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR applies the 2026-06-09 architecture review outcomes by deepening core seams in the Rust CLI and TUI: Context-owned/memoized project-config resolution, a unified OutputResolver pipeline for outputs→env/value materialization, centralized StoreOps mutation chaining, and a KeyRegistry-driven keybinding/help/palette system. It also extracts search-view state and store-health logic into reusable modules, and adds integration tests to lock in the new behaviors.

Changes:

Centralizes project config resolution in Context::project_config() (lazy, memoized across Context clones) and updates commands/views to use it.
Introduces OutputResolver for a single decrypt-once scan and consistent outputs label / selector / alias resolution across exec, generate, and codegen (including new exec multi-ref union behavior).
Adds StoreOps mutation cores + KeyRegistry, and graduates PathFolding/ResultSort/StoreHealth into dedicated modules with updated TUI help/palette integration.

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/integration/cli_test.rs	Adds integration coverage for `--project` resolution, `exec` multi-ref semantics, and codegen duplicate-key warnings.
rust/src/tui/widgets/store_health.rs	New reusable StoreHealth widget + project-store resolution via `ctx.project_config()`.
rust/src/tui/widgets/mod.rs	Exposes the new `store_health` widget module.
rust/src/tui/views/secret_viewer.rs	Routes rekey/delete/set mutations through `store_ops` and scopes mutations to the viewed secret’s store; live-keymap help entries.
rust/src/tui/views/search.rs	Extracts folding/sorting/health into modules; makes more actions keymap-driven; uses `store_ops` for join/rekey.
rust/src/tui/views/remote_add.rs	Updates test contexts to include `project_config_cell`.
rust/src/tui/views/recipient_list.rs	Switches TUI recipient mutations to `store_ops`; updates tests for new context field.
rust/src/tui/views/recipient_add.rs	Switches recipient add to `store_ops`; updates tests for new context field.
rust/src/tui/views/outputs.rs	Ensures cloned contexts carry `project_config_cell`; updates tests.
rust/src/tui/views/new_secret.rs	Switches secret creation to `store_ops::set_secret`; live-keymap help entries.
rust/src/tui/views/help.rs	Makes HelpView accept owned dynamic rows (enables live keymap-derived help).
rust/src/tui/views/command_palette.rs	Derives palette shortcut/action linkage via KeyRegistry rows.
rust/src/tui/model/result_sort.rs	New pure sorting module extracted from Search view, with unit tests.
rust/src/tui/model/path_folding.rs	New pure folding/grouping module extracted from Search view, with unit tests.
rust/src/tui/model/mod.rs	Introduces `tui::model` module namespace for pure state modules.
rust/src/tui/mod.rs	Wires in `tui::model`; updates contexts in init flow/tests for new config memo cell.
rust/src/tui/keymap.rs	Adds KeyRegistry (rows/scopes/palette links), new search actions, `envs`→`outputs` rename w/ serde alias, and `space` roundtripping.
rust/src/tui/harness.rs	Updates TUI harness test contexts to include `project_config_cell`.
rust/src/tui/app.rs	Builds help overlays from live keymap-derived rows; ensures cloned contexts carry memo cell.
rust/src/config/mod.rs	Makes raw project-config loaders `pub(crate)` and documents Context ownership of selection/memoization.
rust/src/completions_cache.rs	Improves cache invalidation fingerprinting (nanosecond mtime + entry count hashing).
rust/src/cli/sync.rs	Ensures derived contexts carry `project_config_cell`.
rust/src/cli/store_ops.rs	New StoreOps mutation seam: shared finalize chain and silent mutation cores with tests.
rust/src/cli/set.rs	Exposes `encrypt_and_write` to sibling modules (used by StoreOps).
rust/src/cli/search.rs	Adds `Default` for `SearchResult` to support new pure-model test helpers.
rust/src/cli/schema.rs	Updates test contexts to include `project_config_cell`.
rust/src/cli/resolver.rs	Removes now-redundant candidate-building helper from SecretResolver.
rust/src/cli/recipient.rs	Removes TUI-facing wrappers; exposes silent cores to `store_ops`; updates tests accordingly.
rust/src/cli/output_resolver.rs	Adds OutputResolver (open/env_map/decode) with invariants and extensive unit tests.
rust/src/cli/mod.rs	Adds `Context::project_config()` w/ shared memo cell; moves mutation finalization to `store_ops`; seeds memo during construction.
rust/src/cli/join.rs	Splits join into silent core + presentation wrapper; returns structured outcomes.
rust/src/cli/init.rs	Ensures derived contexts carry `project_config_cell`.
rust/src/cli/import.rs	Updates test contexts to include `project_config_cell`.
rust/src/cli/get.rs	Extracts channel-free `expiry_message` helper used by OutputResolver.
rust/src/cli/generate.rs	Uses OutputResolver for outputs resolution and value decode; keeps last-wins + warning behavior.
rust/src/cli/export.rs	Switches project config loading to `ctx.project_config()`.
rust/src/cli/exec.rs	Adds multi-ref union semantics, requires `--`, integrates OutputResolver tri-state label resolution.
rust/src/cli/doctor.rs	Updates test contexts to include `project_config_cell`.
rust/src/cli/completions.rs	Updates zsh completion patching for new `exec` positional `refs` shape.
rust/src/cli/codegen.rs	Uses OutputResolver for outputs resolution + decode; adds duplicate-key warning parity with generate.
rust/src/cli/check.rs	Uses `ctx.project_config()` for store discovery.
README.md	Updates `exec` docs for multi-ref usage and keymap action list changes.
docs/ARCHITECTURE.md	Documents `exec` multi-ref union/conflict semantics.
docs/adr/0001-keep-last-wins-collapse-in-file-generators.md	Adds ADR formalizing last-wins behavior with warning parity.
CONTEXT.md	Records architecture terms and the deepened seams introduced by this PR.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The old candidate builder used load_identities().unwrap_or_default(), so codegen --lang worked on machines with no age identity at all; open() propagated the error instead. Restore the tolerant scan (no identities -> no tags -> tag selectors match nothing, mirroring ls --tag) and the tolerant store listing. Needing a value still fails hard at env_map/decode time. Caught by CI only: local runs masked the regression via the developer's ~/.config/sops/age fallback keys. New regression test removes the on-disk key explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- HelpView::draw borrows row descriptions instead of cloning per frame - resolve_selector_env doc: ExecEmptyMatch fires only on zero path candidates; post-filter emptiness is the caller's policy Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 45 out of 45 changed files in this pull request and generated 4 comments.

CodeQL jobs failed with 'Requires authentication' while uploading status reports (runner-side token outage, same window as local gh 401s); the run is marked non-retryable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

- store_ops::set_secret: a qualified ref writes to its own store; the mutation chain (commit/push/completions refresh) now targets that effective store instead of leaving it dirty and refreshing the ambient cache - store_health: the project-store check no longer inherits the ACTIVE store's recipients_path override (false NotRecipient); it resolves store-internal config first, then the project config override, mirroring the dispatcher Both from Copilot review on PR #20; the second is pre-existing behavior surfaced by the widget extraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.

    let secrets_dir = store.join(".himitsu").join("secrets");
    let current_mtime = max_mtime_recursive(&secrets_dir);

-    // Check whether the tree has changed since the last cache build.
-    let stored_mtime: i64 = conn
+    // Check whether the tree has changed since the last cache build. The
+    // stored value is the bit-cast fingerprint; a missing row means "never


+/// Shared memo cell for the lazily-loaded project config. `Default` yields
+/// an empty cell; `Context` clones share one cell, so the config loads (and
+/// the legacy `envs:` migration warning fires) at most once per process.


Copilot AI review requested due to automatic review settings June 10, 2026 15:15

Copilot started reviewing on behalf of cooper (czxtm) June 10, 2026 15:16 View session

style: apply rustfmt

75f315e

CI gates on `cargo fmt -- --check`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI reviewed Jun 10, 2026

View reviewed changes

Comment thread rust/src/tui/views/help.rs Outdated

Comment thread rust/src/cli/exec.rs Outdated

Copilot AI review requested due to automatic review settings June 10, 2026 15:25

Copilot started reviewing on behalf of cooper (czxtm) June 10, 2026 15:25 View session

Copilot AI reviewed Jun 10, 2026

View reviewed changes

Comment thread rust/src/tui/keymap.rs

Comment thread rust/src/tui/views/command_palette.rs

Comment thread rust/src/tui/widgets/store_health.rs

Comment thread rust/src/cli/store_ops.rs

Cooper Maruyama and others added 2 commits June 10, 2026 08:32

ci: retrigger checks after GitHub auth incident

a8e21e6

CodeQL jobs failed with 'Requires authentication' while uploading status reports (runner-side token outage, same window as local gh 401s); the run is marked non-retryable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings June 10, 2026 15:36

Copilot started reviewing on behalf of cooper (czxtm) June 10, 2026 15:36 View session

cooper (czxtm) merged commit a17007a into main Jun 10, 2026
11 of 13 checks passed

Copilot AI reviewed Jun 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: deepen seams from the 2026-06-09 architecture review#20

refactor: deepen seams from the 2026-06-09 architecture review#20
cooper (czxtm) merged 6 commits into
mainfrom
refactor/arch-review-deepening

cooper (czxtm) commented Jun 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

cooper (czxtm) commented Jun 10, 2026

What changed

Bonus fixes (found by the new tests)

Reviewer notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants