Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
wolfiesch
marked this pull request as draft
September 21, 2026 10:35
wolfiesch
marked this pull request as ready for review
September 21, 2026 10:35
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Restore top-level imports for
ModelPickerComponentandModelHubComponent, replacing their shared synchronousrequire()loader. Add an isolated subprocess regression that opens the picker with the real compatibility shim installed, renders a model, dismisses the picker with Escape, and verifies that keyboard focus returns to the editor.Why
On source-linked installs with Bun 1.3.14, pressing Alt+P after compatibility plugins initialize can terminate the session. The deferred
require()passes through the process-global legacy-pi resolver and fails withNameTooLongon a repeatedfile:file:…path. When this exception escapes the key handler, the terminal disconnect path ends the session without showing the underlying loader error.Related to #12293. This patch addresses the model picker and model hub only; it does not claim to close the whole issue. The provider-auth work in #12271 remains separate.
This deliberately restores eager imports for the two model overlays instead of changing the process-global compatibility resolver. Other deferred components remain unchanged. Startup timing was not measured.
Testing
test/model-picker-compat.test.ts: failed before the fix with the reportedNameTooLongerror; passes after the fix. Compatibility plugin registration stays inside a child process so it cannot affect later tests.packages/coding-agent:bun test test/model-picker-compat.test.ts test/model-picker.test.ts test/model-hub.test.ts test/interactive-mode-model-cycle.test.ts.packages/coding-agent:bun run check(lint, formatting, and types). An existing unused-import warning remains insrc/session/session-manager.ts.installLegacyPiSpecifierShim(): Alt+P displayed the model picker, Escape dismissed it, and Alt+M displayed the model hub without exiting.test/modes/controllers/selector-controller-login.test.tsto the focused test group exposes the existing auth loader failures. The same failures were reproduced on untouched upstream commit10b867cb2e, without the new regression test or patch.bun checkpasses (package scope)