chore(deps): Upgrade Pi packages to 0.85.1 - #1206
Merged
Merged
Conversation
Kimchi Code Review
Summary📊 Review Score: 80/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — The benchmark test 📝 Found 3 issue(s). See inline comments for details. (1 finding(s) referenced lines no longer in the diff and were omitted.) What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 80/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 4/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — The benchmark test test_install_accepts_version_override was updated to the new pin (@earendil-works/pi-coding-agent@0.85.1) and still verifies the version-override install path. However, the substantive risk in this PR — the rebased dist patches ({ persist } threading, flag-conflict severity downgrade, the Type Proxy, the required await on exportToJsonl) — has no committed automated coverage; verification is documented as a manual live tmux-harness/runtime check in the patch header.
📝 Found 3 issue(s). See inline comments for details. (1 finding(s) referenced lines no longer in the diff and were omitted.)
mszostok
added this pull request to stack #1208
September 15, 2026 12:39
mszostok
force-pushed
the
upgrade-pi-85.1
branch
2 times, most recently
from
September 15, 2026 18:09
1dfd18a to
79c2c26
Compare
mszostok
commented
Sep 15, 2026
mszostok
marked this pull request as ready for review
September 15, 2026 18:41
|
📚 No documentation changes were needed for this PR. |
mszostok
commented
Sep 15, 2026
pmateusz
approved these changes
Sep 17, 2026
Update pi-coding-agent, pi-tui, pi-ai pins and pnpm.patchedDependencies keys/paths together; rename the three patch files to match. Lockfile regenerated via pnpm install --lockfile-only: chord added to pi-coding-agent, chord/mistral/otel removed from pi-ai, @anthropic-ai/sdk 0.123.0, openai 6.40.0, glob/pi-client/pi-protocol dropped from pi-coding-agent. Dry-run (patch --fuzz=3 vs published 0.85.1 dists) reproduces the runbook-measured failure surface exactly: 11 hunks need rework (pi-ai validation.js 1; pi-tui box/text/tui-main-screen 3; pi-coding-agent edit/model-selector/interactive-mode/loader 7). pnpm install currently fails at patch application (expected; patches are rebased in Phase 2). Co-Authored-By: Kimchi <noreply@kimchi.dev>
- openai-completions.js cache_creation_tokens hunk: context drift only,
re-applied onto 0.85.1 (upstream also added rawUsage.cached_tokens).
- validation.js: REBASED, not re-applied. 0.85.1 added
normalizeOptionalNulls() called at the top of validateToolArguments;
the rebased patch keeps that call first, then applies Kimchi's
JSON-string/object coercion and Value.Convert error wrapping on top.
TYPEBOX_KIND removed (its last upstream reference was the guard this
patch deletes).
- Verified: patch applies with --fuzz=0; runtime smoke covers
JSON-string array coercion, {}-as-empty-array, optional-null
normalization.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
- tui-main-screen.js: scrollback-clear gate rewritten against the new BoundedTerminalWriter (output.append) helper. - text.js: LAYERED on upstream's new paddingX clamp instead of replacing it - clamp kept, Kimchi stroke accounting + hard truncation on top, clamped local paddingX threaded through the stroke sites. - box.js: Kimchi's stroke accounting extended into upstream's new handleMouse() so hit-test/child-height widths match render() (stroke eats 2 cells upstream never knew about). - markdown.js, utils.js hunks: applied without changes. - Verified: regenerated patch applies with --fuzz=0 (pnpm-strict). Co-Authored-By: Kimchi <noreply@kimchi.dev>
… 4-7)
- Step 4 (NEW, upstream candidate): extension setModel/setThinkingLevel
accept { persist } options - threaded through the extension action in
agent-session.js, the loader.js runtime facade, and widened in
types.d.ts. 0.85.1 made the upstream extension wrapper drop options
(session-only always); this preserves the persistence channel for
Kimchi's user-initiated call sites.
- Step 5: edit-rendering hunks retargeted to new core/tools/renderers/
edit.js (upstream extracted renderers; bodies moved unchanged).
- Step 6: model selector rebuilt on the 0.85.1 constructor
(settingsManager removed, onSelectAsDefault/defaultModel added);
Kimchi's sessionId appended LAST to avoid the positional collision;
persistence now flows via selectModel(model, true) per the runbook
decision (0.84.1 semantics); typed /model multi-model gets
{ persist: true }; the general typed path keeps upstream's new
session-only default.
- Step 7: triggerTurn hunk verified in the correct arm of the
restructured sendCustomMessage; compact({force}) rebased intact;
DROPPED the stale exportToJsonl await hunk (dead code);
bun Type2 Proxy re-applied onto the new bundled-runtime context;
bash timeout pipe-destroy now lives in createLocalShellOperations, so
it covers the PowerShell tool too (intentional).
- Step 9: DROPPED the pasteToEditor routing hunk - the TS patch
(src/paste-to-editor-patch.ts, handleTerminalInput) is the single home.
- Verified: regenerated patch applies with --fuzz=0; pnpm install green
with all three patches applied (new patch hashes in lockfile);
patch-pi-ai-oauth.js postinstall passes (oauth-page.js byte-identical
between versions, as expected).
Co-Authored-By: Kimchi <noreply@kimchi.dev>
The rebase commit dropped this hunk as "dead code" because exportToJsonl is synchronous in both upstream tarballs. That held the patch against the wrong baseline: src/cli.ts monkey-patches AgentSession.prototype. exportToJsonl with an ASYNC wrapper (trace-ID injection + awaited PII redaction, fail-closed on error) and documents the dependency outright - "upstream's handleExportCommand is patched to await this". Regression verified live before restore (tmux harness): - 0.84.1 binary: /export prints the real path - broken 0.85.1 binary: prints "[object Promise]" and would return before redactJsonlExport completes (a redaction throw becomes an unhandled rejection instead of a visible error) - restored 0.85.1 binary: prints the real path again Patch header updated: hunk is now documented as RETAINED with the contract explained. NOTE for the runbook maintainer: the v2 runbook's Step 6 claim "the exportToJsonl await hunk applies fine and is dead code" is wrong - it must be read against the cli.ts monkey-patch, not just the tarballs. Co-Authored-By: Kimchi <noreply@kimchi.dev>
…hase 3, Step 11)
0.85.1 flipped AgentSession.setModel persistence: {persist}-gated with a
session-only default (0.84.1 always persisted). With the patch-level
widening in place, apply the persist policy per call site:
- persist: true (user-initiated; preserves 0.84.1 semantics):
ui.ts Ctrl+P model cycling (3 sites), startup-auth login model
selection (3 sites), interactive-model-session new-session model (core
channel, native options), model-switch set_model tool, ACP set_model
(over the wire), login-command-patch browser + API-key adapters.
- session-only (unchanged, new default matches intent): model-switch
orchestrator selection and guard revert paths, router/model-roles/
prompt-enrichment automatic routing.
Tests updated to assert the { persist: true } second argument on the
user-initiated paths. The 4 ACP permission-mode failures in
src/modes/acp/server.test.ts reproduce identically on unmodified master
at 0.84.1 — pre-existing baseline noise, unrelated to this change.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…Step 12) - steering.ts: steer() failure-surface comment now references 0.85.1 (steer() unchanged upstream; logic holds). - benchmark: pi_kimchi_test.py hardcoded 0.84.1 assertion + doc comments in build-pi-bundle.sh / run-pi-kimchi.sh. - re-verified (comment stays accurate at 0.85.1): model-guard tokens:null semantics; pi-coding-agent-utils and inline-compact type shims. Co-Authored-By: Kimchi <noreply@kimchi.dev>
Co-Authored-By: Kimchi <noreply@kimchi.dev>
…el e2e fixtures for 0.85.1
pi-coding-agent 0.85.1 made AgentSession.setModel/reflection persist only
when explicitly requested. This exposed a Kimchi-side regression our Step-11
persist sweep missed: `RouterExtension`'s session_start handler calls
`pi.setModel(ctx.model)` to record the CLI `--model` selection, and under
0.84.1 that silently wrote `settings.defaultProvider/defaultModel`. Under
0.85.1 the call became a no-op for persistence, so sessions launched with
`--model` no longer recorded the default — caught by two auto-model TUI
e2e tests that assert the settings round-trip.
Fixes:
- src/extensions/router/index.ts: pass `{ persist: true }` for the
user-initiated CLI model recording path (matches the 0.84.1 semantics
restored elsewhere in Step 11; auto-restore paths stay session-only).
- src/extensions/router/index.test.ts: update the two recording
assertions to expect the persist option.
- tests/e2e/tui/auto-model.test.ts: accommodate upstream 0.85.1 UI
changes:
- model selector gained a current-model "✓ "/two-space marker
column, so the highlighted row now renders as
"→ auto [kimchi-dev]";
- plain-select persist restores "Default model: <p>/<id>" status
(upstream closure), replacing 0.84.1's "Model: <id>";
- upstream removed the global "Thinking level" /settings row in
favor of /thinking + per-model overrides, so the non-reasoning
test drives /thinking and asserts only "off · No reasoning" is
exposed for the routed model (dropping navigateToSetting).
Re-ran `node scripts/run-tui-e2e.js auto-model` green (13/13) plus the
three MCP e2e files and ferment-phase-review standalone green.
Co-Authored-By: Kimchi <noreply@kimchi.dev>
… model selector
Two new upstream surfaces from 0.85.1 had zero behavioral coverage; add
focused TUI e2e tests in tests/e2e/tui/thinking-level.test.ts:
1. /thinking switches the in-session thinking level: walks the real
selector (which now advertises Ctrl+S for default persistence), picks
"high", and asserts (a) the status line updates, (b) the next chat
request carries reasoning_effort=high over the wire, and (c) neither
settings.defaultThinkingLevel nor modelThinkingLevels was written —
Enter is explicitly session-only here.
2. /settings "Default thinking level per model" submenu (new in 0.85.1,
replacing the removed global "Thinking level" row): drives the
two-step SteppedSubmenu, sets a per-model override for the current
model, and asserts settings.modelThinkingLevels persisted as
{ "kimchi-dev/reasoner": "high" }.
Also remove the now-deceptive "Ctrl+S to set as default" hint from the
patched model selector: in Kimchi, plain Enter already persists the
selection (0.84.1 semantics restored via the interactive-mode
selectModel closure patch), so there is no distinct Ctrl+S affordance
to advertise. Mirror matches in patches/@earendil-works__pi-coding-agent@0.85.1.patch
(+2-line hunk, downstream hunk offsets renumbered; verified
git-apply --check against pristine 0.85.1 dist).
Co-Authored-By: Kimchi <noreply@kimchi.dev>
Commit 02eb912 edited patches/@earendil-works__pi-coding-agent@0.85.1.patch in place (dropping the "Ctrl+S to set as default" hint from the model selector); pnpm hashes patchedDependencies into the lockfile, so frozen CI installs failed with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. `pnpm install --no-frozen-lockfile` regenerates the hash only (no other dep changes). Co-Authored-By: Kimchi <noreply@kimchi.dev>
- Router entry: the CLI-recording path relied on upstream's unconditional persistence at 0.84.1, not a "silent" persist. Records that the e2e assertions that caught it were pre-existing and untouched by the fix. - Removes links into .kimchi/docs (gitignored, so they would be dead references in a committed file) and the internal step reference.
Replace version archaeology ("0.84.1 semantics", "persisted at 0.84.1,
keep persisting") with what the code is actually expressing: whether a
given model change reflects a user's choice and should therefore become
their default.
Also generalises the steering.ts note so it describes steer()'s failure
surface rather than pinning a runtime version, with a reminder to
re-check it on upgrade.
Version history belongs in patches/CHANGELOG.md and git, not inline —
it expires, while the intent does not.
The sessionId injection belongs to showModelSelector, which declares `const sessionId = this.sessionManager.getSessionId()` in scope. The 0.85.1 rebase rewrote that use-site for the new constructor signature but also left the old 0.84.1 hunk, whose trailing context (`}, initialSearchInput);`) fuzzy-matched onto showLoginProviderSelector — a different method with byte-identical context and no such binding. That made sessionId a free variable, so the selector threw `ReferenceError: sessionId is not defined` whenever it rendered. It typechecked and the unit suite passed: the login tests stub the selector, and Kimchi's own /login menu does not route through it. The reachable path is `/login <prefix>` where the prefix matches two providers with different ids, which delegates to upstream. OAuthSelectorComponent takes five parameters, so the argument was inert even had the variable existed — the hunk is deleted, not repaired. Adds a regression test that reads the installed dist and pins the declaration and the use to the same method; verified to fail when the hunk is re-introduced. A grep count alone would not catch this, since the branch kept one use-site while moving the other.
The comment explained the "Default model:" status line in terms of 0.84.1 semantics being restored by our patch. The assertion on the next line already states the expected text, and the 0.84.1 framing dates the comment to an upgrade that is now history.
Upstream 0.85.1 gave both setModel and setThinkingLevel a
ModelMutationOptions parameter, and the rebase threaded { persist }
through both. Only setModel has a caller that needs it.
Kimchi's single setThinkingLevel call site is src/extensions/tags.ts
(set_phase adjusting reasoning depth for a workflow phase). That is
tool-driven rather than user-initiated, so session-only is the correct
behaviour and the call deliberately passes no options — leaving the
plumbing with no caller in all three layers.
Pi's own /thinking and /settings reach the class method directly
(this.session.setThinkingLevel(level, { persist })), so neither is
affected by removing the extension-side parameter.
Reverts the setThinkingLevel half in agent-session.js, loader.js and
types.d.ts to upstream; setModel is untouched. The patch header records
the omission as deliberate so the next rebase does not "restore" it.
Also aligns login-selector-scope.test.ts with the projectRoot + resolve
idiom already used by oauth-page-patch.test.ts for reading installed
dist files.
mszostok
force-pushed
the
upgrade-pi-85.1
branch
from
September 17, 2026 10:12
79c2c26 to
c22b603
Compare
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
2 participants
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 does this PR do?
Manual tests
ferment+ferment-v2Pi follow-ups
earendil-works/pi#9639