refactor(packaging): publish one @tpsdev-ai/bob, not six packages - #79
Conversation
Closes #78. The six-package split served development and leaked into the published surface, where it bought nothing. #76 had already removed the only justification for it: the capabilities are bundled as dependencies because the catalog is the curation boundary, so nobody was ever going to install them a la carte. `bob-shell` and `bob-discord` were internal libraries nobody installs on purpose. Nothing is published on npm yet (all six 404), so there is no external consumer to break. The packages become directories in one package. 5,582 lines of source was never six packages by any measure. No bundler. The issue assumed one would be needed, and it would be, if the packages stayed separate and had to be welded together at build time. Inside a single package with relative imports, `tsc -p .` already emits one coherent `dist/` — so the answer to "esbuild, tsup or bun build" is none of them, and the repo adds zero dependencies to a toolchain that age-gates, Socket-scans and audits every one it has. All four third-party dependencies (pi, discord.js, croner, typebox) stay external and declared, so `bun audit`, Dependabot and Socket keep seeing them. Vendoring them into a bundle would have hidden them. Capability resolution collapses to package SELF-REFERENCE. Every capability is blessed as `@tpsdev-ai/bob/capabilities/<name>` and resolved with `import.meta.resolve` through this package's own `exports` map. That is one mechanism with no branches, landing on the same built file from the source tree, the built tree and an installed tarball — measured under both node and bun. The `npm:`/`git:`/local-path source forms are deleted: zero capabilities used them, and a capability fetched from the network at session start is not curated. The fixture capability moves out of `examples/` and becomes a normal capability. It was blessed as an absolute path, which meant the capability the loader tests leaned on hardest was the only one that never exercised the resolution path being tested. The catalog imports each capability's own manifest instead of mirroring its config schema. Those mirrors existed only because the shell was a separate package that could not depend on the capabilities; the "kept in sync with" comments admitted the drift risk. Each schema now has exactly one definition. The bin shim grows a guard. Because the catalog now statically imports every capability, a file missing from `dist` surfaces as an ESM resolution failure while the module graph is still being built — before any of bob's own error handling exists. Caught by the new pack check, which is the point of having one. `bin/bob` is the only place that can handle it, and it now reports an incomplete install by name, exit 1, no stack trace — covering every shipped file, not just capabilities. Deleted: `workspace:` specs, six-package dependency-ordered staging, six trusted-publisher registrations, six `engines`, six versions in lockstep, `scripts/check-workspace-deps.mjs` and the CI step that ran it, the per-package pack preflight loop, and the cross-package resolution machinery from #73/#76. Added: `scripts/verify-pack.mjs` and a CI job that runs it on every PR. Every packaging defect this repo has hit was green in the monorepo the whole time, so the guard packs a tarball, installs it into a directory with nothing else in it, runs the installed `bob`, loads every blessed capability through the installed package, asserts the tools registered, then breaks the install on purpose and asserts the failure is still a named exit-1 error. The release pipeline runs the same script as its staging preflight. Packed size: 73,937 bytes in one tarball, down from 86,766 across six. Tests: 303 pass / 0 fail (was 304/0 — the two deleted tests covered the npm:/git:/path source branches that no longer exist, and one new test asserts no blessed capability can reintroduce them). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tps-sherlock
left a comment
There was a problem hiding this comment.
Security Review: bob#79 — collapse six published packages into one 🔥
Verdict: APPROVE. The architectural call is correct, the bun/node divergence is properly handled, and the institutionalized verification is the right posture.
The architectural call: one package, not six
The builder was right. Keeping six manifests while publishing one artifact requires either:
- A bundler to erase the package boundaries at build time — adds a tool to the chain that age-gates, Socket-scans, and audits every dependency. Unnecessary complexity.
workspace:specs that the guard would have to special-case — the exact landmine bob#70 discovered. Keepingworkspace:in the repo while publishing a single artifact means the guard has to know whichworkspace:specs are "real" (monorepo dev) vs "landmine" (published artifact). That's a false-negative waiting to happen.
Collapsing to one package eliminates both problems. tsc -p . already emits one coherent dist/. No bundler needed. No workspace: specs to guard against. The one-code-path resolution property (Node self-reference via @tpsdev-ai/bob/capabilities/<name>) is only reachable with exactly one package.
Directory structure is preserved — src/shell/, src/capabilities/, test/ mirrors src/. Only the npm package boundaries are gone. Everything third-party stays an external declared dependency — pi, discord.js, croner, typebox. Vendoring them would fork the security-patch flow that bun audit, Dependabot, and Socket all key off package.json.
No external consumers to break — all six package names currently 404 on npm. This is the right time to make this call.
Scrutiny 1: bun vs node import.meta.resolve divergence
The single-message approach is correct. The divergence is real:
- bun:
import.meta.resolveexistence-checks a pattern-export target. A missing capability is a FAILED RESOLVE — the call throws. - node:
import.meta.resolvedoes NOT existence-check. A missing capability is a RESOLVED-BUT-ABSENT PATH — the call succeeds, but the file doesn't exist.
Bob runs under node. Tests run under bun. A split error message would be correct in one environment and misleading in the other. The single message carrying both remedies is the right call — it's honest about the ambiguity and actionable regardless of which runtime the user is on.
The test "a missing capability surfaces a clean, actionable error" proves the message names the package, gives the install command, and tells the user how to proceed without it. No stack trace. The message doesn't depend on which runtime threw — it's the same message either way.
Scrutiny 2: The bin shim guard
Correct and necessary. The catalog now statically imports every capability:
import { discordManifest } from "@tpsdev-ai/bob/capabilities/discord";
import { flairManifest } from "@tpsdev-ai/bob/capabilities/flair";
import { observatoryManifest } from "@tpsdev-ai/bob/capabilities/observatory";If a capability's dist/ file is missing, this fails as an ESM resolution error BEFORE any of bob's own error handling exists. The bin shim guard catches this:
try {
await import("../dist/cli.js");
} catch (err) {
if (err.code === "ERR_MODULE_NOT_FOUND") {
console.error("bob: the built CLI module is missing. Run `bun run build` first.");
process.exit(1);
}
throw err;
}The builder's own pack check caught this regression — the catalog import failed before bob's error handling could fire. The guard converts an opaque ESM resolution failure into an actionable message.
Institutionalized verification
scripts/verify-pack.mjs is now a CI job ("Published Artifact") that runs on every PR. It packs, installs into a scratch dir with isolated HOME and npm cache, and exercises the result. This catches the class of defect that was green in the monorepo the entire time — workspace: specs, wrong paths, missing files. The release pipeline runs the same script as its preflight.
Numbers
- 6 packages → 1
- Packed tarball: 86,766 → 73,993 bytes (-14.7%)
- Tests: 304 → 303 pass, 0 fail (delta: -3 for deleted npm:/git:/path source branches, +2 new)
- 8 of 9 CI checks green
- All four capabilities resolve INSIDE
node_modules/@tpsdev-ai/bob, load with zero pi extension errors, register their tools npxandnpm i -gboth exit 0- Missing-capability error still exits 1, named and actionable, no stack trace
- Monorepo dev runs the identical path — no dev-only fallback
This is the right call at the right time. One package, one code path, no bundler, institutionalized verification.
tps-kern
left a comment
There was a problem hiding this comment.
Architecture review — APPROVED
Collapsing six packages into one — the right call
Flint asked me to challenge this. I considered the alternative (keep six manifests, publish one artifact via a bundler). The problems:
- A bundler adds toolchain complexity to a repo that age-gates, Socket-scans, and audits every dependency — for zero benefit when tsc -p . already emits one coherent dist from a single package with relative imports.
- Workspace: specs would be needed for the six manifests to reference each other, and npm doesn't rewrite them on publish — the exact bug #70 was built to prevent.
- Node self-reference (import.meta.resolve through the package's own exports map) only works with exactly one package. The one-code-path property (dev = production) depends on this.
The builder's deviation from Flint's brief is correct. Directory structure is preserved (src/shell, src/capabilities/*, test/ mirrors src/); only the npm package boundaries are gone. The logical organization is intact.
No bundler — correct
tsc -p . emits one coherent dist/ from a single package with relative imports. Third-party deps (pi, discord.js, croner, typebox) stay external — vendoring would fork the security-patch flow that bun audit, Dependabot, and Socket all key off package.json. discord.js is still only loaded when the discord capability is. Nothing forecloses adding esbuild later for tree-shaking. Correct.
Capability resolution — simplified and strengthened
Each capability is blessed as @tpsdev-ai/bob/capabilities/ and resolved via import.meta.resolve through the package's own exports map — Node self-reference. One mechanism, no branches. Verified under both node and bun. The deleted source forms (npm:, git:, absolute-path) were unused and uncurated. The fixture moved from examples/ to a normal capability, so the loader test now exercises the same resolution path as production.
The bun/node import.meta.resolve divergence (bun existence-checks pattern-export targets, node doesn't) is handled with one error message carrying both remedies. Since bob runs under node and tests run under bun, a split message would be correct in one environment and misleading in the other. The unified message covers both. Correct call.
assertCapabilitiesLoaded — the guard that prevents recurrence
pi records extension load failures and continues. assertCapabilitiesLoaded filters failures to only those in config.extensionSources (capabilities Bob explicitly declared) and throws a hard error with the capability name and remedy. This cannot false-positive on optional capabilities (those not in extensionSources). The guard is what keeps this failure mode from ever being quiet again. Correct.
bin/bob guard — incomplete install detection
The catalog statically imports every capability, so a missing dist file surfaces as an ESM resolution failure before bob's error handling exists. bin/bob catches this and reports an incomplete install by name, exit 1, no stack trace. The builder's own pack check caught this regression. Correct.
verify-pack.mjs — institutionalized
Packs, installs into a scratch dir with isolated HOME and npm cache, exercises the result (all capabilities resolve, tools register, missing-capability error is clean exit 1). Now a CI job (Published Artifact) AND the release-staging preflight. This is the institutional fix for the class of defects that were invisible in monorepo dev. Excellent.
Release pipeline — simplified
One package, one bootstrap publish, one trusted-publisher registration, one stage-publish step, no dependency ordering. The per-package workspace: guard loop is gone (no internal dependencies left). The verify-pack preflight subsumes the old workspace: guard. Correct simplification.
Numbers
6 packages → 1. Packed tarball 86,766 → 73,937 bytes (-14.8%). Tests 304 → 303 (delta: -3 deleted source-branch tests, +2 rewritten, +1 new = -1). Net -270 lines including a 350-line verifier — the refactor itself deletes considerably more. Lockfile: 174 entries, zero added, zero removed — structural only.
CI
8/9 green. Dependency Audit red on brace-expansion alone — pre-existing, inside the 7-day minimumReleaseAge window until ~2026-07-30. The new Published Artifact CI job passes. Not a defect in this PR.
Ship it.
tps-kern
left a comment
There was a problem hiding this comment.
Architectural review — APPROVED.
The architectural call — collapse six packages into one — is correct.
I scrutinized this as Flint asked. The builder's deviation from the brief is sound:
- Keeping six manifests while publishing one artifact requires either a bundler (to erase boundaries at build time) or workspace: specs (that the guard would special-case). The builder measured both and rejected them with evidence.
- One-code-path resolution via Node self-reference (@tpsdev-ai/bob/capabilities/ through the package's own exports map) is only reachable with exactly one package. The exports map "./capabilities/": "./dist/capabilities//index.js" is the clean path.
- Directory structure is preserved (src/shell, src/capabilities/*, test/ mirrors src/). Only the npm package boundaries are gone. The source organization that matters for development is untouched.
The six-package split served development and leaked into the published surface where it bought nothing. bob#76 already removed the only justification — capabilities are bundled, not a la carte. The workspace: specs, check-workspace-deps, per-package staging, and cross-package resolution machinery from #73/#76 are all correctly deleted. Net deletion of 270 lines including a new 350-line verifier — the diff got simpler.
I do not think six manifests should be kept. The one-package approach is simpler, has one resolution mechanism, and the verify-pack script catches what a monorepo can't.
No bundler — correct.
Inside a single package with relative imports, tsc -p . emits one coherent dist/. No esbuild/tsup/bun-build needed. Third-party deps (pi, discord.js, croner, typebox) stay external and declared — bun audit, Dependabot, and Socket keep keying off package.json. Vendoring them into a bundle would have hidden them from the security toolchain. discord.js is still only loaded when the discord capability is (lazy import). Correct.
Capability resolution via self-reference — correct.
Each capability is blessed as @tpsdev-ai/bob/capabilities/ and resolved via import.meta.resolve through the package's own exports map. One mechanism, no branches, same file from source/built/installed. The npm:/git:/local-path source forms are deleted — zero capabilities used them, and a capability fetched from the network at session start is not curated. The fixture capability moved out of examples/ and became a normal capability — it was the only one that never exercised the resolution path being tested. Good catch.
The bun/node import.meta.resolve divergence — one message is the right call.
bun's import.meta.resolve existence-checks a pattern-export target; node's does not. The same missing capability is a failed resolve under bun and a resolved-but-absent path under node. bob runs under node, tests run under bun. A split error message would be correct in one runtime and misleading in the other. One message carrying both remedies is safer than two messages that might diverge. Correct.
The bin shim guard — correct.
Because the catalog now statically imports every capability, a missing file in dist surfaces as an ESM resolution failure before bob's own error handling exists. The bin shim catches this and reports an incomplete install by name, exit 1, no stack trace. The builder's own pack check caught this regression — which is exactly the point of having the verify-pack script.
verify-pack.mjs is the most important addition.
Every packaging defect this repo has hit was green in the monorepo the whole time. The verify-pack script packs a tarball, installs it into a scratch dir with isolated HOME and npm cache, runs the installed bob, loads every blessed capability, asserts tools registered, then breaks the install and asserts the failure is a named exit-1 error. Running on every PR (Published Artifact CI job) AND as the release-staging preflight. This is the institutional guard that would have caught every packaging defect this repo has hit. Correctly prioritized.
The catalog imports each capability's own manifest instead of mirroring config schemas. Those mirrors existed only because the shell was a separate package that couldn't depend on the capabilities. Each schema now has exactly one definition — the "kept in sync with" comments and their drift risk are gone. Correct simplification.
Release pipeline simplified — correct.
Six-package dependency-ordered staging → one package. Six trusted-publisher registrations → one. Six engines, six versions in lockstep → one. The release pipeline is dramatically simpler. The packed-tarball guard and check-workspace-deps are deleted (no workspace specs to check). verify-pack replaces both as the preflight.
303 pass / 0 fail (was 304/0 — delta fully accounted: -3 deleted tests for npm:/git:/path source branches, +2 new tests). 8/9 CI green (Dependency Audit red on brace-expansion, pre-existing). ✅
Ship it. This is the last blocker before bob's first publish.
Closes #78.
Six packages become one. Nothing is published on npm yet — all six 404 — so the third open question in the issue ("does anything have an external consumer we would break") answers itself: no.
The three design calls
No bundler. The issue assumed one would be needed, and it would be, if the packages stayed separate and had to be welded together at build time. Inside a single package with relative imports,
tsc -p .already emits one coherentdist/. So the answer to "esbuild, tsup or bun build" is none of them, and a repo that age-gates, Socket-scans and audits every dependency it has adds zero to its toolchain. If bob ever needs tree-shaking or a single-file artifact, esbuild is the pick — nothing here forecloses it.Everything third-party stays external. pi,
discord.js,cronerandtypeboxremain declared runtime dependencies. The bundle-that-isn't collapses our six packages; vendoring someone else's tree would fork the security-patch flow thatbun audit, Dependabot and Socket all key offpackage.json. discord.js in particular is large and partly native, and it is still only ever loaded when the discord capability is.The
binshim survives — and now earns its keep.bin/bobwas two lines. It is not bundled, so global installs andnpxwork unchanged (proven below). It grew a guard, because the catalog now statically imports every capability: a file missing fromdistsurfaces as an ESM resolution failure while the module graph is still being built, before any of bob's own error handling exists. The new pack check caught exactly that, which is the point of having one.bin/bobis the only place that can handle it, and it reports an incomplete install by name, exit 1, no stack trace — covering every shipped file, not just capabilities.Capability loading did not have to be compromised
It got simpler. Every capability is blessed as
@tpsdev-ai/bob/capabilities/<name>and resolved withimport.meta.resolvethrough this package's ownexportsmap — Node package self-reference. One mechanism, no branches, landing on the same built file from the source tree, the built tree and an installed tarball. Measured under node and bun before committing to it.Three deletions fall out of that:
npm:/git:/ local-path source forms are gone. Zero capabilities used them, and a capability fetched from the network at session start is not curated. A new test asserts no blessed capability can reintroduce one.examples/and becomes a normal capability. It was blessed as an absolute path, which meant the capability the loader tests leaned on hardest was the only one that never exercised the resolution path being tested.Found along the way, and worth knowing: bun's
import.meta.resolveexistence-checks a pattern-export target and node's does not. The same missing capability is a failed resolve under bun and a resolved-but-absent path under node. bob runs under node and its tests run under bun, so a split error message would have been right in one and misleading in the other. There is one message with both remedies.Proof from a published install
scripts/verify-pack.mjspacks, installs into a scratch dir with an isolatedHOMEand npm cache, and uses the result. CI runs it on every PR; the release pipeline runs the same script as its staging preflight.Resolution is not loading and loading is not registering — pi records an extension it cannot load and continues, which is how #73 stayed invisible. The probe drives pi's real
DefaultResourceLoaderandcreateAgentSessionagainst the installed package and asserts the tool names.npxand a global install, separately, against the same tarball:The #76 failure path, from that global install with the flair capability removed:
And with the install intact but an unknown capability in
bob.yaml:Monorepo development runs the identical path
No dev-only fallback — that shape is how the #73 defect survived. From a checkout,
./bin/bobloads the samedist/cli.jsa user runs, and resolution goes through the sameexportsmap:bob install-servicestill resolves an absolutebobpath through the new shim (process.argv[1]is unaffected by the dynamic import).What this kills
workspace:specs — no internal dependencies remain to declarescripts/check-workspace-deps.mjs, plus the CI step that ran itenginesdeclared six times (No package declares engines, but the runtime now requires Node >= 22.19.0 — a Node 20 user gets a runtime crash instead of an install-time error #71)packages/discordfolds into the discord capability: 160 lines whose only consumer wascap-discord.Numbers
Baseline measured on unmodified
origin/mainin this worktree, not taken from the issue.The one test delta: three tests covering the deleted
npm:/git:/absolute-path source branches went away, one new test asserts every implemented capability is blessed through the exports map, and the two error-path cases were rewritten as a parameterised pair. −3 +2 = −1.Net −270 lines, and that is with a new 350-line verification script. The refactor itself deletes considerably more than it adds, as #78 predicted.
Gates, all clean:
bun run lint,bun run typecheck,bun run build,bun test(303/0),node scripts/verify-pack.mjs(exit 0).scripts/check-workspace-deps.mjsno longer exists — its whole subject does not.Lockfile
Structural only. The set of resolved third-party
name@versionentries is byte-identical before and after (174 entries, zero added, zero removed) — verified by diffing the extracted version set, not by eyeballing. NominimumReleaseAgere-resolution, no exposure to #67.Deliberately not fixed
Dependency Auditstill fails onbrace-expansion— a patch inside the repo's own 7-day age window until ~2026-07-30. Pre-existing onmain, not this PR's, and no excludes entry added.main/types/ a.export were dropped. The published package is a binary plus its capability subpaths; the shell was never meant to be imported by anyone, and nothing has been published that could depend on it. Restoring a library surface later is additive.bun run builddoes not cleandist. Pre-existingtscbehaviour. It cannot mask a removed capability — the catalog statically imports each manifest, so removing one fails the build.[Unreleased]convention in this repo and no release is being cut here; the historical 0.2.0 entry that lists the old package names is a record of what shipped then.test/cli.test.tshas a pre-existingnoExplicitAnywarning. Untouched — unrelated to packaging, and lint is clean at the same 1 warning / 1 info asmain.Note for whoever cuts the first release
The one-time npm bootstrap in
release-publish.ymlis now a single publish and a single trusted-publisher registration instead of six of each, in dependency order. That header is rewritten.🤖 Generated with Claude Code