Pi 0.80.8 spawn runtime compatibility#16
Conversation
|
Implemented the approved CURe repair scope:
Pre-existing child-runtime semantics are tracked separately in #17. Verification is green: typecheck, 430 unit tests, 16 E2E tests, snapshots, audit, exact-floor compatibility, current-Pi compatibility, package-host smoke, and all five PR checks including the expanded Windows compatibility run. |
ofriw
left a comment
There was a problem hiding this comment.
Note: This review was generated by an AI agent. If you'd like to talk with other humans, drop by our Discord!
Nice progress overall, but there’s one blocking portability issue in the new compatibility scripts. Both scripts/test-compat-current.mjs and scripts/test-package-host.mjs derive the repo root with new URL("..", import.meta.url).pathname. That gives you a URL pathname, not a safe filesystem path, so encoded characters like spaces stay encoded and Windows path handling is unreliable. In practice that can break cpSync(root, ...), cwd: root, and tarball path joins depending on where the repo is checked out.
The evidence is straightforward: the project already uses the correct pattern in scripts/run-node-test.mjs (fileURLToPath(import.meta.url) + dirname/resolve), while the two new scripts use .pathname directly. Since these scripts are part of the compatibility proof this PR adds, they need to be robust on the same cross-platform matrix the PR is trying to guarantee. Please switch both scripts to filesystem-path conversion via fileURLToPath(...) and keep the rest of the logic as-is.
Attached is an agent optimized description of the changes in this PR - AGENT_REVIEW.md
|
@ofriw Thanks for the review. This appears to target the snapshot before current PR head 71b95ad (fix(scripts): make compatibility checks portable). The flagged .pathname expressions exist in 71b95ad^, but current HEAD replaces both with repoRootFromScript(import.meta.url), implemented using fileURLToPath(), So the concern was valid for the earlier revision, but is resolved at the current PR head. |
Why this PR exists
Upgrading Pi from 0.80.7 to 0.80.8 broke every
/spawnpath in this extension before a child agent could be created. Pi 0.80.8 removed the old extension-ownedAuthStorage/ModelRegistry.create()session bootstrap and replacedcreateAgentSession({ authStorage, modelRegistry })with the publicModelRuntime-based session API. The existing spawn boundary still constructed the removed objects, so inherited-model and explicitly selected-model invocations both failed withTypeError: Cannot read properties of undefined (reading 'create').Model routing exposed this compatibility break; it did not cause it. The failure happened before route selection could affect child creation, and the original non-routing spawn path used the same obsolete bootstrap. Here, a child session is the isolated Pi agent session that executes a spawned task, while ModelRuntime is Pi's public runtime object for model discovery, provider authentication, and session model access.
User-visible before and after
Before: on Pi 0.80.8,
/spawncould not create a child, whether it inherited the parent's model or received an explicit selection.After: child sessions are created through Pi's supported public runtime/session APIs on Pi 0.80.8 and later. The selected model and thinking level, active tools, notebook access, readonly boundaries, working directory, and transcript isolation are preserved. Child sessions have exactly-once cleanup; renderer recovery remains TUI-safe; and compatibility is checked at the supported floor, against current Pi, and in a packaged host. Parent-only transient provider or credential state fails explicitly rather than silently falling back to another model.
What changed
ModelRuntimeandAgentSessionlifecycle.ctx.modelRegistryonly as Pi's synchronous compatibility facade for extension-side model lookup; it is not used to construct child sessions.Requirements
Acceptance criteria
maxthinking level is accepted and reaches the child unchanged.console.*output, does not block healthy components, and recovers on a later frame.Contract changes
maxthinking level.Out of scope
How to verify
npm ci npm run typecheck npm test npm run test:e2e npm run test:snapshots:check npx audit-ci --config audit-ci.jsonc npm run test:compat:floor npm run test:compat:current npm run test:package-host npm pack --dry-run --ignore-scriptsConfirm that the exact-floor and current-Pi checks report a coherent installed Pi graph, the packaged-host check loads the extension beside Pi 0.80.8 using host-provided peers, and all commands complete without outbound model requests or terminal diagnostics.
References
pi-runtime-compatibilityspawn-pi-0808-runtime-compatibility— Pi 0.80.8 spawn runtime compatibilityopenspec/changes/spawn-pi-0808-runtime-compatibility/