feat: roundhouse bundle — skills sync, CLI tools, extensions#4
Conversation
- Skills NOT shipped in npm package (too large, updates independently) - roundhouse setup will git clone inceptionstack/loki-skills → ~/.pi/agent/skills/ - roundhouse update will re-sync skills alongside npm update - Extensions (web-search.ts) shipped in pi/extensions/ (small, stable) - package.json pi.extensions points to ./pi/extensions for pi auto-discovery - Design doc updated with full implementation plan
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b93f25dd2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,99 @@ | |||
| import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; | |||
| import { Type } from "typebox"; | |||
There was a problem hiding this comment.
This extension imports typebox at runtime, but package.json does not declare it in this package’s dependencies. Because distributed pi packages are installed with production deps and Node resolves imports from the package’s own dependency tree, installs that do not hoist typebox to the top level will fail with Cannot find module 'typebox', which prevents the web_search tool from loading.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bcdc14d08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| execFileSync("playwright-cli", ["install"], { | ||
| encoding: "utf8", stdio: "pipe", timeout: 300_000, | ||
| }); |
There was a problem hiding this comment.
This step treats playwright-cli install as the browser download command, but Playwright’s Agent CLI docs define explicit browser provisioning as playwright-cli install-browser and use install --skills for skill files; as written, first-time setups can complete without actually installing Chromium, leaving browser automation unavailable until users manually install a browser (the same subcommand pattern is also used in /update provisioning).
Useful? React with 👍 / 👎.
| try { | ||
| const checks = [ | ||
| { cmd: "mcporter", install: "npm install -g mcporter" }, | ||
| { cmd: "playwright-cli", install: "npm install -g @playwright/cli && playwright-cli install" }, | ||
| { cmd: "uvx", install: "curl -fsSL https://astral.sh/uv/install.sh | sh" }, |
There was a problem hiding this comment.
The /update migration path handles pre-bundle tool provisioning but never updates ~/.pi/agent/settings.json to include npm:@inceptionstack/roundhouse, which is now required for Pi to load this package’s bundled extensions; as a result, users upgrading from pre-bundle installs can restart successfully yet still miss web-search until they rerun setup or edit settings manually.
Useful? React with 👍 / 👎.
79f5d52 to
6ce7afe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ce7afe9d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| execFileSync("rm", ["-rf", dest], { stdio: "pipe", timeout: 10_000 }); | ||
| execFileSync("cp", ["-r", src, dest], { stdio: "pipe", timeout: 30_000 }); |
There was a problem hiding this comment.
The sync loop deletes each existing skill directory before attempting to copy the new one, and failures are only logged. In cases like transient I/O errors, permission issues, or disk pressure, this turns a recoverable sync failure into data loss (~/.pi/agent/skills/<name> disappears), which contradicts the additive/non-fatal behavior described for bundle provisioning and can remove working skills after an update.
Useful? React with 👍 / 👎.
b54f0df to
229a90b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 229a90b15a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| describe("syncSkillsFromRepo", () => { | ||
| it("returns a positive count when git is available", () => { | ||
| const log = createMockLog(); | ||
| const count = syncSkillsFromRepo({ log }); |
There was a problem hiding this comment.
This test calls syncSkillsFromRepo() directly, which executes real git clone, rm -rf, and cp operations via src/bundle.ts instead of mocking process execution. As a result, vitest now depends on external network/tools and machine-specific state, and can modify ~/.pi/agent/skills on the developer/CI host; in offline CI or clean environments this will fail nondeterministically, and on developer machines it can unexpectedly rewrite local skill directories.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd2325419
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run("npm install -g @inceptionstack/roundhouse@latest"); | ||
| // Provision bundle (skills sync + CLI tools + config) | ||
| try { | ||
| const { provisionBundle } = await import("../bundle.js"); | ||
| provisionBundle(); |
There was a problem hiding this comment.
cmdUpdate() installs the new package and provisions the bundle, but it never migrates ~/.pi/agent/settings.json to include npm:@inceptionstack/roundhouse. That migration is required for Pi to load package-declared resources (pi.extensions) and is already handled in performUpdate(), so users who upgrade via roundhouse update from pre-bundle installs can restart successfully but still not load the bundled web_search extension until they edit settings or rerun setup.
Useful? React with 👍 / 👎.
- stepInstallBundle(): syncs skills from loki-skills, installs mcporter, playwright-cli (+Chromium), uv/uvx, copies mcporter config - Called in all 3 setup flows (interactive, headless, legacy) - /update command also syncs skills after npm update - All operations are non-fatal (warn + continue on failure) - Skills sync is additive (never deletes custom skills)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cb0c0a0b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!latestVersion || latestVersion === currentVersion) { | ||
| return { action: "already-latest", currentVersion }; | ||
| } |
There was a problem hiding this comment.
Don't return immediately when latestVersion === currentVersion; this skips both provisionBundle() and the settings migration, so /update cannot refresh skills/config unless a new npm version exists. That breaks the bundle model where skills evolve independently of package releases, and it also leaves already-current installs without the npm:@inceptionstack/roundhouse package entry un-migrated.
Useful? React with 👍 / 👎.
…31) (#133) * refactor(session-repair): beautify per code review (7 findings) Addresses all 7 findings from the post-v0.5.30 maintainability review. Pure refactor \u2014 zero behavior change. 536 tests passing (+2 new for the divergent-change fix). #1 + #7 \u2014 DRY: extract matchesErrorPatterns + fix divergent change isContextOverflowError and isToolPairingError shared ~80% of structure (top-level .message, cause-chain walk, stringify-gate, circular try/catch) but with subtle drift: only one walked the cause chain. Extracted matchesErrorPatterns(err, patterns, { stringifyGate }) shared helper. Both classifiers now ~7 lines and BOTH walk cause chains. Added 2 tests proving isToolPairingError now classifies wrapped/Bedrock-nested tool-pairing errors (regression guard for the divergent-change fix). Also extracted looksLikeValidationError() shared gate so the two classifiers don't repeat the 4xx/ValidationException check. #2 \u2014 long method: extracted buildTrimmedEntries(entries, cutIdx) from softResetSessionFile. The orchestration is now linear: validate \u2192 cut \u2192 build \u2192 repair \u2192 write \u2192 report. Each step at the same abstraction level. #3 \u2014 long catch with nested try: extracted attemptSoftResetRecovery() in lifecycle.ts. flushMemoryThenCompact catch block went from ~60 lines with nested try/catch to ~25 lines, linear: classify \u2192 recover \u2192 log \u2192 persist. #4 \u2014 magic number: MAX_CAUSE_CHAIN_DEPTH = 5 named constant. #5 \u2014 file >400 lines: split src/agents/shared/session-repair.ts (574 lines, two domains) into: - session-repair.ts (81 lines) \u2014 public surface, tool-pair repair, re-exports for backward compat - session-soft-reset.ts (120 lines) \u2014 softResetSessionFile + types - error-classifiers.ts (75 lines) \u2014 matchesErrorPatterns + isContextOverflowError + isToolPairingError + MAX_CAUSE_CHAIN_DEPTH - session-repair-internal.ts (239 lines) \u2014 shared filesystem + in-memory repair primitives All public exports preserved via re-exports (Feathers seam pattern). Existing callers compile unchanged. #6 \u2014 naming: introduced RepairResult { entries, report } named type for the previously-anonymous repairEntriesInMemory return shape. Better IDE hover/autocomplete; no inline destructuring noise at call sites. Verification: npm test \u2014 536 passing (was 534, +2 for cause-chain regression tests) wc -l src/agents/shared/session-repair.ts \u2014 81 (was 574) All public API surfaces preserved. * chore: bump to v0.5.31 + CHANGELOG --------- Co-authored-by: Roy Osherove <575051+royosherove@users.noreply.github.com>
* design: soft-reset pre-turn gap recovery (v0.5.38)
Codex (gpt-5.1) recommended Alternative D: shared reactive overflow
recovery helper, deferred-retry UX, fallback to pendingCompact=emergency.
Full design + open-question resolutions in
docs/design/v0.5.38-soft-reset-pre-turn-gap.md.
Implements next: classify-in-catch + agent.softReset() + state write +
fallback when softReset is unavailable/fails.
* fix(gateway): classify and recover from context overflow in agent.prompt catch (v0.5.38)
Closes the soft-reset pre-turn gap: when an idle session has already
grown past the provider context limit (e.g. via background cron/boot/
sub-agent activity that didn't trip soft/hard pressure), the next user
turn's agent.prompt() / agent.promptStream() throws 'prompt is too long'
before any pre-turn pressure handler can fire. v0.5.29-v0.5.32 only
recovered when *compact itself* overflowed; this catch path was
unprotected, so the gateway just posted the raw provider error and the
loop continued.
Changes:
- Extract recoverFromContextOverflow() into src/agents/shared/overflow-
recovery.ts (file-private attemptSoftResetRecovery in lifecycle.ts is
removed; lifecycle now imports the shared helper).
- Add src/gateway/overflow.ts with recoverFromAgentTurnOverflow():
classify -> softReset -> persist memory state (forceInjectReason or
pendingCompact='emergency' fallback when softReset is unavailable
but compact isn't) -> append telemetry (level='gateway-overflow')
-> post deferred-retry hint.
- Wire it into Gateway.handleAgentTurn's catch. Plumb turnSource
('user'|'boot'|'subagent') through so background turns get distinct
copy ('Original work was not retried.' instead of 'please resend').
- handleStreaming() now returns hadVisibleText so the catch can
distinguish 'please resend' from 'response was interrupted' on
streaming overflows after partial text.
- export appendCompactLog from lifecycle so the gateway-side recovery
uses the same compact-timing.jsonl schema.
UX: deferred retry only. No transparent same-turn replay because a
streamed turn that already emitted text or executed tools would
duplicate output / side effects on retry.
Design: docs/design/v0.5.38-soft-reset-pre-turn-gap.md (codex-cli
Alternative D).
* test: gateway and helper-level overflow recovery tests (v0.5.38)
19 new tests across two files (584 total, +19 net):
test/overflow-recovery.test.ts (8 tests) — direct unit tests on the
extracted recoverFromContextOverflow helper:
- non-overflow returns not-overflow without calling softReset
- adapter without softReset returns unsupported
- softReset success returns recovered with report + ✅ progress
- softReset reset:false returns noop with reason + ⚠️ progress
- softReset throws returns failed with message + ❌ progress
- softReset throws non-Error (string) — String() coerced, no TypeError
(regression for v0.5.32 fix #4)
- overflow buried in .cause chain still classifies (v0.5.30 regression)
- onProgress optional — undefined callback handled
test/gateway-overflow-recovery.test.ts (11 tests) — gateway-level
integration tests on recoverFromAgentTurnOverflow with fake adapter +
fake thread, covering the brief's required surface:
- overflow during non-streaming prompt + softReset success → recovered
hint + state.forceInjectReason='after-soft-reset' + cleared pendingCompact
- overflow + softReset undefined + no compact → sanitized error, no arming
- overflow + softReset throws + has compact → pendingCompact='emergency'
armed + 'Recovery armed (failed: …)' hint
- overflow + softReset reset:false + has compact → pendingCompact armed
+ 'Recovery armed (noop: <reason>)' hint
- overflow + softReset fails + no compact → sanitized error, no arming
(don't arm a flag the next turn can't honor)
- non-overflow error → sanitized error, no recovery, no state mutation
- streaming overflow before any text → '✅ Recovered. Please resend…'
- streaming overflow after partial text → '♻️ Response was interrupted'
- background turns (boot/subagent/cron) → distinct copy, no resend ask
- thread.post throws → recovery still updates state, doesn't propagate
- overflow in cause chain → still triggers recovery
All tests pass: 584 / 584.
* fix(streaming): escalate model_error overflow to gateway recovery (F1, v0.5.38)
pi-ai's streaming surfaces provider errors as model_error EVENTS, not
thrown exceptions. The v0.5.38 catch in Gateway.handleAgentTurn only
saw synchronous throws from agent.prompt()/promptStream(), so streamed
'prompt is too long' bypassed recoverFromAgentTurnOverflow entirely:
the model_error case posted the raw error inline, the for-await
continued to turn_end, and the function returned normally. On Telegram
(streaming-default), the v0.5.38 fix was effectively a no-op.
Per codex-cli design (see ~/.roundhouse/workspace/softreset-f1-codex-design.md):
classify the model_error message in streaming.ts via isContextOverflowError.
Non-overflow keeps the existing inline post + continue-loop. Overflow
flushes, suppresses the inline raw post, and throws a typed
StreamModelOverflowError carrying the provider message. The gateway
catch's existing recoverFromAgentTurnOverflow handles it identically to
synchronous-throw overflow — single recovery surface, no duplicate
posts, no flag plumbed through StreamResult.
7 new streaming-overflow tests:
- model_error overflow throws + suppresses raw post
- model_error non-overflow keeps inline post + continues loop (regression)
- text_delta then overflow throws with overflow message
- non-overflow inline post format unchanged
- end-to-end: overflow → recovery → 'please resend' wording
- end-to-end: partial text → recovery → 'interrupted' wording
- end-to-end: non-overflow → no recovery, raw post stands alone
591 tests passing (584 + 7).
* fix(gateway): drop dead 'cron' TurnSource and clarify unsupported-overflow hint (F2+F3)
F2: TurnSource included 'cron' but cron jobs run via cron/runner.ts in
their own session and never invoke Gateway.handleAgentTurn. Verified
via grep on this.handleAgentTurn — only user-message-handler,
fireBootTurn, and handleSubagentCompletion reach it. Removed 'cron'
from the union. If a future patch adds cron→main injection, add it
back at that point (YAGNI).
F3: pickUserMessage's 'unsupported' branch (no softReset on adapter,
no armed pending) returned the raw sanitized provider error
('⚠️ Error: prompt is too long: N > M'). User saw a wall-of-numbers
with no indication that recovery was unavailable. Now returns:
'⚠️ Session full — adapter doesn't support automatic recovery.
Run /compact manually or restart session.'
Updated:
- src/gateway/overflow.ts — TurnSource type, file header comment,
pickUserMessage unsupported branch.
- test/gateway-overflow-recovery.test.ts — drop 'cron' from
background-turn iteration; rewrite the 'unsupported-no-compact' test
to assert the new clear-guidance copy and verify the raw provider
error does NOT leak through.
- docs/design/v0.5.38-soft-reset-pre-turn-gap.md — match TurnSource shape.
591 tests passing.
* refactor: extract telemetry, dedupe MAX_ERROR_PREVIEW (F4+F5+F6)
F4 — appendCompactLog cross-domain import.
src/gateway/overflow.ts imported appendCompactLog from src/memory/lifecycle,
coupling gateway → memory/lifecycle (flagged as follow-up in the v0.5.38
design doc). Extracted appendCompactLog + CompactLogEntry to a new
src/memory/telemetry.ts (~43 LOC). Both lifecycle.ts and gateway/overflow.ts
now import from telemetry. lifecycle.ts re-exports the symbol for
backwards compatibility (no external callers found via grep).
F5 — MAX_ERROR_PREVIEW duplication.
The constant existed in both src/gateway/gateway.ts and src/gateway/overflow.ts.
gateway.ts had only the declaration (its sole consumer moved to overflow.ts
with the v0.5.38 catch refactor) — deleted as dead code. overflow.ts keeps
its single definition.
F6 — slice(0, 100) magic number.
pickUserMessage's failed-outcome reason truncation used a bare 100. Added
named constant MAX_FAILURE_REASON_PREVIEW = 100 with a comment explaining
why it's shorter than MAX_ERROR_PREVIEW (UI message bracketed by other
copy, not a standalone error preview).
591 tests passing.
* docs(changelog): v0.5.38 streaming-path fix + F2-F6 polish
Reflects the post-review changes:
- F1: streaming model_error overflow path now routed through the same
recovery surface as synchronous-throw overflow (was the primary
Telegram-path bug; pi-ai surfaces provider errors as events, not
exceptions).
- F2: TurnSource cron removal.
- F3: clearer unsupported-overflow user message.
- F4-F6: telemetry extraction, MAX_ERROR_PREVIEW dedup, named constant
for failure-reason preview length.
- Test count corrected to 591 (+26 net).
---------
Co-authored-by: roundhouse-fix-bot <bot@local>
Co-authored-by: Roy Osherove <575051+royosherove@users.noreply.github.com>
Summary
Adds bundle installation to
roundhouse setupand/updatecommand. On setup, roundhouse now installs:Changes
src/cli/setup.ts: newstepInstallBundle()function called in all 3 setup flowssrc/gateway.ts:/updatecommand re-syncs skills after npm updatepi/extensions/web-search.ts: Tavily web search extensionpi/config/mcporter.json: Static MCP server config (5 servers)docs/bundle-design.md: Full design documentREADME.md: Documents bundle in new sectionDesign decisions
rm -rfbeforecp -rprevents directory nesting on re-runsfind -type dfilter ensures only skill directories are copiedRemaining (deferred)