Skip to content

fix(tui): avoid stacking editor layers on session start - #741

Open
outlier27-cell wants to merge 8 commits into
openpi-dev:mainfrom
outlier27-cell:fix/issue-740-editor-layer-reinstall
Open

outlier27-cell wants to merge 8 commits into
openpi-dev:mainfrom
outlier27-cell:fix/issue-740-editor-layer-reinstall

Conversation

@outlier27-cell

@outlier27-cell outlier27-cell commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Closes #740. Repeated Pi session starts could compose editor layers from an already wrapped factory. Actual native /new acceptance also exposed a pending projection reading an ExtensionContext after Pi invalidated it during replacement.

Value

Keeps one composition of each active editor layer and preserves a usable editor across session replacement, layer removal, and native editor changes.

Approach

Preserve each native UI owner's base and installed factory identities, rebuild from the base, and reject stale queued projections after an external editor replacement. Capture the UI while registration's context is valid; timer/removal paths retain no ExtensionContext. During native session_shutdown, cancel queued installation, restore the base only while the current editor is still coordinator-owned, and clear active ownership before Pi invalidates the runner. Preserve foreign replacements.

Keep the contributor history and merge main f98d6a4737461ba8d8ee100f113b5a7c3a0001a0. Add focused identity, stale-context, shutdown, fresh UI wrapper, and foreign replacement regressions. Update two existing test doubles to expose native lifecycle registration and stable editor factory identity; their assertions remain intact.

Validation

Final head: a467505. Local macOS, Node v26.8.1 / Bun 1.3.14; native TUI Pi 1.1.0 with an isolated agent directory loading only this OpenPI checkout.

  • Frozen install: passed.
  • Focused editor regression: 17 passed; related editor/clear-context/Windows-compat fixtures: 33 passed.
  • bun run check: passed.
  • bun run test: passed; Node 2,465 passed / 9 skipped, UI 1,338 passed in 88 files.
  • Real native TUI: two /new replacements survived, each startup installed the observer layer once; draft input rendered and clean exit succeeded. Separate native install/re-register/remove returned wrap counts 1/1/0; draft input remained usable after removal. No model calls were made.
  • git diff --check: passed.

The pre-repair native /new crash and two RED regressions were captured. The first full run then exposed two incomplete test doubles (missing pi.on, unstable factory getter); those were corrected without weakening assertions before the final full run. Fresh current-head CI remains separate from local validation. Historical contributor and earlier maintainer validation remains in the PR timeline.

  • Two independent final source reviews passed on this exact head. Current-head CI run 38694724322 passed all required checks, including Windows. Independent last-push approval remains required before merge.

Impact

  • User-visible behavior: editor layers remain singular and the native editor survives Session replacement and removal.
  • Model-visible context/tools: None.
  • Runtime/lifecycle: projection ownership now releases through Pi's native shutdown event before runner invalidation; native replacements remain authoritative.
  • Persisted config/data: None.
  • Compatibility/risk: uses existing Pi lifecycle/UI APIs; no global cache, schema migration, or installed-package changes. Reversible with a code revert.

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

@tt-a1i 请审核此 PR。当前精确提交的 GitHub CI 已全部通过,修改范围与回归测试均已在 PR 说明中列明;现仅等待维护者审核。

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

@testikun @tt-a1i 请审核此 PR。最新提交的 CI 已通过,当前仅等待维护者审核与合并。

@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.

Exact reviewed head: 19ba94d85b93fda55ece73ad925d0e881ed715e8.

Spec:

  • [P1] Fresh native contexts still stack editor layers: Pi 0.99.1 creates a fresh context for each event while retaining the same UI. Registering the same layer through two different contexts sharing that UI produces [one, one], because the second registration treats the already-composed factory as its base. Existing 2/2 tests passed. Cache the base under stable UI/editor ownership and add the two-context/shared-UI regression.

Verification: prior exact-head Standards and Spec evidence reused because this head is unchanged; current review/CI/integration metadata refreshed. No complete-suite rerun, installed-runtime/provider run, or visual acceptance performed in this update.

Maintainer fix follow-up

The requested stable UI ownership fix is implemented in 26cfd8922c35f6c303a42a9aa04f0e9948afaaf7, appended to the original contributor branch by non-force push. Base/installed factory ownership survives fresh contexts, UI revisits and last-layer removal. Only the coordinator's own installed factory is restored; a native replacement, including the default before queued installation, remains owned by Pi. Malformed registrations without a UI are ignored.

Independent automated Spec and Standards reviews have no open findings after the three-round bounded review. Focused editor/clear-context tests 18/18, bun run check and final bun run test all passed: Node 2,326 passed / 9 skipped, UI 1,158 passed across 82 files. Earlier unrelated setup-lock timeout and superseded runs remain recorded as historical failures.

The code finding is addressed. Exact-head remote CI is running; branch protection still requires valid final-push approval before merge.

tt-a1i
tt-a1i previously approved these changes Oct 13, 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.

Exact reviewed head: 26cfd8922c35f6c303a42a9aa04f0e9948afaaf7.

Spec:

Stable UI ownership now prevents fresh-context and repeated registration from recapturing a composed editor factory. Last-layer removal restores only an actually installed owned factory, preserving native replacement/default selection; missing-UI event input is ignored.

Standards:

Independent automated Spec and Standards reviews have no open findings on the final source freeze. The patch preserves original contribution commits and Pi-owned lifecycle and authority; it adds no model-facing tool, defaults or provider stack.

Validation:

Focused 18/18, bun run check and final bun run test passed: Node 2,326 passed / 9 skipped, UI 1,158 passed. Earlier timeout and superseded runs remain recorded as historical failures. Current-head remote CI completed 18/18 SUCCESS, including Runtime, Windows, UI and Web E2E. Local and remote results are separate receipts.

Merge gates:

The final push was made by tt-a1i. Valid last-push approval from another eligible maintainer and all current branch-protection gates remain required before merge.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(tui): repeated session starts stack shared editor layers

2 participants