report-fields.md

Reassessment of field-level RefCell grouping

Analysis baseline: df5f6e5f9ff3a2ed83aa52c53d16ac3f71b0ce5d.

Goal: find coherent state groups that reduce RefCell storage and simplify borrowing. This replaces the report against 1e08b793f; already consolidated fields are no longer recommendations for new storage reductions.

This is a source-review report, not an execution plan or a completed safety proof. The reassessment inventoried RefCell type occurrences throughout src/ and examined declarations and selected access paths in client, window, pane, session, server, terminal, overlay, and getopt code. The inventory also includes aliases, accessors, shared handles, and tests; its occurrence count is not a count of independently removable cells. No runtime tracing or tests were run for this documentation change. References below use paths and function names so they remain useful when line numbers move.

Findings and priorities

A useful group has a common state machine or publication boundary and compatible access lifetimes. Its members need not always change together. Different updates should remain different operations on the group.

Priority	Proposed group	Existing RefCells → proposed	Assessment
1	Getopt parser state, nine fields	9 → 0 with Cell<GetoptState>; 9 → 1 with a borrowed parser state	Strong semantic group; largest reduction; requires accessor/helper refactor
2	Attached-client exit state, six fields	6 → 1	Strong group; preserve partial updates and send ordering
3	Client overlay descriptor, check, owned state	3 → 1	Strong group after replacing the escaping mutable data accessor
4	Client terminal definition: name, capabilities, enabled/disabled features	4 → 1	Good group with existing field-splitting boundary; narrower feature pair is also viable
5	Pane border-status screen and line metadata	2 → 1	Coherent published rendering result; closure-held read needs validation
6	Pane output positions: parser, pipe, base	2 → 0 with a copyable aggregate	Good shared coordinate system; preserve independent readers and callback phasing

These are alternative designs within each row, not additive savings. Priorities balance semantic confidence and benefit, not implementation size. Completing all six preferred designs would remove 22 field-level RefCells; using one RefCell<GetoptState> instead would remove 21. This is an opportunity estimate, not a measured or approved implementation result.

Already implemented: retain these groups

58fc3b002 consolidated client state and the pending exec request; 4cb566f2a consolidated window modal selection.

Group	Current representation	Reassessment
Click tracking	Cell<ClickState>	Appropriate snapshot/update group; initialization explicitly sets pane id to -1
Pan coordinates	Cell<PanOffsets>	Appropriate coordinate group; leave the weak window anchor separate unless a later lifecycle change needs it
Activity timestamps	Cell<ActivityTimes>	Appropriate; current-only update and input activity shift remain distinct operations
Status-message text, flags, saved screen	RefCell<ClientMessageState>	Coherent group; improve operation boundaries, not cell count
Daemon-side exit payload	RefCell<ClientExitState>	Coherent group; mapped field guards remain, but detach now copies its session before sending
Window current/previous modal pane	RefCell<WindowModalState>	Appropriate narrow selection group; do not automatically absorb active pane, layout, or owning pane collection
Attached-client shell/command request	RefCell<ClientExecRequest>	Appropriate publish/take group; keep separate from daemon-side exit state

Message and daemon exit storage still expose mapped Ref/RefMut accessors in src/client_state.rs. Each such guard borrows the entire group. Their presence is not itself evidence of a bug, but further callers cannot assume independent field borrowing. Prefer install/take/snapshot operations when changing these paths rather than extending the guard surface.

In src/status.rs, status_message_clear takes the saved screen into a local before status_pop_screen; retain that release boundary. Setting with zero delay preserves the old ignore-keys value, and clearing leaves the ignore flags intact. In src/server/client.rs, server_client_check_exit prepares owned detach data before peer.send; the old report's outstanding session-guard concern is already addressed. Neither group needs another storage merge to realize its existing gain.

1. Getopt parser state

Fields in ServerState: bsdopterr, bsdoptind, bsdoptopt, bsdoptreset, optarg, place, nonopt_start, nonopt_end, getopt_posixly_correct.

Evidence: src/compat/getopt_long.rs owns the parser's accessors, getopt_internal, parse_long_options, current_argument, and argument permutation. The scalar fields and both Option<ArgumentPosition> values are copyable (ArgumentPosition already derives Copy). BSDoptarg returns a reference into the supplied argument vector, not into parser storage. This makes Cell<GetoptState> a realistic preferred representation, beyond the previous report's single-RefCell suggestion.

Proposed access design:

• Copy parser state at entry, run internal helpers on &mut GetoptState, and publish it after the internal call returns. Put publication outside the helper containing early returns so error and end-of-options results are saved too.
• Replace the parser-specific Value constants with short snapshot/update accessors. Value<T> currently requires a LocalField<RefCell<T>>; changing declarations alone will not work. Do not convert every unrelated Value user as part of this group.
• Pass state explicitly to long-option handling and current_argument. Internal helpers must not reread the global state while a local copy is active.
• Preserve process-global parser semantics and all initialization values, including the -1 sentinels. External option-index/reset operations still operate between parser calls.

The reviewed external uses in src/tmux.rs and src/tests/test_compat_getopt_long.rs read or set results; they do not retain parser guards. The parser calls environment lookup, diagnostics, and mutates long-option flag references. Check these boundaries for parser reentry before choosing snapshot/writeback: nested mutation would otherwise be overwritten. A single RefCell<GetoptState> borrowed at entry and passed through helpers is a valid fallback when retaining checked exclusive access is preferable. Do not implement helpers by recursively calling global accessors under that borrow.

Validation: existing getopt tests cover resets, permutation, short/long options, missing/ambiguous arguments, flag outputs, and argument positions after storage replacement, including non-UTF-8 values. Exercise early-return publication and repeated parsing, then the hmux gates.

2. Attached-client exit state

Fields in ServerState: client_exitreason, client_exitflag, client_exitval, client_exittype, client_exitsession, client_exitmessage.

Suggested shape: RefCell<AttachedClientExitState>, with reason/status snapshots, owned display-message generation, and short protocol-specific update operations. Keep the six existing values and their valid intermediate combinations; a new mutually exclusive enum is not needed to obtain the reduction.

Evidence in src/client.rs:

• client_dispatch_exit_message updates status only when supplied and records a supplied message plus its reason. An empty payload must not reset other fields.
• client_dispatch_attached publishes detach session/type/reason before sending MSG_EXITING; MSG_EXEC updates the type after publishing its separate request.
• client_dispatch_wait marks the exit flag before client_exit, which may wait for file output to drain. Exit intent and process termination are distinct.
• Signal and connection-loss paths update reason/status independently.
• client_exit_message and the tail of client_main consume the resulting state.

All these currently use Value snapshots and short updates. Grouping is more natural than six cells and does not require strings to escape as guards. Release borrows before peer sends, file operations, signals, execution, and process exit. Preserve the actual ordering of each branch: the attached MSG_SHUTDOWN branch currently sends before updating reason/status; do not reorder it merely to match the detach branch.

Keep client_exec as its already coherent request object initially. Keep client_attached, client_suspended, transport handles, file ownership, and flags outside the exit group. The daemon's ClientExitState belongs to another context and must remain distinct.

Validation: use src/tests/test_coverage_client.rs to exercise attached and waiting dispatch, optional exit payloads, detach variants, shutdown, connection loss, and pending exec publication/take. Include exit after output drains.

3. Client overlay state

Fields: ClientOverlayState::{overlay, check, data} in src/client_state.rs. Proposed shape: one cell containing descriptor, effective check, and owned state. Keep message restoration and the overlay timer outside it.

set_overlay, take_overlay, and set_overlay_view already express the shared transitions. take_overlay returns owned state so free callbacks and destruction can happen after the client borrow ends. The production uses of overlay_data() found by this review are popup_modify and popup_write in src/overlay/popup.rs; both immediately obtain a cloned popup owner, then borrow the popup separately. The menu-focused test also obtains an owned menu handle. Replace the escaping RefMut<OverlayState> API with owned popup/menu snapshots.

There is an additional nested-borrow boundary: OverlayState::view_data in src/types.rs borrows a popup to obtain its menu for PopupMenu. Prefer taking the view tag and cloned popup handle under the client borrow, then looking up the popup's menu after releasing it. This avoids extending client borrowing into the popup object. Installation/replacement should similarly return displaced owned state for destruction outside the group borrow.

Do not derive check solely from overlay. popup_write temporarily selects Nothing around parsing, then restores Popup; menu views also change the check independently of the descriptor. clear_overlay_view currently changes data's view without changing check; preserve that distinction.

Validation: popup writes while hidden, nested menu views, replacement, clear/free callbacks, and retained popup/menu handles. Existing menu-focused tests are a useful starting point; they do not by themselves prove all callback lifetimes.

4. Client terminal definition

Fields in ClientValues: term_name, term_caps, term_features, term_nofeatures. Suggested shape: RefCell<ClientTerminalDefinition>.

This is a stronger grouping than collecting all client identification strings. ClientRef::with_terminal_definition currently borrows precisely these four fields: two immutably, two mutably, then calls a closure. A single aggregate borrow can split them into ordinary field references without cloning the name or capability vector. parse_terminal_features updates the enabled/disabled pair; default_terminal_features respects the disabled mask; identification messages populate name/capabilities/features independently.

The principal production closure is tty_open → tty_term_create in src/tty/driver.rs and src/terminfo/term.rs. Terminal creation reads the client's environment and client name, plus global options, while definition fields remain borrowed. These must stay outside the new cell, as must the TTY and client flags. The existing terminal_definition_callback_can_reenter_independent_resources test explicitly exercises TTY and flag access under the definition borrow.

A narrower RefCell<TerminalFeatureMasks> holding only enabled/disabled bits is also sound in shape and saves one cell instead of three. Start there if callers need the existing independence of name/capabilities. Although the masks are copyable, switching directly to Cell would change the callback API's checked exclusive-mutation behavior; do not silently replace it with snapshot/writeback. For either scope, split fields inside with_terminal_definition; merely mapping four existing guards into one cell would immediately conflict.

Keep term_type and ttyname separate initially: they have separate response and identity readers and are not part of this construction boundary. Before merging the four-field group, audit every escaping name/capability accessor and the closure callers, including tests. Validation should cover identification, terminal construction success/failure, enabled/disabled feature precedence, and resource reentry through terminal setup.

5. Pane border-status publication

Fields: window_pane::{status_screen, border_status_line}; include status_size (already Cell) as ordinary data if useful. Proposed shape: RefCell<PaneBorderStatus>.

publish_border_status in src/window_pane/traits.rs compares the prior screen and publishes a screen, ranges, expanded text, and width. These form one rendering result. Publish with one borrow and field assignment; keep screen construction and format expansion outside it. border_status_range returns an owned range.

with_status_screen retains a read borrow while running caller code. screen_redraw_draw_pane_status in src/screen/redraw.rs uses it to call tty_draw_line in src/tty/draw.rs. The inspected entry reads the supplied grid and writes terminal state; overlay range calculation happens before the screen closure. This supports the grouping, but this review did not exhaustively prove all downstream drawing paths incapable of republishing border status. Exercise that boundary and any other screen-closure callers before claiming safety.

Keep base screen, modes, palette, and parser ownership separate. Do not clone a rendered screen merely to avoid a borrow. Validate changed/unchanged screen publication, width/range consistency, mouse range lookup, and redraw under clipping/overlays.

6. Pane output coordinate state

Fields: window_pane::{offset, pipe_offset, base_offset}. The first two are RefCell<RustPaneOutputOffset>; the third is already Cell<usize>. Proposed shape: Cell<PaneOutputPositions> with parser, pipe, and base values. Two separate Cell<RustPaneOutputOffset> conversions are an alternative if the aggregate would add more bookkeeping than useful operations.

The grouping has a concrete shared invariant: parser and pipe positions use the same base for retained output and wraparound. maintain_output in src/window_pane/output.rs finds the oldest consumer and rebases positions. parse_output in src/window_pane.rs and the pipe callback copy positions and write them back around processing. The offset type is already copied by value.

Use field-specific updates and a dedicated rebase operation. Do not equate parser and pipe progress. In particular, do not snapshot the whole group before parsing or a client walk and overwrite it afterward: intervening callbacks could update another member. Snapshot what is needed, perform external work, then reread the group before publishing the one changed position. Preserve the current rule that pipe rebasing depends on an active pipe. Control-client cursors remain in their own owners and must still be rebased as part of wrap handling.

unread_output(&offset.borrow()) currently holds a guard through buffer access; replace it with a position value before consolidating. Keep stream handles, parser, and pipe lifecycle outside the group. Existing tests retention_waits_for_pipe_and_clamps_independent_readers and wrapped_output_rebases_the_parser_without_losing_unread_bytes provide useful coverage. Also exercise active-pipe wrap, control readers, and pipe reopen.

Other reductions and boundaries

Fields/resources	Recommendation and reason
Client written, discarded, redraw	Prefer individual Cell conversions with record/update operations. Three removable RefCells, but independent accounting transitions do not require one group. tty_block_maybe, discard, and redraw paths currently use mutable guards; rewrite those callers.
Client retval and existing exit payload	Plausible one-cell saving, but not a priority group. Audit command/error writers before widening exit borrowing; an independent Cell<c_int> also removes the cell without that coupling.
Other client copyable scalars, flags, progress and theme values	Evaluate snapshot/update APIs individually. Type similarity does not justify one large state cell. Do not count savings until escaping mutation accessors and callbacks are audited.
Entire client storage categories	Reject wholesale grouping: terminal setup already demonstrates reentry into independent resources.
Client TTY, prompt, status	Preserve deliberate simultaneous access (prompt_render_parts, terminal_status_mut) and rendering callback boundaries.
Client theme plus computed colours	Compute formats/options before publishing colours; a shared borrow through computation is inappropriate. Small copyable state may need no RefCell instead.
Client current/last session and pan window	Related handles, but no additional sufficiently audited transition group established here. Keep separate from mutable session operations.
Window active/modal/last-panes and owning panes	Keep the existing narrow modal group. Activation, liveness, notification, and redraw callbacks make a larger selection borrow domain costly.
Window layout, old layout, pane collection	Independent escaping views and layout_and_panes_mut already express access distinctions. No mechanical merge.
Pane base screen, modes, parser	Independent closure-held borrows and parser callbacks; no established safe combined borrow domain.
Pane resize queue/timer	Preparation and timer arming straddle external process-resize delivery. Possible storage grouping only with explicit release/reacquire; low benefit.
Session links, group membership, name, cwd, environment	Links already form a coherent group. Other ownership and escaping-resource lifetimes differ; no new merge established. Timestamps already use Cell.
Server input-buffer limit and character buffer	Configuration and scratch character data are different state, despite similar names.
Global registries, plugin slots/revisions/timer, reactor resources	Do not combine by owner alone; callback and independent-handle lifetimes need dedicated review.
Whole-object Rc/RefCell handles in process, command, modes, options, screens	Not field-level fragmentation; this inventory does not propose combining separate object identities.

Implementation acceptance criteria

For a selected group, enumerate construction, teardown, direct accesses, forwarded traits, tests, and escaping guards. Review same-owner calls and aliased handles, including temporary lifetimes in if let/match. Mapped guards still borrow the whole cell; RefMut::map_split only helps deliberate simultaneous field access, not arbitrary reentry through the owner.

Use operation methods that snapshot, split fields, or take ownership. Release aggregate borrows before callbacks, sends, parsing, drawing that can mutate the owner, and destruction of displaced resources. With Cell, distinguish short read/modify/write from a stale snapshot written back after external work. If runtime tracing is added, include acquisition/drop, mapped/split descendants, take/replace, owner identity, and exercised paths; a conflict-free trace proves only the exercised paths.

For an implementation, format/check only changed Rust files with edition 2024 and the crate configuration, run focused tests for the boundaries above, then make -C .. lint SUT=hmux and make -C .. test SUT=hmux from this checkout. Reject conformance runs unless the reference matches nix/tmux-target.nix (tmux next-3.9). No daemon behavior change is intended; any observable change requires human signoff. This report does not authorize replacing foundational components or adding competing implementations.

For this regeneration, validation is source inspection and git diff --check; there are no Rust changes requiring formatting or runtime gates.