fix(meta): support pi 0.85 (peer range + 0.85.1 devDeps + legacy-shape test casts) - #18
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe package now supports pi versions from 0.83.0 through 0.85.x. Development dependencies use 0.85.1. Four test fixtures use casts compatible with the updated ChangesPi 0.85 compatibility
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The Pi 0.85 compatibility update preserves the supported runtime paths and introduces no established merge-blocking risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Apply upstream BlockedPath#18: - widen peer range to >=0.83.0 <0.86.0; bump devDeps to 0.85.1 - cast legacy 0.83-shape refresh fixtures as unknown as RefreshModelsContext - probe reasoning.encrypted_content entitlement once per key per process (~16-token /v1/responses call); strip the include for unentitled keys so requests never 400, re-probe transient failures after a 5-minute cooldown - before_provider_request hook is now async and resolves the active key through ctx.modelRegistry.getProviderAuth
BlockedPath
left a comment
There was a problem hiding this comment.
The Pi 0.85 peer-range and legacy fixture changes look sound. At this head, typechecking and hermetic tests passed (35 passed, 2 live tests skipped). However, the added encrypted-reasoning probe changes runtime behavior beyond the compatibility update and need the fixes below. Prefer splitting the probe work into a separate PR so the compatibility update can land independently, or fix it here with hook-level and multi-turn regression coverage. Please also rebase on main and preserve the max-effort support now merged in #13. Findings below were reproduced with mocked requests; no live or billable Meta requests were made.
| // probeEncryptedReasoningEntitlement). Other include entries survive. | ||
| if (!keepEncryptedReasoning && Array.isArray(body.include)) { | ||
| const include = body.include.filter( | ||
| (item) => item !== "reasoning.encrypted_content", |
There was a problem hiding this comment.
[P2] Handle encrypted reasoning replay, not just the output include. When keepEncryptedReasoning is false, this only removes include; Pi still serializes historical thinkingSignature items into input with their encrypted_content intact. A mocked multi-turn wire reproduction confirms that ciphertext from the previous caller remains in the outgoing request after this helper runs. Resuming a session under a caller that cannot replay that ciphertext is therefore not protected by this mitigation. Add safe handling for incompatible historical reasoning items and a regression test that includes assistant history, while preserving ordinary messages and tool calls.
| ) { | ||
| return; | ||
| } | ||
| if (Date.now() - entitlementCache.lastAttemptAt < PROBE_RETRY_MS) return; |
There was a problem hiding this comment.
[P2] Scope the retry cooldown to the key being probed. lastAttemptAt is global: after a successful probe for key A, switching to key B within five minutes returns here without ever probing B. keepEncryptedReasoningFor(B) then returns false and strips the include throughout that window, even if B is entitled. This was reproduced with the registered hook and a mocked successful probe: switching to B left the probe count at one. Keep attempt timestamps and results per key (or reset the cooldown on a key change), and test immediate key rotation as well as transient retries.
….85.1 RefreshModelsContext dropped the 0.83 store read/write pair in pi 0.84 in favor of immutable stored snapshots plus generation-checked publish, and pi 0.85 keeps that shape. The extension's runtime probing already handles both, but the peer range (<0.85.0) rejected pi 0.85 installs and the test suite couldn't verify them. - Widen peerDependencies to <0.86.0 - Bump devDependencies to 0.85.1 so CI typechecks+test against current pi - Cast the four legacy store-shape fixtures (as already done for the 0.84 shape) so dual-shape runtime coverage is preserved
af5a9c3 to
075a03e
Compare
BlockedPath
left a comment
There was a problem hiding this comment.
Re-reviewed the compatibility-only revision at 075a03e. The probe changes and associated findings have been moved out of this PR; only package.json and four legacy fixture casts remain. Rebased on current main with #13 preserved. Typecheck and 31 hermetic tests pass on both Pi 0.83.0 and 0.85.1 (2 live tests skipped in each run). GitHub Typecheck and test CI also passes. Approving this narrowed scope; the deferred probe work is not approved.
|
Merged the compatibility-only revision after rebasing onto main, successful local validation on Pi 0.83.0 and 0.85.1, and green GitHub CI. The three removed encrypted-reasoning commits are preserved with original authorship in draft #19; its review findings remain open there. Thanks for the compatibility fix! |
Fixes #17
Scope
Compatibility-only update, rebased on current
mainincluding #13:>=0.83.0 <0.86.0.store-shape test fixtures throughunknown, preserving their runtime coverage alongsidestored/publishcoverage.No provider runtime or prompt-cache behavior changes are included. The encrypted-reasoning probe commits have been preserved separately on
deferred/meta-encrypted-reasoning-probefor a draft follow-up addressing the review findings.Verification
At commit
075a03e0b476778461bd3610b97f10592450c54e:bun run typecheckpasses;bun test: 31 pass, 2 live tests skipped, 0 fail.bun run typecheckpasses;bun test: 31 pass, 2 live tests skipped, 0 fail.