Skip to content

Codex/fix tui stale autocomplete 407 - #495

Open
REI-330 wants to merge 5 commits into
openpi-dev:mainfrom
REI-330:codex/fix-tui-stale-autocomplete-407
Open

Codex/fix tui stale autocomplete 407#495
REI-330 wants to merge 5 commits into
openpi-dev:mainfrom
REI-330:codex/fix-tui-stale-autocomplete-407

Conversation

@REI-330

@REI-330 REI-330 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Problem

Windows regular TUI 在 slash 命令自动补全列表缩小时可能残留旧行,影响界面可读性。

Value

避免 Windows 用户看到过期的自动补全内容,恢复稳定的终端重绘行为。

Approach

  • 新增 Windows TUI 兼容层,仅对 regular TUI 启用 Pi 的 clearOnShrink
  • fullscreen TUI 和非 Windows 平台保持原有行为。
  • 提供 OPENPI_WINDOWS_TUI_COMPAT=0 环境变量作为退出开关。
  • 添加针对平台、渲染模式和退出开关的回归测试。

Validation

  • bun test tests/extensions/windows-terminal-compat/index.test.ts:3/3 通过
  • Biome 检查通过
  • TypeScript 检查通过
  • 完整检查受本地 Windows 环境既有 CRLF、权限和进程基线测试影响

Impact

仅改变 Windows regular TUI 的终端重绘策略,不改变命令注册、编辑器行为、fullscreen TUI 或其他平台行为。

Fixes #407

@github-actions github-actions Bot added documentation Improvements or additions to documentation area:workflows Workflow engine, capability, skills, or tests area:setup OpenPI setup, configuration, or setup documentation labels Sep 8, 2026
@REI-330
REI-330 force-pushed the codex/fix-tui-stale-autocomplete-407 branch from f9ed007 to 6f10280 Compare September 8, 2026 12:39
@github-actions github-actions Bot removed the area:workflows Workflow engine, capability, skills, or tests label Sep 8, 2026

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@REI-330
REI-330 requested a review from tt-a1i September 8, 2026 14:16

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@REI-330
REI-330 requested a review from tt-a1i September 8, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:setup OpenPI setup, configuration, or setup documentation documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(tui): prevent stale autocomplete rows on Windows terminals

2 participants