Skip to content

refactor(tui): one palette, one settings save path, per-project prompt history - #314

Merged
Max17190 merged 1 commit into
mainfrom
trim-frontend-accretion
Sep 7, 2026
Merged

Max17190 merged 1 commit into
mainfrom
trim-frontend-accretion

Conversation

@Max17190

@Max17190 Max17190 commented Sep 7, 2026 •

Copy link
Copy Markdown
Owner

Why

The shipped shape carried frontend accretion that a user had to understand without benefiting from it, two mechanisms for one job, a cross-project leak, and three self-descriptions that no longer matched the code. Each item below was checked against the code before it was changed.

Summary

  • /theme and its three color palettes are removed. The shipped default was monochrome, no theme was persisted (settings.json has no field for one), and the command was undocumented, so a chosen palette lived exactly one process. Tokens are now constants: no palette store, no lock, no init call.
  • One settings save path. The core's save_settings fingerprints the exact bytes it writes under its lock; the model picker used a TUI-side config::save followed by adopt_saved_settings, which fingerprinted a re-serialization. The picker now uses the core's path like /provider does, the adoption shim is deleted, and config::save is gone, so no future caller can write settings.json past the fingerprint. The receipt test asserts the same drift detection through the single path.
  • Prompt history is per project. It was one global file, so one project's prompts surfaced in another project's composer through Up and Ctrl+R. It is now keyed by the canonical project root like every other per-project store, through a shared ledger::project_key. That key now hashes the path's bytes rather than a lossy UTF-8 rendering, so two roots differing only in invalid bytes no longer share a key (a pre-existing collision the ledger directory had too); valid UTF-8 paths keep their existing keys, and a regression test pins the distinction.
  • The empty-session "READY" widget was a module with two tests for one dim word; it is one function beside its only caller.
  • /compact and /export were shipped but absent from the command table; they are documented.
  • Three self-descriptions corrected. The --run-examples help still described the pre-sandbox refusal and omitted that auto runs a valid tool on the host without content approval, under the same permission rules and pre_tool_use hooks as a turn. The ledger module doc claimed hash chaining makes tampering through bash detectable; it detects a torn write or a naive edit, not a rewrite of log and pin together. Recall's doc listed session titles as a searched source when collect_chunks deliberately does not search them.

Test Plan

  • cargo test --workspace --locked: 940 passed, 0 failed, 11 ignored (the four removed tests were the palette-distinctness test, the two "READY" size tests, and a failure-path test for a helper that no longer performs I/O).
  • cargo +1.97.0 clippy --workspace --all-targets --locked -- -D warnings: clean.
  • git grep for every removed identifier (ThemeId, set_tokens, detect_color_level, adopt_saved_settings, save_model_selection, invalidate_styles, the ready module) returns nothing.

Greptile Summary

This PR simplifies the TUI palette and empty-session rendering, consolidates settings persistence through the core, scopes prompt history by canonical project root, and corrects user-facing documentation.

The project-specific prompt-history key now preserves raw Unix path bytes, preventing collisions between roots with distinct invalid UTF-8 sequences. The --run-examples help text now accurately explains that permission rules and pre-tool-use hooks apply.

Confidence Score: 5/5

Safe to merge.

The previously reported prompt-history collision is fully fixed because the current project-key implementation hashes canonical Unix path bytes. The help-text thread was resolved by greptile-apps[bot] without explanation; the current help text describes the applicable permission rules and pre-tool-use hooks.

Files Needing Attention: None.

Reviews (2): Last reviewed commit: "refactor(tui): one palette, one settings..." | Re-trigger Greptile

@Max17190
Max17190 force-pushed the trim-frontend-accretion branch from 7354d60 to 92864e7 Compare September 7, 2026 15:37
Comment thread crates/tui/src/input.rs
// surface in another project's composer.
let history_path = data_dir
.join("history")
.join(format!("{}.json", open_max_core::ledger::project_key(project_root)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 security Preserve Unix Path Bytes

If two Unix project roots contain different invalid UTF-8 byte sequences, the project key converts both paths to the same lossy string before hashing. Those projects then use one prompt-history file, allowing prompts entered in one project to be recalled from the other. This must be fixed before merging.

How this was verified: Two directories differing only by invalid path bytes produced the same history path, and the second recalled a prompt written by the first.

Artifacts

Evidence from the check

  • Authored Rust validation constructs two project roots with distinct invalid UTF-8 bytes and evaluates the exact history-path expression used by the TUI, demonstrating the collision condition.

Command output from the check

  • Executed control run shows distinct UTF-8 project directories have distinct keys and history paths, so the second project cannot read the first project's prompt.

Command output from the check

  • Executed non-UTF-8 run shows byte-distinct project paths produce equal keys and history paths, and the second project reads the first project's prompt, confirming the disclosure.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/tui/src/input.rs
Line: 139

Comment:
**Preserve Unix Path Bytes**

If two Unix project roots contain different invalid UTF-8 byte sequences, the project key converts both paths to the same lossy string before hashing. Those projects then use one prompt-history file, allowing prompts entered in one project to be recalled from the other. This must be fixed before merging.

**How this was verified:** Two directories differing only by invalid path bytes produced the same history path, and the second recalled a prompt written by the first.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment thread crates/tui/src/main.rs Outdated
…t history

- `/theme` and its three color palettes are removed. The shipped default
  was monochrome, no theme was persisted (settings.json has no field for
  one), and the command was undocumented, so a chosen palette lived
  exactly one process. Tokens are now constants: no palette store, no
  lock, no init call.
- Settings had two save paths: the core's `save_settings`, which
  fingerprints the exact bytes it writes under its lock, and a TUI-side
  `config::save` followed by `adopt_saved_settings`, which fingerprinted
  a re-serialization. The model picker now uses the core's path like
  `/provider` does, the adoption shim and `config::save` are deleted so
  no future caller can write settings.json past the fingerprint, and the
  receipt test asserts the same drift detection through the single path.
- Prompt history was one global file, so one project's prompts surfaced
  in another project's composer through Up and Ctrl+R. It is now keyed
  by the canonical project root like every other per-project store,
  through a shared `ledger::project_key`.
- The empty-session "READY" widget was a module with two tests for one
  dim word; it is one function beside its only caller.
- `/compact` and `/export` were shipped but absent from the command
  table; they are documented.
- Three self-descriptions said things the code does not do. The
  `--run-examples` help described the pre-sandbox refusal and omitted
  that auto runs every valid tool on the host. The ledger module doc
  claimed hash chaining makes tampering through bash detectable; it
  detects a torn write or a naive edit, not a rewrite of log and pin
  together. Recall's doc listed session titles as a searched source
  when `collect_chunks` deliberately does not search them. Each now
  states the mechanism precisely.
@Max17190
Max17190 force-pushed the trim-frontend-accretion branch from 92864e7 to 9188396 Compare September 7, 2026 15:50
@Max17190
Max17190 merged commit edcc52f into main Sep 7, 2026
11 checks passed
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