feat: chain onto previously installed editor factory (#3935)

[kylesnowschwartz]

Disclosure: the patch in this PR was generated by an AI coding assistant working under my supervision. I reviewed the diff, ran the full test suite, and exercised the change in a live pi session against a real chain (this extension paired with @jordyvd/pi-image-attachments) before opening the PR. Treat the prose as mine and the code as something I'd cosign.

This opts pi-vim into the new ctx.ui.getEditorComponent() API that landed in @mariozechner/pi-coding-agent 0.71 (pi-mono#3935). With this change, pi-vim can be loaded alongside another extension that also installs a custom editor (the concrete case I had: @jordyvd/pi-image-attachments) and both behaviors compose, instead of last-session_start-wins clobbering one of them.

The structural change is converting class ModalEditor extends CustomEditor into a class-mixin factory:

export function createModalEditor<TBase extends CustomEditorConstructor>(Base: TBase) {
  return class ModalEditor extends Base { /* ... */ };
}
export const ModalEditor = createModalEditor(CustomEditor);
export type ModalEditor = InstanceType<typeof ModalEditor>;

ModalEditor remains exported as both a value and a type, so external consumers (and the existing test suite) don't have to change anything. The session_start handler reads getEditorComponent() and, when a previous factory is present, probes it once to grab its class and uses that as Base instead of CustomEditor. With no previous factory, the canonical ModalEditor is reused so instanceof ModalEditor checks keep working for the standalone case.

A few small follow-on changes were necessary to make the mixin pattern clean:

• The constructor became a (...args) passthrough. labelColorizers was the one extra arg the constructor used to take, and that doesn't survive a chain — a wrapping subclass would have to know how to thread it through. I moved colorizers to a setColorizers(colorizers: ModeLabelColorizers | null) method called post-construction by the session_start handler. The one test that constructed a ModalEditor with colorizers directly was updated to call the setter instead.
• The @mariozechner/pi-coding-agent dev dependency was bumped from 0.55.x to ^0.71.0 so tsc sees the new ExtensionUIContext.getEditorComponent member. The peerDependencies range stays at * and the runtime call uses optional chaining (ctx.ui.getEditorComponent?.()), so older pi versions still work — they just don't compose, which is the existing behavior.
• node_modules/ was added to .gitignore (was missing). I noticed this when I accidentally tracked it on the first push and had to force-push to clean up.
• script/pack-check.ts size thresholds bumped by ~3% to accommodate the README section and the wrapper. Comment updated.

Tested by:

• npm run check (lint + typecheck + 500 tests) all green. Added two new tests in test/modal-editor.test.ts under a composable editor factory (#3935) describe block: one installs a fake previous factory and verifies the resulting editor is instanceof PreviousEditor and the probe runs exactly once; the other confirms standalone use still produces an instanceof ModalEditor instance.
• Manual smoke test in pi 0.71.0 with @jordyvd/pi-image-attachments listed before pi-vim in settings.json.packages. Both extensions install cleanly with no shadowing in pi --verbose's [Extension issues] section, the footer renders the vim mode label, and image-path pastes still produce attachment placeholders inside the modal editor.

A note on extension authors composing on top of pi-vim: createModalEditor(Base) is now part of the public API. If you want to layer something on top of vim modal editing, you can call it with your own base class and you'll get a subclass back. README documents this.

Companion PR on jordyvandomselaar/pi-image-attachments doing the same opt-in from the other side: jordyvandomselaar/pi-image-attachments#3

[lajarre]

Thanks very much for the PR and the upstream API fix. I heard about the ModalEditor issue but didn't act on it as I was not facing it myself.

Looks good at first glance, will consider merging with my next batch very soon.

[lajarre]

[DRAFT COMMENT]

I found one blocking issue in the composition contract.

ctx.ui.getEditorComponent() returns an EditorFactory, but index.ts uses it only to create a probe instance, reads probe.constructor, discards the probe, then constructs a new subclass with the standard (tui, theme, kb) arguments.

That loses valid factory-level behavior from the previous editor factory:

• extra constructor arguments supplied from the factory closure
• instance mutations after construction
• listeners/resources registered by the factory
• cleanup references stored by the factory

It also runs probe side effects on an instance that is never mounted.

Concrete repro shape:

class PreviousEditor extends CustomEditor {
  constructor(tui, theme, kb, required) {
    if (!required) throw new Error("missing required factory arg");
    super(tui, theme, kb);
  }
}

ctx.ui.getEditorComponent = () =>
  (tui, theme, kb) => new PreviousEditor(tui, theme, kb, { ok: true });

With the current implementation, pi-vim probes that factory successfully, then mounts new Composed(tui, theme, kb), which calls PreviousEditor without the required factory-provided arg.

The same issue applies when pi-vim is the inner editor for a later extension: pi-vim's per-instance configuration is currently factory-level (setColorizers, clipboard policy, quit handler, notify handler). A later extension that probes pi-vim and subclasses only ModalEditor would drop that configuration on the mounted editor.

Please either preserve the previous factory semantics, or document/test a class-only composition contract. If this keeps the class-only approach, add coverage where the previous factory supplies required closure state or mutates the returned instance; the current test only covers a zero-arg class with constructor-initialized state.

Verification I ran on this branch:

• npm ci
• npm run check — 500 tests passed
• npm run pack:check — passed

[lajarre]

Quick context for anyone following this PR.

#21 hardens the delegation layer this PR introduced — same architecture, plus the review findings (malformed-previous-editor rejection, scoped wantsKeyRelease, mode-label colorizer compatibility, image-attachments e2e infra failures). That's the immediate path forward.

Separately, building and reviewing this work made us want to articulate a longer-term alternative for pi-vim: a full editor-replacement shape, rather than a wrapper sitting on top of an unknown previous editor. The composition direction pushes every editor extension to learn every other one's surface, and existing modal-editing implementations in similar agents (Codex CLI's composer Vim mode, Claude Code's prompt-input Vim mode) take the other route — the app owns the prompt editor with Vim as an internal sub-engine, and slash commands / paste / history / autocomplete are integrated inside the owned editor rather than negotiated across a chain.

To be clear: that direction is exploratory and contingent on Pi's own evolution and on whether enough editor-owning extensions exist to make replacement a worthwhile peer architecture. pi-vim is not committing to executing on it. We're shipping the composition bridge here and keeping it healthy in the meantime.

[kylesnowschwartz]

Thanks for the update @lajarre I came to the conclusion that the extension interface was a bit unstable too. I considered your feedback and came to no definitive conclusion on best approach. So for now I'll maintain a local fork until a major release.

[lajarre]

Hey @kylesnowschwartz

Closing the loop on this. TL;DR: please install in order (i) pi-vim, then (ii) pi-image-attachments. This should work today.

Happy to continue the discussion and please reach out if you have any issue.

More details below (clankertext).

---

where we landed

After working through #21 (which hardened the wrap-of-previous-editor approach this PR introduced), I pivoted on the architecture. The complexity cost of "pi-vim wraps any previous editor as an INSERT delegate" wasn't paying back — ~3.4k LOC in index.ts, ~6.7k LOC of tests, a ~200-line README section documenting the reconciliation rules — for what is effectively one ecosystem integration.

The shape I settled on is in #23 (feat/wrappable-editor): pi-vim stays a pure replacement editor but is wrapper-tolerant — its surface is documented so that other extensions (like pi-image-attachments) can wrap pi-vim cleanly. The inverse direction (pi-vim wrapping a previously-installed editor) is intentionally not supported.

This will close #18 and #21 in favor of #23. Thanks for forcing the conversation — most of the framing that ended up in #23's README and the retrospective fell out of working through your code.

what landed on pi-image-attachments' side

jordyvandomselaar/pi-image-attachments#4 merged 2026-05-05 — a different contributor shipped composition from the other side. Your #3 was closed in favor of that one.

what this means for your setup

You don't need the local fork:

• update @jordyvd/pi-image-attachments to the version containing ^ with 0 #4
• use pi-vim once test: support pi-vim wrapper integration #23 merges (or pin the feat/wrappable-editor branch if you want it now)
• install order: pi-vim first, then pi-image-attachments. pi-image-attachments wraps pi-vim, not the inverse. (The inverse is unsupported in test: support pi-vim wrapper integration #23.)

bigger picture

The retrospective + the architectural decisions are written up here:

earendil-works/pi#4798

It also flags a UX risk (silent clobbering when multiple editor extensions install) and a strategic gap that would surface for any future full-replacement editor (e.g. a Neovim bridge). Not asking pi-mono for anything actively; flagging for the maintainer's awareness.

Thanks again for the PR.