Codex/fix tui stale autocomplete 407 - #495
Conversation
f9ed007 to
6f10280
Compare
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed at head 6f10280.
Standards
[P1] The new user-facing OPENPI_WINDOWS_TUI_COMPAT opt-out bypasses the package configuration contract. Package-owned feature toggles must use the canonical /openpi-setup path and update extensions/setup/, extensions/shared/setup-config.ts, the no-argument status output, SETUP.md, and README. This PR only adds the environment variable and SETUP.md documentation. Please either integrate the complete configuration path or remove the public toggle.
[P3] The same environment-variable check is repeated in install() and the editor wrapper. Once install() has passed, the second expression is always true.
Spec
[P2] applyWindowsTerminalCompatibility() unconditionally sets clearOnShrink=true in Windows regular mode, overriding an explicit terminal.clearOnShrink=false preference and leaving persisted configuration inconsistent with runtime behavior.
[P2] The compatibility layer runs only when the editor factory is created. Pi 0.85.1 does not rerun that factory when switching from fullscreen to regular mode, so that supported lifecycle can still retain clearOnShrink=false and reproduce the stale-row bug. The current tests cover only the pure helper, not an actual mode transition.
Please preserve explicit user configuration and add a lifecycle regression for fullscreen to regular before merging.
tt-a1i
left a comment
There was a problem hiding this comment.
Re-reviewed at head bd4b76e.
The custom OpenPI environment toggle and its configuration-contract issue are fixed. Two runtime blockers remain.
Standards
[P1] Please register the listener through the Pi extension lifecycle instead of calling tui.addInputListener() directly. The tui value passed to the editor factory is a stable proxy, but addInputListener() returns a subscription bound to the current underlying renderer. Pi replaces that renderer when switching TUI modes and only rebinds subscriptions registered through ctx.ui.onTerminalInput(). As written, the listener remains attached to the old fullscreen renderer and the new regular renderer has no listener. It also retains the old renderer until shutdown.
A reproduction using the Pi 0.85.1 stable-TUI proxy produced:
{"oldListeners":1,"newListeners":0,"clearOnShrink":false}
The new test changes mode on one mock object, so it does not model renderer replacement and gives a false positive. Please test with two renderer instances or the real Pi lifecycle.
[P3] Listener registration should be gated to Windows with the opt-out disabled. The current code installs a per-keystroke listener on Linux, macOS, fullscreen-only, and opted-out sessions even though it can never change behavior there. The PI_CLEAR_ON_SHRINK expression is also duplicated between initial application and listener installation.
Spec
[P2] An explicit terminal.clearOnShrink=false setting is still overwritten on the next input. Pi SettingsManager preserves the provenance only before it reaches TUI; TUI.getClearOnShrink() returns false for both the default and an explicit false value. applyWindowsTerminalCompatibility() therefore cannot distinguish them and sets the runtime value back to true, leaving persisted configuration false while runtime behavior is true.
The test named preserves an already enabled or explicitly disabled renderer setting mocks getClearOnShrink() as true, so it only covers the already-enabled case. Please add the explicit-false case and preserve the native setting, including changes made through /settings.
Problem
Windows regular TUI 在 slash 命令自动补全列表缩小时可能残留旧行,影响界面可读性。
Value
避免 Windows 用户看到过期的自动补全内容,恢复稳定的终端重绘行为。
Approach
clearOnShrink。OPENPI_WINDOWS_TUI_COMPAT=0环境变量作为退出开关。Validation
bun test tests/extensions/windows-terminal-compat/index.test.ts:3/3 通过Impact
仅改变 Windows regular TUI 的终端重绘策略,不改变命令注册、编辑器行为、fullscreen TUI 或其他平台行为。
Fixes #407