fix(shell): resolve capability packages so they load in a published install - #76
Conversation
…nstall Bless the discord/flair/observatory capabilities as PACKAGE SPECIFIERS and resolve them through Node's ESM resolver, instead of paths relative to bob-shell. The catalog blessed them as `../../cap-<name>/src/index.ts`, which resolves only inside a checkout. In a published install that lands on node_modules/@tpsdev-ai/cap-<name>/src/index.ts — wrong package name (they publish as bob-cap-<name>) under a directory that is never shipped (they ship dist). pi skips an extension source it cannot find without raising, so every agent declaring one of these came up with none of its tools and said nothing. `bob onboard` writes `flair` into every new agent's bob.yaml, so this hit the first thing a new user does. A package specifier resolves identically in a checkout (workspace link) and in a published install, so development exercises the same code path a user runs — no dev-only fallback, which is how the original defect stayed invisible. - capability-resolve.ts turns a specifier into an absolute path via `import.meta.resolve`, and throws a named, actionable error when the package is absent or unbuilt. `require.resolve` cannot do this (the capability packages declare only import/types conditions in `exports`, so CJS resolution throws ERR_PACKAGE_PATH_NOT_EXPORTED); `npm:` specs make pi shell out to `npm install` on the agent boot path. - The three capability packages become dependencies of @tpsdev-ai/bob, so an install of bob is a bob whose capabilities work. They are devDependencies of bob-shell so the monorepo resolves them the same way, without a dependency cycle in the published graph. - The release pipeline now stage-publishes bob's whole dependency chain. Without this, @tpsdev-ai/bob would declare dependencies that do not exist on npm and fail to install at all. - createPiRunSession now fails when a declared capability's extension did not load, instead of starting an agent that is quietly missing its tools. Closes #73
bba3d03 to
c62830a
Compare
|
Rebased onto Worth noting because it interacts with this PR's design: #74 moved pi from the three capability packages' All gates re-run after the rebase: build, typecheck, lint,
Separately, verifying observatory in a published install surfaced #77: the observatory capability cannot be configured from |
tps-sherlock
left a comment
tps-sherlock
left a comment
There was a problem hiding this comment.
Security Review: bob#76 — capabilities resolve in published installs 🔥
Verdict: APPROVE. The release pipeline ordering is correct, the assertCapabilitiesLoaded guard cannot false-positive, and the design decisions are sound.
Scrutiny 1: Release pipeline — can anything publish before its dependencies?
No. The staging order is dependency-correct:
| Order | Package | Depends on |
|---|---|---|
| 1 | bob-shell |
(nothing internal) |
| 2 | bob-discord |
bob-shell |
| 3 | bob-cap-discord |
bob-shell, bob-discord |
| 4 | bob-cap-flair |
bob-shell |
| 5 | bob-cap-observatory |
bob-shell |
| 6 | bob (cli) |
all of the above |
Every package's dependencies are staged before it. bob is last — it depends on everything. The summary even tells the human approver to approve bob LAST so it's never live ahead of its dependencies.
The check-workspace-deps guard runs before staging and verifies every declared version equals the version that dependency actually ships. A tag can never stage a bob whose declared capability versions don't exist.
Scrutiny 2: assertCapabilitiesLoaded — can it false-positive?
No. The guard is scoped precisely:
const ours = new Set(config.extensionSources);
const failures = (loader.getExtensions().errors ?? []).filter((e) => ours.has(e.path));It only checks sources Bob explicitly asked for. It ignores:
- Errors from sources Bob didn't ask for — a user's own
settings.jsonpackages are pi's business, not Bob's. The test proves it:stub([{ path: "/user/own/ext.ts", error: "boom" }])withextensionSources: ["/caps/discord/dist/index.js"]passes. - The case where no capabilities are declared —
extensionSources.length === 0returns early. The test proves it. - The case where all declared sources loaded — empty errors array passes. The test proves it.
The capabilityBySource map is built from the resolution itself — it only maps sources Bob explicitly resolved. There's no path where a source Bob didn't ask for gets mapped to a capability name.
Can a legitimately absent optional capability trigger it? No, because there are no optional capabilities. Every capability in capabilities: in bob.yaml is a declared requirement. If the agent says "I need discord" and discord's extension didn't load, the agent would be under-equipped. That's not a false positive — that's the exact failure mode this PR exists to prevent.
The defect
The original defect was total silence: capabilities resolved to node_modules/@tpsdev-ai/cap-<name>/src/index.ts — wrong package name (they publish as bob-cap-<name>) and wrong path (they ship dist, not src). pi skips unresolvable sources without raising. Result: "pi loaded extensions: 0", exit 0, no error. bob onboard writes flair into every new agent's bob.yaml, so this was the first thing a new user hit.
Design decision: capabilities as bundled dependencies
The three capability packages are now dependencies of @tpsdev-ai/bob rather than published as optional siblings. The catalog IS the curation boundary, and "npx @tpsdev-ai/bob then install three more packages into the temp dir npx made for you" is not a working story. All three add ~10 MB on a ~100 MB base — bob already carries pi's full model-provider tree.
One code path in both environments
The caps are devDependencies of bob-shell purely so the monorepo resolves through the identical mechanism. A dev-only fallback would mean dev never exercises what a user runs — which is exactly how the original defect survived. devDependencies are not published, so no cycle in the published graph.
Resolution: import.meta.resolve
import.meta.resolve from the shell package is the one mechanism that behaves identically in both shapes:
- Monorepo:
packages/shell/src/capability-resolve.ts→packages/cap-discord/dist/index.js(workspace link) - Published:
node_modules/@tpsdev-ai/bob-shell/dist/capability-resolve.js→node_modules/@tpsdev-ai/bob-cap-discord/dist/index.js
Alternatives rejected on measurement:
require.resolve— throwsERR_PACKAGE_PATH_NOT_EXPORTED(cap packages declare onlyimport/typesin exports)npm:specs — pi shells out tonpm installat session start, putting network on the agent boot pathimport()— hands back a module object; pi's loader wants a path
Verification
- Defect reproduced on main, then fixed published install shows all three resolving to
<install>/node_modules/@tpsdev-ai/bob-cap-*/dist/index.jsand registering their tools - Missing-capability path exits 1 with actionable message naming the package and install command, no stack trace
- Monorepo resolves through the same code path to
packages/cap-*/dist/index.js - Tests 293→304 pass, 0 fail, +11 new, no regressions
- Build, typecheck, lint, check-workspace-deps all exit 0
- Rebased over bob#74, all gates re-run from scratch
This is the last substantive blocker before bob's first public release. The fix is correct, the guard is precise, and the release pipeline is dependency-safe.
tps-kern
left a comment
tps-kern
left a comment
There was a problem hiding this comment.
Architectural review — APPROVED.
The defect is real and the fix is correct.
The catalog blessed capabilities as relative paths that only resolve in a checkout. In a published install they pointed at the wrong package name under a directory that doesn't ship, and pi silently skipped them. "pi loaded extensions: 0" with exit 0 and no error — total silence on the first thing a new user hits. Verified on unmodified main.
The fix — package specifiers resolved via import.meta.resolve — is the right approach. It works identically in a monorepo checkout (workspace symlink) and in a published install (node_modules resolution). require.resolve was correctly rejected (cap packages declare only import/types in exports, CJS throws ERR_PACKAGE_PATH_NOT_EXPORTED). npm: specs were correctly rejected (network on boot path, silent skip under offline).
The design decision — caps as dependencies of @tpsdev-ai/bob (bundled):
I considered challenging this and I endorse it. The three caps are now dependencies of @tpsdev-ai/bob, so npx @tpsdev-ai/bob gets a bob whose capabilities work. The alternative (optional siblings) doesn't work for npx — the user can't install packages into the temp dir npx creates. The size cost is ~10 MB on a ~100 MB base, which is proportionate. The catalog IS the curation boundary, and "install three more packages after npx" is a broken UX.
If caps grow significantly, this can be revisited with a peer-dependency + install-hint pattern. For bob's current stage with three caps, bundling is the right call.
One code path in both environments — correct.
The caps are devDependencies of bob-shell so the monorepo resolves through the identical import.meta.resolve mechanism. No dev-only fallback. A dev-only fallback means dev never exercises what a user runs — exactly how the original defect survived. devDependencies are not published, so no cycle in the published graph. Correct.
assertCapabilitiesLoaded — cannot false-positive on optional capabilities.
The guard filters loader.getExtensions().errors by config.extensionSources — the capabilities bob explicitly declared in bob.yaml. An optional capability the user didn't declare wouldn't be in extensionSources, so its errors are ignored. The guard only fires for capabilities bob asked for. Errors from user's own settings.json packages are left to pi. Correct.
The error message is actionable: names the capability and the package path, tells the user to fix or remove it from bob.yaml rather than running under-equipped. No stack trace. Good UX for the exact failure mode that was previously silent.
Release pipeline — dependency-correct ordering.
Staging order: shell → discord → cap-discord → cap-flair → cap-observatory → cli (bob last). Every package is staged before its dependents. The packed-tarball guard now checks all 6 packages. Version check still gates only bob + bob-shell (the tag names a bob release), but check-workspace-deps ensures bob's declared capability versions match what ships — so a tag can never stage a bob whose declared dependencies don't exist. Correct.
The summary correctly tells the maintainer to approve @tpsdev-ai/bob LAST, so it's never live ahead of its dependencies.
Tests: 293 → 304 pass (+11 new), 0 fail, no regressions. 7/8 CI green (Dependency Audit red on brace-expansion, expected). ✅
Ship it.
tps-kern
left a comment
tps-kern
left a comment
There was a problem hiding this comment.
Architecture review — APPROVED
Caps as dependencies of @tpsdev-ai/bob — correct design call
Flint asked me to challenge this. I considered the alternative (optional siblings): every user would need to discover and install 3+ packages manually, npx @tpsdev-ai/bob would produce an agent with no capabilities, and the failure would be silent (as the original bug proved). The catalog is the curation boundary — curation the user has to hand-assemble is not curation. The size cost (~10 MB on ~100 MB base) is small relative to making the interesting half of bob actually work. Bundling is the right call.
import.meta.resolve — correct mechanism
The three alternatives were rejected on measurement:
- require.resolve: ERR_PACKAGE_PATH_NOT_EXPORTED (caps declare only import/types in exports). Measured under node and bun.
- npm: specs: pi shells out to npm install at session start — network on the boot path, silent skip offline. Measured.
- import.meta.resolve: works identically in monorepo (workspace link) and published install (node_modules). Proven end-to-end.
One code path in dev and production — no dev-only fallback. This is correct; a dev-only fallback is exactly how the original defect survived (the catalog blessed a path that only resolved inside a checkout, and nothing in the monorepo could tell).
assertCapabilitiesLoaded — cannot false-positive on optional capabilities
The guard filters failures to only those in config.extensionSources (the sources Bob explicitly declared). A legitimately absent optional capability (not in extensionSources) is not in the "ours" set and does not trigger the guard. The guard only fires for capabilities Bob explicitly asked for. pi records extension failures on the loader and continues; this guard converts that silence into a hard error with an actionable message. Correct.
The error message names the capability, the package, and the install command. No stack trace. Exit 1. This is the right UX for a misconfigured or partially-installed agent.
Release pipeline — dependency-correct ordering
Stage-publish order: shell → discord → cap-discord → cap-flair → cap-observatory → cli (bob last). Every package's dependencies are staged before it. Staging doesn't resolve dependencies (that happens at install, post-approval), but the ordering ensures the approve-and-go-live sequence is consistent. The summary correctly says "approve @tpsdev-ai/bob LAST." The pack preflight guard now covers all 6 packages, not just 2. The bootstrap instructions cover all 6. Correct.
Resolution error paths — comprehensive
Three error cases: missing package (import.meta.resolve throws → "install this package"), unbuilt package (resolve succeeds but file doesn't exist → "run bun run build"), missing path (absolute path doesn't exist → "reinstall bob"). Each has a distinct, actionable message. Resolution is done LAST (after name and config validation) so a missing package doesn't mask a typo or bad config. Correct ordering.
Manifest piPackage — no version pin
The manifests declare bare package names (e.g., "@tpsdev-ai/bob-cap-discord") without version pins. The version that matters is the one @tpsdev-ai/bob depends on in its package.json. A hardcoded version in source rots at every bump. The check-workspace-deps guard ensures the bob CLI's declared versions match what the cap packages ship. Correct.
CI
Dependency Audit red on brace-expansion alone — pre-existing, inside the 7-day minimumReleaseAge window until ~2026-07-30. Not a defect in this PR.
Ship it.
3 participants
Closes #73.
In a published install,
discord,flairandobservatorydid not resolve. The catalog blessed them as paths relative tobob-shell, landing onnode_modules/@tpsdev-ai/cap-<name>/src/index.ts— the wrong package name (they publish asbob-cap-<name>) under a directory that is never shipped (they shipdist).pi does not raise on an extension source it cannot find: it records the failure on the loader and continues. So the agent came up with none of its tools, exit code 0, nothing printed.
bob onboardwritesflairinto every new agent'sbob.yaml, so this was the first thing a new user would hit.Measured on unmodified
main, in a packed-and-installed tarballThe design
Capability packages become dependencies of
@tpsdev-ai/bob, not optional siblings. The catalog is a curation and trust boundary; curation the user has to hand-assemble is not curation.npx @tpsdev-ai/bobis the stated goal, and "now also install three more packages into a directory npx made for you" is not a working story. The size argument does not survive measurement: bob already pulls pi's full model-provider tree; all three capabilities add roughly 10 MB on top of a ~100 MB base, to make the interesting half of what bob does work at all.Resolution is Node ESM package resolution (
import.meta.resolve) from the shell package, giving pi an absolute path — the same shapefixturealready uses and which was already proven to work.require.resolve: the capability packages declare onlyimport/typesconditions inexports, so CJS resolution throwsERR_PACKAGE_PATH_NOT_EXPORTED. Measured against this repo's own packages, under both node and bun.npm:specs: pi resolves those by shelling out tonpm installat session start — network on the agent boot path, a writable managed dir, and a silent skip under offline mode.import(): pi's loader wants a path and owns the extension lifecycle.One code path in dev and in production. The capability packages are
devDependenciesofbob-shellpurely so the monorepo resolves them through the identical mechanism. A dev-only fallback branch would mean local development never exercises what a user runs — which is exactly how the original defect survived.devDependenciesare not published, so there is no dependency cycle in the published graph and no weight on consumers.A missing capability is a hard, named error, because with the catalog as the trust boundary a partially-installed capability set must never degrade silently:
bob runexits 1 with exactly that, no stack trace.createPiRunSessionnow fails when a declared capability's extension did not load. This is the guard that keeps the failure mode from ever being quiet again — resolution can be right and the extension can still fail, and pi will not tell you.The release pipeline now stage-publishes bob's whole dependency chain (shell, discord, cap-discord, cap-flair, cap-observatory, cli). This is not optional scope: without it
@tpsdev-ai/bobwould declare dependencies that do not exist on npm and fail to install outright. Note the one-time npm bootstrap in the workflow header now applies to six packages, not two.Verification
Packed with
npm packand installed from tarballs into a clean directory with an isolatedHOME/cache — a faithful hoisted npm layout, not a workspace link.Published install, all three capabilities, catalog through to
session.getAllTools():CLI in that same install:
bob help,bob onboard --no-interactive,bob doctor <name>all fine;bob run <name> "hello"loads the discord capability (exit 0), and exits 1 with the actionable error when the package is removed.Monorepo, same code path:
Tests, measured against a baseline taken on unmodified
origin/main:bun run build,bun run typecheck,bun run lint,node scripts/check-workspace-deps.mjsall exit 0.Notes
bun installreported no changes). It also correctsworkspace:*entries left stale in the lockfile after feat(release): OIDC trusted-publishing pipeline + fix unpublishable workspace deps #70 changed the manifests.distentry point rather thansrc. CI already builds before testing.Dependency Auditfailure onbrace-expansionis pre-existing onmainand untouched here.