Load user Pi configuration in the bundled runtime - #1080
Conversation
|
🚨 SLOP COP 🚨 · I am SlopCop, and I am reviewing this pull request under the The stack contains only PR #1080: Load user Pi configuration in the bundled runtime. I will review security, code quality, performance, architecture, duplication, and end-to-end behavior. |
|
🚨 SLOP COP 🚨 · Security phase complete. I found one high-severity defect. BB skips the Pi project trust check in both new service paths. An untrusted repository can run project extension code during The final review will include the exact source line and required correction. |
|
🚨 SLOP COP 🚨 · Code quality and architecture phase complete. I confirmed the high-severity trust defect with a direct execution test. I also found a medium-severity error-reporting defect. The bridge checks The model picker and session paths also duplicate the complete Pi service setup. A shared policy helper should resolve trust and collect errors. Focused tests, typechecks, the host build, and the bundle check passed. |
|
🚨 SLOP COP 🚨 · Performance and end-to-end phase complete. The packaged-runtime end-to-end test passed in 1 minute 56 seconds. This change has no browser route, so a browser test does not apply. Each thread now creates a new model runtime and repeats all Pi configuration scans. A 20-session test used about 4 MiB more heap than shared model runtimes. Cold bridge start time increased by about 54 percent. The npm download also grew by at least 4.8 MB. |
| }, | ||
| }); | ||
|
|
||
| const serviceErrors = services.diagnostics.filter( |
There was a problem hiding this comment.
🚨 slopcop/review — Report resource loader failures from the session path
This check also misses extension and resource loader errors. A broken project extension can silently disappear while the session continues with fewer models or tools. Use one shared Pi service helper to collect all failures for both the model and session paths. Add a test that proves the thread reports a broken extension.
| // Use the full service path here too. This adds models from configured Pi | ||
| // extensions to BB's model picker. The bridge process starts in the current | ||
| // workspace, so the loader also sees that workspace's .pi configuration. | ||
| modelRuntimePromise ??= createAgentSessionServices({ |
There was a problem hiding this comment.
🚨 slopcop/review — Use the requested project path for project extension models
The normal daemon model list starts this bridge in provider-maintenance-workspace. Thus, process.cwd() cannot see a project .pi directory. The smoke test starts the bridge inside a project and hides this mismatch. Pass the project path through the model-list contract, or limit the documentation to global extension models. Increment the daemon protocol version if the wire contract changes.
| cwd: process.cwd(), | ||
| }) | ||
| .then((services) => { | ||
| const errors = services.diagnostics.filter( |
There was a problem hiding this comment.
🚨 slopcop/review — Report resource loader failures from the model-list path
services.diagnostics does not contain all extension and resource errors. A direct syntax-error test produced zero service diagnostics and one extension error. The model then disappears without a useful message. Collect resource loader and settings errors with the service diagnostics. Add a failed-extension test.
| // Pi's service factory reads the global and project settings files. It also | ||
| // discovers packages, extensions, skills, prompts, themes, context files, | ||
| // auth, and custom models from the user's normal Pi directories. | ||
| const services = await createAgentSessionServices({ |
There was a problem hiding this comment.
🚨 slopcop/review — Resolve project trust before this factory loads project resources
Pi 0.84 uses projectTrusted: true by default. A repository can execute .pi/extensions code with the provider process host access. Resolve the saved or global trust policy before this call. Keep unresolved projects untrusted. Add tests for rejected, saved, and unresolved trust decisions.
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Final review complete for stack PR #1080.
I found one high-severity defect and two medium-severity defects.
- High: The session path trusts project Pi resources by default. Repository extensions can run with host access.
- Medium: The normal model picker runs in a maintenance directory. It cannot see project extension models.
- Medium: Both service paths ignore resource loader failures. Broken extensions and resources can disappear without a useful error.
The model and session paths duplicate the complete Pi service setup. Use one helper for trust, diagnostics, and service creation.
The performance tests found a 54 percent cold-start increase. The npm download grew by at least 4.8 MB. These are documented tradeoffs.
The packaged-runtime end-to-end test passed. Focused tests, typechecks, the host build, and the bundle check also passed.
GitHub failed several other jobs during action downloads. Those failures did not run product code.
The GPT-5.6 review gate returned REQUEST CHANGES. I used only the comment review control, as required.
2287ea9 to
fae3d1f
Compare
fae3d1f to
9aebfa8
Compare
1 participant
Summary
Why
The bridge created in-memory settings and bundled Pi into one file. This bypassed the user's Pi configuration.
The single-file package also prevented extensions from sharing the host Pi modules.
Impact
Users keep their Pi settings and extensions while BB controls the Pi package version.
Users do not need an installed Pi executable.
Validation
pnpm exec turbo run typecheck --filter=@bb/agent-runtime --filter=@bb/host-daemon --filter=@bb/server --filter=bb-app --forcepnpm exec turbo run smoke:tarball --filter=bb-app --force