Skip to content

TEMP diagnostics: PHANTOM_DIAG on the candidate-set matcher (do not merge) - #15324

Draft
warp-agent-staging[bot] wants to merge 21 commits into
masterfrom
factory/native-shell-completions-phantom-block-diagnostics-r4
Draft

TEMP diagnostics: PHANTOM_DIAG on the candidate-set matcher (do not merge)#15324
warp-agent-staging[bot] wants to merge 21 commits into
masterfrom
factory/native-shell-completions-phantom-block-diagnostics-r4

Conversation

@warp-agent-staging

Copy link
Copy Markdown
Contributor

Do not merge. This branch carries temporary PHANTOM_DIAG logging only, for the round-4 investigation of the Tab-triggered stray block on factory/native-shell-completions-generator. It changes no behavior.

What this is

The round-3 diagnostics commit (68aa3406) was based on an older head, before the expected-echo matcher was rewritten to track a set of candidate positions. This branch ports that logging onto the current head (74bcfcae) and adapts it: every line now reports the live expected_echo_positions set rather than a single queue position.

It also adds two instruments the original diagnostics did not have, and those two are the ones that decided the round:

  • backspace() logging in early_output.rs. A literal backspace rewind is invisible to the matcher — backspace() delegates straight to the background block and never touches the candidate set — so without this line it leaves no trace at all.
  • A raw PTY byte dump in TerminalModel::on_finish_byte_processing, escaped with escape_ascii(). This is what makes the line editor's rewind shape readable.

What it found

With ForceNativeShellCompletions, zsh 5.9, starship's zsh completions, and completions_open_while_typing = false:

  • 26 Tab-triggered requests total. With Warp drawing the prompt (honor_ps1 = false), 16 requests produced zero backspaces, zero input() fall-throughs and zero background blocks; the redraw shape was s \r starship pr, and the carriage-return rearm consumed all of it.
  • With the shell drawing the prompt (honor_ps1 = true), all 10 requests leaked, every one of them with exactly one \x08 in the restore window. On a minimal .zshrc the redraw is s \x08 starship pr and exactly one character falls through — the stray block holds a single s.
  • Adding zsh-syntax-highlighting adds a second redraw pass that rewinds with CUB (\x1b[11D) rather than a carriage return, so the 8 characters of the recoloured command word fall through too, on top of the backspace leak.

Both gaps are in the same family as the existing carriage-return rearm: maybe_rearm_expected_echo is reached only from carriage_return(), so a rewind expressed as a backspace or as a cursor-left is not treated as a rewind at all.

Full per-character log windows for all three shapes are attached to the conversation linked below.

Computer-use video recordings

Recording of typing 'starship pr' then Tab (Case A), and typing 'starship p' Tab then 'r' Tab (Case B) in Warp terminal to observe completion behavior.
Warp Tab-completion Case A and B: Recording of typing 'starship pr' then Tab (Case A), and typing 'starship p' Tab then 'r' Tab (Case B) in Warp terminal to observe completion behavior.

Computer-use screenshots (10)

Stage 4, THE TEST repetition 3: same result again - the persistent 'starship' block (Lines: 96) still shown above the prompt, completions menu displaying preset/print-config/prompt.
Stage 4, THE TEST repetition 3: same result again - the persistent 'starship' block (Lines: 96) still shown above the prompt, completions menu displaying preset/print-config/prompt.

Stage 4 control: same as prior stages - second Tab selects 'preset', autocompletes input to 'starship preset', tooltip shown; persistent 'starship' block still visible above.
Stage 4 control: same as prior stages - second Tab selects 'preset', autocompletes input to 'starship preset', tooltip shown; persistent 'starship' block still visible above.

Stage 5 prompt appearance immediately after relaunch with minimal zshrc (no starship, no plugins) and PS1-honoring settings: prompt reads "5hamt6q19ilm8% " in plain white monospace text, single line, no colors or icons.
Stage 5 prompt appearance immediately after relaunch with minimal zshrc (no starship, no plugins) and PS1-honoring settings: prompt reads "5hamt6q19ilm8% " in plain white monospace text, single line, no colors or icons.

Stage 5, THE TEST repetition 1: after Tab press, a stray block containing exactly one character "s" followed immediately by a filled blue cursor rectangle appears between the previous MARKER_STAGE5 output and the completions menu. The completions menu lists preset, print-config, prompt. The input line at bottom reads "5hamt6q19ilm8% starship pr" unchanged.
Stage 5, THE TEST repetition 1: after Tab press, a stray block containing exactly one character "s" followed immediately by a filled blue cursor rectangle appears between the previous MARKER_STAGE5 output and the completions menu. The completions menu lists preset, print-config, prompt. The input line at bottom reads "5hamt6q19ilm8% starship pr" unchanged.

Stage 5, THE TEST repetition 2: identical result to repetition 1 - stray block with single character "s" plus filled cursor rectangle between output and completions menu; menu shows preset/print-config/prompt; input line reads "5hamt6q19ilm8% starship pr".
Stage 5, THE TEST repetition 2: identical result to repetition 1 - stray block with single character "s" plus filled cursor rectangle between output and completions menu; menu shows preset/print-config/prompt; input line reads "5hamt6q19ilm8% starship pr".

Stage 5, THE TEST repetition 3: identical stray block with single 's' character plus cursor rectangle, same completions menu (preset/print-config/prompt), input line "5hamt6q19ilm8% starship pr" with cursor at end.
Stage 5, THE TEST repetition 3: identical stray block with single 's' character plus cursor rectangle, same completions menu (preset/print-config/prompt), input line "5hamt6q19ilm8% starship pr" with cursor at end.

Stage 6 prompt appearance after relaunch with zshrc-stage2 (starship enabled, no line-editor plugins): two-line starship prompt - line 1 shows "~" (cyan) "on" (white) a branch icon (yellow) and "warp-factory-agent@warp-terraform.iam.gserviceaccount.com" (blue); line 2 shows a red circle icon, "[Docker]" in red, and ">" prompt arrow, with cursor after.
Stage 6 prompt appearance after relaunch with zshrc-stage2 (starship enabled, no line-editor plugins): two-line starship prompt - line 1 shows "~" (cyan) "on" (white) a branch icon (yellow) and "warp-factory-agent@warp-terraform.iam.gserviceaccount.com" (blue); line 2 shows a red circle icon, "[Docker]" in red, and ">" prompt arrow, with cursor after.

Stage 6, THE TEST repetition 1: stray block with single character 's' + cursor rectangle appears between MARKER_STAGE6 output and completions menu; menu lists preset/print-config/prompt; input line reads "[Docker] > starship pr" with starship two-line prompt visible.
Stage 6, THE TEST repetition 1: stray block with single character 's' + cursor rectangle appears between MARKER_STAGE6 output and completions menu; menu lists preset/print-config/prompt; input line reads "[Docker] > starship pr" with starship two-line prompt visible.

Stage 6, THE TEST repetition 2: identical to repetition 1 - stray 's' + cursor block, same completions menu, same input line state.
Stage 6, THE TEST repetition 2: identical to repetition 1 - stray 's' + cursor block, same completions menu, same input line state.

Stage 6, THE TEST repetition 3: identical to repetitions 1 and 2 - stray 's' + cursor block persists, completions menu unchanged, input line unchanged.
Stage 6, THE TEST repetition 3: identical to repetitions 1 and 2 - stray 's' + cursor block persists, completions menu unchanged, input line unchanged.

warp-agent-staging Bot and others added 21 commits August 18, 2026 23:25
Replaces the zsh-only, key-triggered OSC 9280 round trip with a single
generator-shaped client path across zsh, bash, fish, and PowerShell.

- zsh: a new foreground generator (select can't run through the
  backgrounded warp_run_generator_command) chains the user's
  zle-line-init/zle-line-finish, arms a guard flag, and reuses the
  existing compadd shim's OSC 9280 emission. Also fixes CORE-3795 (the
  shim's -d flag lookup missed _describe's clustered -ld).
- bash: complete -p plus bash-completion's lazy loader, synthesizes
  COMP_WORDS/COMP_CWORD/COMP_LINE/COMP_POINT, calls the -F function
  directly, emits COMPREPLY (names only).
- fish: complete -C "<line>", already used elsewhere in the bootstrap
  for executable discovery.
- PowerShell: [System.Management.Automation.CommandCompletion]::CompleteInput.

Removes the client-side trigger state machine
(NativeShellCompletionsState::AwaitingPrompt, SendCompletionsPrompt,
the write-blocking clause, and the Ctrl-Y write) and reuses the
existing OSC 9280 completions wire protocol for all four shells.

CHANGELOG-NONE
Two mechanical fixes found by building and running presubmit, neither of
which the author's 4GB sandbox could surface:

- pty_controller_lifecycle_tests.rs: ShellCompletion doesn't derive
  (E0369). Assert the results are empty instead.
- native_shell_completions.rs: the repo forbids inline #[cfg(test)] test
  modules (script/check_no_inline_test_modules). Move the tests to a
  sibling native_shell_completions_tests.rs included via #[path].
…ORE-3795 regression, empty-input handling

Critical fixes:
- zsh: scope the zle-line-init takeover to the single select iteration
  it drives, capturing/restoring by widget name (zle -A/-N) rather than
  functions[...], so a differently-named bound widget (add-zle-hook-widget,
  used by p10k/zsh-syntax-highlighting/zsh-autosuggestions) survives a
  completion request instead of being silently replaced forever.
- zsh: guarantee the request always terminates independently of the armed
  flag by restoring the widget unconditionally right after select returns,
  and make the capture idempotent against repeat firings.
- pty_controller.rs: RunNativeShellCompletions was missing from the
  is_command check in execute_next_queued_write, so the next queued write
  could be drained straight into a shell mid-select and lost.
- zsh: the CORE-3795 fix regressed -- (i) returns one past the array
  length (not 0) on no match, so the presence guard was always true.
  Reverted to (I) and restricted the search to the leading flags prefix
  so a real -d/-ld completion candidate is never mistaken for the flag.

Robustness fixes:
- zsh/fish/PowerShell: empty decoded line now returns zero matches
  immediately instead of dumping every top-level command/file.
- fish: guard missing/empty hex argument in the decoder (was a stack
  trace landing in-band); drop "command" from printf for macOS \x
  decoding, then drop the now-unneeded "--" since fish's builtin printf
  doesn't treat it as an end-of-options marker.
- PowerShell: decode inside try/finally so the OSC terminator is always
  emitted even if decoding or completion throws; empty/missing hex now
  decodes to '' instead of crashing on GetString(null).
- bash: force IFS for read -ra instead of trusting the session's value;
  COMP_POINT is now a byte count, not a locale-dependent character
  count; COMP_WORDS/COMP_CWORD/COMP_LINE/COMP_POINT/COMP_TYPE/COMP_KEY
  are now local so they never leak into the user's session.
- Removed the already-orphaned ^X/list-choices completions path: it had
  no client-side trigger before this PR, and removing the client's
  ability to answer the 9280;P read turned it from unreachable into a
  hang risk if anything ever did trigger it.

Test fixes:
- Fixed ShellCompletion PartialEq build error (assert is_empty() instead
  of == Vec::new()).
- Rewrote the hex round-trip test to assert the actual per-shell decoder
  contract (lowercase, unseparated, even-length) instead of just
  round-tripping through the hex crate.
- Replaced a self-referential command comparison in the queueing test
  with the exact literal, keeping the shell-type-from-session assertion.

All shell-side fixes re-verified empirically via PTY-driven harnesses.
…d the input line

In-app verification found that typing with native completions on produced a
new visible block per keystroke, truncated/emptied the real input buffer,
and left the session unable to submit real commands afterward (Enter
produced empty blocks, Up-arrow/Ctrl+U were broken). Root-caused to two
separate issues, both fixed here:

1. `is_in_band_command()` only recognized the literal
   "warp_run_generator_command " / "Warp-Run-GeneratorCommand " prefixes
   (with a trailing space). All four native-completions generator function
   names are longer ("warp_run_generator_command_foreground_completions",
   "warp_run_generator_command_native_completions",
   "Warp-Run-GeneratorCommand-NativeCompletions") and never matched, so the
   client classified every completions request as a normal, visible user
   command -- hence one new block per keystroke. Relaxed the check to match
   on the shared prefix alone, the same substring convention the shell
   scripts themselves already use for this exact purpose
   (`_is_warp_generator_command`).

2. Running the generator command as a foreground command necessarily kills
   and replaces the shell's real input buffer (see `bytes_to_execute_command`)
   to type the invocation and press Enter -- required for zsh's `select`
   mechanism, and used uniformly across all four shells. Nothing restored the
   user's actual buffer afterward, so on the very next keystroke the shell's
   buffer was still empty, corrupting every subsequent request (a two-
   character buffer produced a truncated completions request, and the final
   Enter submitted whatever fragments had landed on the empty buffer in the
   meantime). `PtyController` now tracks the buffer_text a native-completions
   request was computed from, and once results come back
   (`ModelEvent::CompletionsFinished`), queues it to the front of the write
   queue so it is written back to the pty verbatim as soon as the line editor
   is active again -- ahead of anything queued in the meantime, including a
   newer completions request for what the user has typed since.

Both fixes are shell-agnostic: the prefix relaxation covers all four
generator names, and the buffer restoration is generic to
`RunNativeShellCompletions`, so this should equally resolve the failure for
bash/fish/PowerShell if they hit the same underlying issue.

Added a focused unit test for `is_in_band_command` covering all four
generator name shapes plus a negative case. Could not verify this live (no
computer-use in this environment); requesting re-verification from the
computer-use pass.
…-verification pass

- PowerShell: split the kill-buffer chord (Alt+2, an ESC-prefixed two-byte
  sequence) into its own pty write. PSReadLine sometimes fails to disambiguate
  it when it arrives concatenated with the command text that follows in a
  single write/read, leaving the buffer unclouded and typing the command text
  literally on top of it. Splitting the write (with no explicit delay needed)
  reliably fixes this, confirmed empirically via a PTY harness.

- Fix the phantom block that appeared when PtyController restored the input
  buffer after a native-completions request: the restore write's echo was
  being treated as unexpected background output. Added
  EarlyOutput::push_expected_echo, which registers characters as expected
  echo regardless of TypeaheadMode (push_user_input only does this for
  InputMatching mode), and made handle_potential_typeahead consume it first.
  PtyController now calls this before writing the restored buffer text back.

- Defensively exclude in-band/generator command blocks from client-side
  history: TerminalModel::restored_block_commands() and
  update_command_history() both now skip is_in_band_command-matching text,
  so a generator command can never leak into the Up-arrow history overlay
  (this also plausibly explains the stray "No such widget `zle-line-init'"
  string reported in phantom blocks, which had identical stale metadata
  across unrelated shells -- consistent with a restored, pre-fix block from
  an earlier test run rather than live output).

- fish: override fish_title (which by default shows the currently-running
  command, truncated, in the window/tab title via its own OSC title-setting
  path, independent of Warp's own hooks) to fall back to its own
  "just show pwd" behavior for generator commands, matching upstream's
  format otherwise. Verified with the installed fish for a real command, a
  generator command, fish's own builtin case, and no argv.
…usion

- Fixed the regression from e44204c where typing duplicated the buffer
  (g -> gigi -> gigitgigit, compounding, sometimes CPU-pinning the app).
  Root cause: push_expected_echo populated the same unmatched_input queue
  push_user_input uses, so a restore write's echo was surfaced as genuine
  typeahead via TerminalEvent::Typeahead -> insert_typeahead_text. Real
  typeahead is meant to be inserted because the editor lost that text;
  here the editor never lost it (only the real shell's buffer was
  cleared), so re-inserting it duplicated it on top of what was already
  there, and the duplication compounded on the next keystroke's restore.
  Fixed by giving push_expected_echo its own backing queue
  (EarlyOutput::expected_echo) and a dedicated consume_expected_echo,
  wired into input()/carriage_return()/linefeed() ahead of the existing
  typeahead/background-output logic: a match there is dropped entirely
  now, never surfaced as typeahead and never rendered as background
  output. handle_potential_typeahead itself is reverted to its original,
  unmodified behavior. Updated the unit test to assert typeahead() stays
  empty rather than getting duplicated into it.

- Fixed the fish native-completions history leak: fish has no
  configurable history-exclusion mechanism (unlike bash's HISTIGNORE or
  zsh's hist_ignore_space) -- a leading space is the only, default,
  non-configurable way to omit a command from its history file.
  generator_command_for's fish case never added one, so every request
  leaked into ~/.local/share/fish/fish_history. Added the leading space,
  matching the exact convention InBandCommandExecutor already uses for
  the existing warp_run_generator_command mechanism; the bracketed-paste
  leading-whitespace preservation this depends on already existed in
  bytes_to_execute_command. Updated existing tests for the new leading
  space and added a dedicated test locking in the behavior (and that the
  other three shells don't gain an unwanted leading space).
…leak

The zle-line-init restore's else branch unconditionally ran `zle -D
zle-line-init`, which errors if the widget no longer exists by the time
the select loop returns (e.g. a chained hook that itself rebinds
zle-line-init via add-zle-hook-widget during the chain call). Guard it
with the same existence check the other branch already gets for free
from zle -A's auto-creation semantics.

Also: warp_set_title_active_on_preexec (zsh and bash) is a preexec hook
that fires for every command including generator commands, and had no
exclusion for them (unlike warp_preexec's own PID-killing logic, which
already excludes warp_run_generator_command*). This briefly set the tab
title to 'warp_run_generator_comma...' during every native-completions
request. Fixed both to skip title-setting for generator commands,
matching the existing exclusion convention.
…mand execution

Replaces the kill-buffer+type+Enter+restore idiom for PowerShell with a
dedicated PSReadLine key handler (Alt+3) that reads the buffer directly
via GetBufferState, computes completions via CompleteInput, and reverts
the buffer -- never AcceptLine. This eliminates all three PowerShell
failure modes found in verification (kill-buffer chord atomicity,
buffer concatenation, auto-execution) at the root, since none of them
can occur when nothing is ever typed as a command or submitted:

- generator_command_for's PowerShell case now returns just the
  hex-encoded buffer text, with no function-call syntax at all.
- send_write_to_event_loop's RunNativeShellCompletions handling branches
  on shell_type: PowerShell types the hex text (registered via
  push_expected_echo so it isn't rendered as a phantom block) followed
  immediately by the new trigger chord, with is_for_command=false and
  no buffer_text stored for restoration, since nothing here ever
  touches the real buffer to begin with.
- execute_next_queued_write's is_command gating is now shell-type-aware
  for the same reason: PowerShell's write never transitions the line
  editor back to active the way a real command's precmd would, so
  gating queue draining on it would stall forever.
- Removed the old Warp-Run-GeneratorCommand-NativeCompletions function
  and its module export; PowerShell no longer needs the
  AddToHistoryHandler exclusion either, since nothing is ever submitted.

Verified empirically end to end via tmux (needed a real terminal size --
a bare 0x0-sized PTY made RevertLine throw): the Alt+3 binding registers
correctly, Get-Ch decodes and completes to Get-ChildItem with its
description via the same OSC 9280 wire format the other three shells
use, the buffer ends up empty afterward (confirmed via GetBufferState),
nothing auto-executes, and the session stays fully functional
afterward. Added a unit test for the new dispatch path and updated the
existing generator_command_for tests for the format change.
…ces the error

The previous fix (checking \${+widgets[zle-line-init]} before deleting) does not
actually prevent the "No such widget \`zle-line-init'" error in the common case:
zle-line-init still exists at that point (we are the ones who bound it), so the
elif branch is still true and zle -D still runs, still corrupting zsh's internal
state for the next prompt read. Verified empirically with a minimal repro (select
+ a zle-line-init handler that calls accept-line on itself, then zle -D
zle-line-init) that the elif-guarded delete still reproduces the error, and that
never deleting the widget in the "nothing was bound before" case (only restoring
when something WAS bound) does not.
…tions request

The previous fix (e62d9c3) stopped restoring/deleting zle-line-init
after each select, leaving our capture widget permanently bound to
avoid the 'No such widget' error from a chained hook rebinding it
during the chain call. But it didn't guard the *next* request's own
takeover: on request 2+, zle-line-init is already bound to our own
capture widget, so `zle -A zle-line-init _warp_saved_zle_line_init`
aliases the saved-widget name to itself. The widget's own
chain-to-saved-widget call then recurses into itself indefinitely
(measured: 'maximum nested function level reached'), so it never emits
the OSC terminator or calls accept-line, leaving the select blocked on
a real read forever -- wedging the session (no further menus, Enter
stops working) with nothing visible in the GUI, since the select's
stderr redirect swallows the error.

Fixed by comparing the current zle-line-init binding against our own
widget name before capturing: skip the takeover entirely when it's
already us, since there's nothing new to save.

Verified with the minimum bar requested: at least two (here, three)
consecutive completions requests in one session, both with and without
a simulated chained hook (add-zle-hook-widget style, matching how
p10k/zsh-syntax-highlighting/zsh-autosuggestions register). All three
requests return the correct 8 matches with clean stderr in both
scenarios; the chained hook's call count increases consistently across
requests (confirming the chain keeps working, not just not-crashing);
and the session remains fully responsive to ordinary commands
afterward in both scenarios.
…and a phantom-block race

- last_completed_command_text() (used as the vertical tab's primary label
  fallback whenever the OSC-set title equals the working directory, which is
  the common idle-prompt case) never excluded in-band command blocks, so a
  just-completed native-completions request's full, untruncated command text
  could surface as the tab label. This explains why the leak showed the
  complete command rather than warp_title's 25-char-truncated output, and why
  the earlier zsh/fish preexec-hook guards didn't fully address it: this path
  never went through warp_title in the first place. Fixed by excluding
  is_in_band_command_block() blocks, matching restored_block_commands()'s
  existing filter.

- fish_title's leading-space-defeats-the-match issue (found in the last
  verification round) is fixed by trimming before matching.

- warp_preexec's generator-command-detection guard had the same leading-space
  issue, plus a separate, pre-existing bug: 'test (! string match -q ...)'
  always evaluates false regardless of the match, since string match -q
  prints nothing for '!' to negate via command substitution, and bare 'test'
  with no arguments is false. This meant stale generator PIDs were never
  killed for *any* command, not just ones affected by the leading space.
  Fixed by using fish's own 'not', which negates a command's exit status
  directly, verified empirically for both a real command and a generator
  command.

- Root-caused and fixed the intermittent phantom block containing typed text:
  ModelEvent::CompletionsFinished's buffer-restore write and
  LineEditorStatusEvent::Active's input-reporting-sequence write both
  push_front from the same underlying trigger (the shell returning to a fresh
  prompt after the generator command completes), in an order that depends on
  unrelated PTY buffering/event-processing timing. When input reporting fires
  after the restore, it reports and clears the text just written back,
  producing PTY output push_expected_echo never registered -- rendering as an
  unexpected background (phantom) block. Fixed with a one-shot flag that
  skips re-queueing input reporting immediately after a restore, since it
  would be redundant: the client already knows the buffer's contents.
…ore than once per redraw

Root cause, from PHANTOM_DIAG evidence on a real session: push_expected_echo
registered the restored buffer's characters exactly once, but both zsh's
ZLE and fish's line editor echo that buffer more than once while redrawing
after a native-completions restore -- zsh echoes one character, returns to
column 0, then reprints the whole line; fish echoes the whole buffer,
returns to column 0 (twice), then echoes it again. The one-shot queue had
nothing left to match on the second (or third) copy, so the surplus
characters fell through to an unmatched background block -- frozen at one
character for zsh, since the surplus overwrites in place at column 0 each
cycle, and growing with the buffer for fish, since the whole buffer is
re-echoed and re-mismatched every keystroke.

Fixed by replacing the one-shot VecDeque-based queue with a Vec plus a
match-position index: matched content is never removed, only the position
advances, and a carriage return rearms the position back to 0 unconditionally
(regardless of how much of the current pass matched), since a carriage
return is exactly what precedes each repeat of the echo. push_expected_echo
now replaces the registered content outright rather than appending, since
each restore is an independent echo to expect. Defensively cleared on
precmd too, so a restore whose echo never fully arrives can't bleed into an
unrelated later prompt cycle.

Added three regression tests reproducing both shells' exact echo shapes
from the diagnostic evidence, plus one confirming the precmd boundary.
…t-suite build error

Diagnostics on a real session showed CompletionsFinished (and the
push_expected_echo call it triggers) fires when 9280;B is parsed, which
precedes the shell's own in-band-command precmd DCS -- so precmd normally
lands inside the restore window, not after it. The defensive clear added
alongside the matcher fix wiped a just-registered echo before its own
characters had even arrived in ~5 of 6 restores, which is worse than the
original bug: the whole buffer fell through instead of just a surplus
copy. Removed the clear; push_expected_echo replacing its content outright
on every call already bounds staleness, which was the clear's only
intended purpose.

Replaced the test that encoded the clear's behavior (and so was passing
while asserting the bug) with two tests for the actual invariants: the
registration survives a precmd within the same restore window, and
staleness is bounded by the next push_expected_echo call rather than by
precmd.

Separately, pty_controller_lifecycle_tests.rs used bytes.as_ref() on a
Cow<'_, [u8]>, which is E0283-ambiguous once typed_path is in the
dependency graph (it adds a competing AsRef impl) -- present since the
pwsh test landed, unrelated to this commit's own changes. Switched to
slice indexing, which is unambiguous.
…e current one

Diagnostics showed a third echo shape: fish sometimes returns to column 0
mid-line and continues the same echo from wherever it left off, rather
than restarting from the beginning. The unconditional rearm (reset to 0)
from the previous fix discarded the in-progress match position in exactly
this case, so the continuation was compared against the wrong expected
character and fell through.

Replaced the single match position with a set of live candidate positions
(expected_echo_positions). A carriage return now *adds* a position-0
candidate without discarding whatever was already live, since it isn't
knowable in advance whether a given carriage return means a restart or a
mid-line continuation. Each subsequent character advances every candidate
whose next expected character matches it and drops the rest; a character
counts as expected echo if any live candidate predicts it. This is a
generalization of the two-shape fix, not a special case: with only one
candidate ever live, it behaves exactly as before.

Added tests for the exact split-continuation shape from the diagnostics,
its minimal two-character form, and the ambiguous case where a carriage
return leaves two candidates that both match the same next character.
… -ld

The completions `compadd` shim located the description array with an exact
match on `-d`, but `_describe` never passes that flag on its own -- it passes
it clustered with other short flags, as `-ld`. The lookup therefore found
nothing, no description array was resolved, and every match produced via
`_describe` came back with an empty description. `_arguments`-based option
descriptions, which do pass `-d` unclustered, were unaffected, which is why
some completions had descriptions and others silently did not.

Match any flag token of a leading `-`, zero or more letters and a trailing
`d` instead of requiring an exact `-d`, and keep using `(I)` rather than
`(i)`: `(i)` returns one past the end instead of 0 when nothing matches,
which would make the presence test true on every call. The search is also
restricted to the leading flags-only prefix the neighboring `-O`/`-A`/`-D`
check already uses, so a completion candidate that happens to look like a
flag -- a literal `-d` or `-ld`, as `ls` and `find` offer -- is never
mistaken for the flag itself.

Fixes CORE-3795.
`FeatureFlag::NativeShellCompletions` had no corresponding cargo feature, so
the only ways to turn it on were editing `DOGFOOD_FLAGS` or setting the
`ForceNativeShellCompletions` private pref -- unlike every other flag, which
can be enabled per-build with `cargo run --features <name>`.

Declare `native_shell_completions` in the app crate and map it to the flag in
`enabled_features()`, following the same pattern as the neighboring
completions flags. It is not added to any default or channel feature set, so
the flag stays off unless asked for explicitly.
…existing fallback

The requester hit this: in natural-language mode, native shell completions
being enabled anywhere disabled the file-path completions fallback that
AI input mode depends on entirely, and also sent the natural-language
buffer text to the shell for completion, which the shell has no basis to
answer meaningfully.

Confirmed via a git diff against master that this file was never touched
by this PR: the gap already existed for zsh (the only shell native
completions supported before this PR), and was reachable whenever a zsh
session had the feature/pref on while in AI input mode. This PR's
expansion of native shell completions to all four shells, plus the
ForceNativeShellCompletions pref used for testing it, is what made the
requester actually hit it, not a change to this file.

use_native_shell_completions gated fallback_strategy (via a match keyed
only on completions_trigger) and independently gated whether a native
shell completions request was dispatched at all (via a check on
use_native_shell_completions alone, regardless of trigger). Both need
input_type excluded: fallback_strategy's Keybinding/SlashCommandAutoOpen
case needs the same file-path fallback AI mode already used before this
existed, and the native-completions dispatch must never fire for AI mode
at all, on any trigger -- there's no command spec to hand a natural-
language buffer to a shell for.

Extracted the eligibility and fallback-strategy decisions into small pure
functions (should_use_native_shell_completions,
completions_fallback_strategy_for_trigger) so they're directly testable
without driving the full Input view's GUI test harness, and added eight
unit tests pinning every branch, including the exact regression: AI input
mode must keep the FilePaths fallback for an explicit Tab press even when
native shell completions would otherwise be eligible.
… request would immediately undo

Correctness cleanup, NOT a fix for the reported lone-trailing-character
ghost block: the requester's own repro (Tab-triggered, a single
completions request, on a buffer that was fully typed before the request
was made) has no newer request queued behind the restore at all, so this
change cannot explain or fix that symptom. Pushing it on its own merits.

The defect: CompletionsFinished queues the buffer restore to the front of
pending_writes and drains it via execute_next_queued_write. That function
is meant to stop draining immediately behind a foreground command --
RunNativeShellCompletions already gets that treatment for the three
shells that run it as one -- but the restore write undoes such a
command's buffer-clearing effect without being flagged as needing the
same protection, since it goes out as a plain PtyWrite::Bytes rather than
being recognized as tied to a command's aftermath. If a newer completions
request's own write is already sitting in the queue behind the restore
when the restore drains, execute_next_queued_write's existing recursion
sends that newer request's kill-buffer immediately behind the restore,
before the shell can have processed it.

I originally described this fix as gating execute_next_queued_write's
is_command check on the restore the same way it already does for
RunNativeShellCompletions. Writing the actual change surfaced that this
would deadlock: the gate's only means of unblocking is the shell's own
precmd firing LineEditorStatusEvent::Active again, and nothing about a
plain buffer write -- no command runs, no prompt cycle happens -- ever
causes that to fire on its own. Gating on it would leave every write
queued after a restore stuck until an unrelated real command happened to
run, which is worse than the race it would fix.

Implemented instead as: don't queue the restore at all when a newer
RunNativeShellCompletions request is already waiting behind it, since
that request's own kill-buffer is about to clear the line again anyway,
making the restore pointless to send. This sidesteps the race without
touching execute_next_queued_write's draining logic or its unblocking
condition at all -- the newer request proceeds exactly as it would
otherwise, and produces its own, current restore once it completes.

No test added: the existing PtyController test file has no precedent for
driving ModelEvent::CompletionsFinished's dispatch path (via the raw
Event channel ModelEventDispatcher forwards from) synchronously inside
App::test, and I don't have a confirmed way to verify a test exercising
it wouldn't just pass trivially without exercising the code path.
…ssion

A future reader of the CompletionsFinished handler will see a restore
being deliberately dropped and needs the reasoning at the point where
that happens, not only in a review thread.
Ports the PHANTOM_DIAG logging from 68aa340 onto the current head of
factory/native-shell-completions-generator (74bcfca), adapting it to the
candidate-set matcher: every log line reports the live
expected_echo_positions set rather than a single queue position.

Adds two instruments the original diagnostics did not have, both of which
turned out to be the ones that decided the round:
- backspace() logging, since a literal backspace rewind is invisible to the
  matcher and so leaves no other trace.
- a raw PTY byte dump in on_finish_byte_processing, escaped with
  escape_ascii(), which is what makes the rewind shape readable at all.
@cla-bot cla-bot Bot added the cla-signed label Aug 19, 2026
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

This PR was generated with Warp.

Comment @warp-factory on this PR to send it follow-up work.

View run View conversation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant