fix(pi): honest capability warning when the host lacks ctx.isProjectTrusted - #1355
Merged
Merged
Conversation
The capability-absent warning told every host to update Pi, but forks that never implemented ctx.isProjectTrusted (oh-my-pi) also hit this path, and update Pi is wrong advice there. Neither Pi's nor oh-my-pi's extension context exposes a host name or version, so the two audiences cannot be reliably told apart at runtime. The warning now states the capability gap without guessing the host, and says what still works: bundled and global config load regardless (only project-local config is trust-gated in loadPlannotatorConfig). Fail-closed behavior is unchanged: capability absent still skips .pi/plannotator.json, a host-provided true is still honored verbatim (the oh-my-pi shim in can1357/oh-my-pi#7958 will work unmodified), and a throwing trustFn still propagates. Tests pin all four paths. Reported by @materemias in #1353.
1 task
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.
TLDR: The Pi extension's capability-absent trust warning told every host to "update Pi". That is correct advice on a real Pi older than 0.79.1, but wrong and confusing on forks like oh-my-pi (17.4.0) that never implemented
ctx.isProjectTrusted(). The two cases are not reliably distinguishable at runtime, so the warning now states the capability gap honestly for both audiences, without guessing the host. Fail-closed behavior is byte-for-byte unchanged; only the warning text changed.New message:
Closes #1353. Thanks to @materemias for the report.
The two audiences
ctx.isProjectTrusted()shipped in Pi commitdb3f9953e("feat(coding-agent): expose project trust to extensions", closes pi#5523), whose earliest release tag isv0.79.1, confirmed by Pi's CHANGELOG entry under[0.79.1] - 2026-06-09. For this audience "update Pi" was correct.isProjectTrusted; itsExtensionContext(packages/coding-agent/src/extensibility/extensions/types.ts:454-524, built inrunner.ts:1146-1194) simply never adopted the capability. "Update Pi" is wrong advice here: no update fixes it, and the user is not even running Pi.Why one message instead of host detection
Neither host gives an extension a reliable identity from the context:
ExtensionContext(packages/coding-agent/src/core/extensions/types.ts:307-348) has no version or app-name field. The branding constants Pi itself uses for fork detection (APP_NAME,PACKAGE_NAMEinsrc/config.ts:488-491) are not exported to extensions.ExtensionContextlikewise exposes no host name or version. Identity exists only via thepiAPI object (pi.pi.VERSION) or shape sniffing (pi.zod,ctx.invokeTool), all heuristic and version-fragile, and a fork resolving the host package resolves to its own values anyway.Since detection cannot be proven reliable, the message is one sentence pair that is true for both audiences: it names the missing capability and the Pi version that introduced it (so a real-Pi user knows to update), and says what still works. The "bundled and global config still load" claim is verified in code:
loadPlannotatorConfig(apps/pi-extension/config.ts:240-271) always loads the internal and global configs and gates only project-local.pi/plannotator.jsononprojectTrusted.Behavior and capability contract: unchanged
true: project-local config loads. This is both the trusted real-Pi path and the OMP path once fix(extensions): expose ctx.isProjectTrusted for legacy pi extensions can1357/oh-my-pi#7958 ships itsisProjectTrusted: () => trueshim; that shim adds a plain function on both of OMP's context builders, and ourtypeofprobe works through OMP's prototype-delegated handler contexts, so the existing guard works there with no change on our side.false: still fails closed, no warning (the host answered; that is not a capability gap).runner.assertActive): still propagates, config loading never runs, project-local config still cannot load. Now pinned by a test so a future try/catch cannot silently flip it to trusted.Nothing else in the extension breaks under OMP's context shape: the only members we touch (
ui,hasUI,cwd,sessionManager,modelRegistry,model,isIdle,isProjectTrusted) all exist on OMP's context except the feature-detectedisProjectTrusted.Tests
falsefails closed with no capability warning.isProjectTrustedpropagates and project config stays unloaded.bun test apps/pi-extension: 224 pass; the only 6 failures are the pre-existing environmental "pi review server" GitButler/semantic-diff/workspace failures, identical before and after this change.bun run typecheckgreen.Smoke: real Pi 0.84.1 (installed binary) loaded the modified extension from source in both trusted (
--approve) and untrusted (--no-approve) print-mode runs; both reached the model call with no capability warning and no errors (a control run without-erejects the extension-registered--planflag, proving the extension loaded). The warning path itself requires an old Pi or an OMP host, neither of which is runnable in this environment (installed Pi is 0.84.1; OMP has no binary and its checkout is read-only), so that path is covered by the unit tests with context doubles derived from the two checkouts' actual shapes.AI-assisted (Claude) under maintainer direction.