spec: configure clear command behavior#9353
Conversation
Co-Authored-By: Oz <oz-agent@warp.dev> Co-Authored-By: Zach Lloyd <zachlloyd@users.noreply.github.com>
|
@oz-for-oss[bot] I'm starting a first review of this spec-only pull request. You can follow along in the session on Warp. I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for a configurable clear command behavior that preserves history by default and optionally deletes prior terminal history. The specs cover settings UI, model plumbing, cleanup expectations, validation, and major implementation risks.
Concerns
- The delete-history contract is ambiguous about whether deleted output is removed only from the current rendered view or also from restored-session persistence, shared-session state, AI history/context, telemetry, and logs.
- The primary-screen ANSI
CSI 2Jguidance does not precisely map to the current block-list viewport-gap behavior, which can lead to an implementation that differs from the product spec for non-shell-hook clears. - The design needs deterministic deduplication when a shell hook and ANSI clear sequence arrive for one logical
clear. - Shared-session propagation is underspecified: a local model/view cleanup event is not enough to guarantee viewers observe the sharer's history deletion.
- AI context cleanup should define pruning versus wholesale reset so unrelated user-provided context is not removed accidentally.
Security
- Clarify the retention boundary for deleted terminal output so users and implementers do not treat a visual/session-view deletion as erasure from persistence, shared-session transport, AI conversations, telemetry, or logs unless the implementation actually guarantees that.
Verdict
Found: 0 critical, 4 important, 1 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| 6. **Delete session history** mode removes session history that appeared before the clear request: | ||
| - After `clear` completes, scrolling to the top of the current terminal view lands at the current prompt or the first output produced after the clear. | ||
| - Blocks that were above the clear point are no longer visible, selectable, searchable, copyable from the block list, or attachable as AI context in that terminal view. |
There was a problem hiding this comment.
Delete session history is framed as removing prior output, but the spec only scopes that to the current terminal view; explicitly state whether deleted blocks are purged from restored-session persistence, shared-session streams, AI conversation/history state, telemetry, and logs, or label the setting as visual-only deletion.
| This helper owns the state cleanup required by product invariant 13: | ||
|
|
||
| - clear selected blocks and block-list text selection | ||
| - reset or prune AI context references to deleted blocks |
There was a problem hiding this comment.
💡 [SUGGESTION] Define whether cleanup should prune only AI context entries that reference removed blocks or reset all AI context; the product invariant only requires removed-block references to disappear, and over-resetting could drop unrelated user-added context.
|
|
||
| - Add a `clear_primary_screen_all()` helper on `TerminalModel` or `BlockList` that is used when the active surface is the primary block list and `ClearMode::All` is received. | ||
| - In `TerminalModel::clear_screen(ClearMode::All)`, preserve current alternate-screen behavior. For the primary block list: | ||
| - `PreserveSessionHistory`: keep `delegate!(self.clear_screen(ClearMode::All))`, preserving today's viewport-gap behavior. |
There was a problem hiding this comment.
delegate!(self.clear_screen(ClearMode::All)) does not document the block-list viewport-gap primitive and currently routes to the grid clear path, so preserve mode may not match the shell-hook behavior promised in the product spec; name the exact helper to call for primary block-list CSI 2J in each mode.
|
|
||
| Do not alter `ClearMode::Saved`; it already represents an explicit saved-history deletion request and should remain independent of the new setting. | ||
|
|
||
| If implementation finds that shell-integrated `clear` always emits `ClearValue` before the ANSI sequence, guard against duplicate handling by ensuring a single clear request produces one history deletion. A short-lived parser/model flag for "handled clear hook during this byte-processing batch" is preferable to allowing two consecutive `ResetAndClear` passes. |
There was a problem hiding this comment.
CSI 2J, so require one logical clear to produce exactly one history deletion/model event and add the corresponding test.
|
|
||
| ### 7. Shared sessions and restored sessions | ||
|
|
||
| For shared sessions, the sharer's model mutation should determine what viewers see. Do not re-interpret the clear behavior on a viewer using the viewer's local setting; viewers should receive the resulting ordered terminal/model events and render the already-preserved or already-deleted history. |
There was a problem hiding this comment.
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for a Terminal setting that preserves Warp's current clear behavior by default and lets users opt into deleting session history for shell-originated clear requests.
Concerns
- The shared-session design is underspecified: current shared sessions replay PTY bytes to viewers, so the viewer model could apply its own clear setting instead of the sharer's chosen mutation, which conflicts with the product requirement and can leave supposedly deleted terminal history visible to viewers.
Security
- Shared-session propagation needs an explicit protocol/model mechanism so delete-history clear requests do not depend on viewer-local settings.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
|
||
| ### 7. Shared sessions and restored sessions | ||
|
|
||
| For shared sessions, the sharer's model mutation should determine what viewers see. Do not re-interpret the clear behavior on a viewer using the viewer's local setting; viewers should receive the resulting ordered terminal/model events and render the already-preserved or already-deleted history. |
There was a problem hiding this comment.
DeleteSessionHistory actually removes prior blocks for viewers and viewer-local settings cannot diverge.
1 participant
Summary
clearcommand behavior setting.Related issue: #9216