feat(tui): restore composer history in app-server tui (#14945)

## Problem The app-server TUI (`tui_app_server`) lacked composer history support. Pressing Up/Down to recall previous prompts hit a stub that logged a warning and displayed "Not available in app-server TUI yet." New submissions were silently dropped from the shared history file, so nothing persisted for future sessions. ## Mental model Codex maintains a single, append-only history file (`$CODEX_HOME/history.jsonl`) shared across all TUI processes on the same machine. The legacy (in-process) TUI already reads/writes this file through `codex_core::message_history`. The app-server TUI delegates most operations to a separate process over RPC, but history is intentionally *not* an RPC concern — it's a client-local file. This PR makes the app-server TUI access the same history file directly, bypassing the app-server process entirely. The composer's Up/Down navigation and submit-time persistence now follow the same code paths as the legacy TUI, with the only difference being *where* the call is dispatched (locally in `App`, rather than inside `CodexThread`). The branch is rebuilt directly on top of `upstream/main`, so it keeps the existing app-server restore architecture intact. `AppServerStartedThread` still restores transcript history from the server `Thread` snapshot via `thread_snapshot_events`; this PR only adds composer-history support. ## Non-goals - Adding history support to the app-server protocol. History remains client-local. - Changing the on-disk format or location of `history.jsonl`. - Surfacing history I/O errors to the user (failures are logged and silently swallowed, matching the legacy TUI). ## Tradeoffs | Decision | Why | Risk | |----------|-----|------| | Widen `message_history` from `pub(crate)` to `pub` | Avoids duplicating file I/O logic; the module already has a clean, minimal API surface. | Other workspace crates can now call these functions — the contract is no longer crate-private. However, this is consistent with recent precedent: `590cfa617` exposed `mention_syntax` for TUI consumption, `752402c4f` exposed plugin APIs (`PluginsManager`), and `14fcb6645`/`edacbf7b6` widened internal core APIs for other crates. These were all narrow, intentional exposures of specific APIs — not broad "make internals public" moves. `1af2a37ad` even went the other direction, reducing broad re-exports to tighten boundaries. This change follows the same pattern: a small, deliberate API surface (3 functions) rather than a wholesale visibility change. | | Intercept `AddToHistory` / `GetHistoryEntryRequest` in `App` before RPC fallback | Keeps history ops out of the "unsupported op" error path without changing app-server protocol. | This now routes through a single `submit_thread_op` entry point, which is safer than the original duplicated dispatch. The remaining risk is organizational: future thread-op submission paths need to keep using that shared entry point. | | `session_configured_from_thread_response` is now `async` | Needs `await` on `history_metadata()` to populate real `history_log_id` / `history_entry_count`. | Adds an async file-stat + full-file newline scan to the session bootstrap path. The scan is bounded by `history.max_bytes` and matches the legacy TUI's cost profile, but startup latency still scales with file size. | ## Architecture ``` User presses Up User submits a prompt │ │ ▼ ▼ ChatComposerHistory ChatWidget::do_submit_turn navigate_up() encode_history_mentions() │ │ ▼ ▼ AppEvent::CodexOp Op::AddToHistory { text } (GetHistoryEntryRequest) │ │ ▼ ▼ App::try_handle_local_history_op App::try_handle_local_history_op message_history::append_entry() spawn_blocking { │ message_history::lookup() ▼ } $CODEX_HOME/history.jsonl │ ▼ AppEvent::ThreadEvent (GetHistoryEntryResponse) │ ▼ ChatComposerHistory::on_entry_response() ``` ## Observability - `tracing::warn` on `append_entry` failure (includes thread ID). - `tracing::warn` on `spawn_blocking` lookup join error. - `tracing::warn` from `message_history` internals on file-open, lock, or parse failures. ## Tests - `chat_composer_history::tests::navigation_with_async_fetch` — verifies that Up emits `Op::GetHistoryEntryRequest` (was: checked for stub error cell). - `app::tests::history_lookup_response_is_routed_to_requesting_thread` — verifies multi-thread composer recall routes the lookup result back to the originating thread. - `app_server_session::tests::resume_response_relies_on_snapshot_replay_not_initial_messages` — verifies app-server session restore still uses the upstream thread-snapshot path. - `app_server_session::tests::session_configured_populates_history_metadata` — verifies bootstrap sets nonzero `history_log_id` / `history_entry_count` from the shared local history file.
2026-07-01 00:31:56 +08:00 · 2026-03-18 11:54:11 -06:00
parent 580f32ad2a
commit 334164a6f7
9 changed files with 337 additions and 98 deletions
@@ -63,7 +63,7 @@ mod mcp_tool_call;
 mod memories;
 pub mod mention_syntax;
 mod mentions;
-mod message_history;
+pub mod message_history;
 mod model_provider_info;
 pub mod path_utils;
 pub mod personality_migration;
@@ -66,14 +66,22 @@ fn history_filepath(config: &Config) -> PathBuf {
    path
 }

-/// Append a `text` entry associated with `conversation_id` to the history file. Uses
-/// advisory file locking to ensure that concurrent writes do not interleave,
-/// which entails a small amount of blocking I/O internally.
-pub(crate) async fn append_entry(
-    text: &str,
-    conversation_id: &ThreadId,
-    config: &Config,
-) -> Result<()> {
+/// Append a `text` entry associated with `conversation_id` to the history file.
+///
+/// Uses advisory file locking (`File::try_lock`) with a retry loop to ensure
+/// concurrent writes from multiple TUI processes do not interleave. The lock
+/// acquisition and write are performed inside `spawn_blocking` so the caller's
+/// async runtime is not blocked.
+///
+/// The entry is silently skipped when `config.history.persistence` is
+/// [`HistoryPersistence::None`].
+///
+/// # Errors
+///
+/// Returns an I/O error if the history file cannot be opened/created, the
+/// system clock is before the Unix epoch, or the exclusive lock cannot be
+/// acquired after [`MAX_RETRIES`] attempts.
+pub async fn append_entry(text: &str, conversation_id: &ThreadId, config: &Config) -> Result<()> {
    match config.history.persistence {
        HistoryPersistence::SaveAll => {
            // Save everything: proceed.
@@ -243,22 +251,29 @@ fn trim_target_bytes(max_bytes: u64, newest_entry_len: u64) -> u64 {
    soft_cap_bytes.max(newest_entry_len)
 }

-/// Asynchronously fetch the history file's *identifier* (inode on Unix) and
-/// the current number of entries by counting newline characters.
-pub(crate) async fn history_metadata(config: &Config) -> (u64, usize) {
+/// Asynchronously fetch the history file's *identifier* and current entry count.
+///
+/// The identifier is the file's inode on Unix or creation time on Windows.
+/// The entry count is derived by counting newline bytes in the file. Returns
+/// `(0, 0)` when the file does not exist or its metadata cannot be read. If
+/// metadata succeeds but the file cannot be opened or scanned, returns
+/// `(log_id, 0)` so callers can still detect that a history file exists.
+pub async fn history_metadata(config: &Config) -> (u64, usize) {
    let path = history_filepath(config);
    history_metadata_for_file(&path).await
 }

-/// Given a `log_id` (on Unix this is the file's inode number,
-/// on Windows this is the file's creation time) and a zero-based
-/// `offset`, return the corresponding `HistoryEntry` if the identifier matches
-/// the current history file **and** the requested offset exists. Any I/O or
-/// parsing errors are logged and result in `None`.
+/// Look up a single history entry by file identity and zero-based offset.
 ///
-/// Note this function is not async because it uses a sync advisory file
-/// locking API.
-pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<HistoryEntry> {
+/// Returns `Some(entry)` when the current history file's identifier (inode on
+/// Unix, creation time on Windows) matches `log_id` **and** a valid JSON
+/// record exists at `offset`. Returns `None` on any mismatch, I/O error, or
+/// parse failure, all of which are logged at `warn` level.
+///
+/// This function is synchronous because it acquires a shared advisory file lock
+/// via `File::try_lock_shared`. Callers on an async runtime should wrap it in
+/// `spawn_blocking`.
+pub fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<HistoryEntry> {
    let path = history_filepath(config);
    lookup_history_entry(&path, log_id, offset)
 }