feat: optional pi-hard-no & pi-branch-enforcer + shared pi-settings module (v0.5.40)

[royosherove]

Summary

Makes pi-hard-no (quality inspector) and pi-branch-enforcer opt-in for new setups, with explicit chat commands to toggle them on/off after install. Existing setups are unaffected.

What changed

1. Shared pi-settings module (src/pi-settings.ts) — centralises all ~/.pi/agent/settings.json I/O with atomic writes, per-process queue + on-disk lockfile (proper-lockfile), and typed MalformedPiSettingsError. All 4 existing writers migrated.

2. Opt-in default — Removed pi-hard-no and pi-branch-enforcer from coreExtensions in runtime.ts and bundle.ts. New installs no longer auto-enable them. The npm packages stay globally installed (via /update) so toggling on doesn't require a fresh install.

3. Toggle commands — /toggle-quality-inspector [on|off] and /toggle-branch-enforcer [on|off]:

   • Idempotent setter semantics (safe for delivery dups)
   • No arg → inline keyboard showing current state
   • Stage: pre-turn (works during in-flight agent turn)
   • Distinct action IDs (ext_toggle_qi, ext_toggle_be) to avoid command-registry duplicate rejection
   • Button values are just "on"/"off" (≤3 bytes, under Telegram 64-byte callback limit)

4. Doctor check improvement — pi-settings check now distinguishes ENOENT (warn) from SyntaxError (fail + parse message + path).

5. Tests — 21 new tests (12 pi-settings + 7 extension-toggle + 2 bundle provisioning). All 612 tests green.

Codex review history (3 rounds, 11 issues addressed)

Full plan at ~/.roundhouse/workspace/roundhouse-optional-extensions/PLAN.md. Key issues resolved:

• High add .hardno/ignore to skip .pi/ from reviews #1: Blind toggle → idempotent setter with on/off args
• High feat: memory system optimizations #2: RMW race → locked updatePiSettings() with per-process queue + lockfile
• High feat: /update command — update roundhouse from Telegram #3: In-turn stage wrong for control-plane mutation → pre-turn
• High fix: mock child_process in bundle tests for CI #7: Duplicate action ID → distinct IDs per command
• High fix: split CI into test + tag-triggered publish #8: Telegram 64-byte payload limit → value is just "on"/"off"
• Medium feat: roundhouse bundle — skills sync, CLI tools, extensions #4: 5th ad-hoc writer → shared module
• Medium chore: bump version to 0.4.0 #5: Malformed settings → consistent throw contract
• Medium feat: provision core extensions + show in /status #9: Mixed throw/return contract → always throw
• Medium chore: bump to v0.4.3 #10: Doctor check undifferentiated → ENOENT vs SyntaxError
• Medium chore: sync package-lock.json #11: Transitive lockfile dep → direct dependency

Commits

• Part 0: Extract pi-settings module + migrate writers
• Part 1: Stop auto-enabling on new setups
• Part 2+3: Toggle commands + wiring
• Part 4: Tests
• Part 5: Docs + version bump

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af6bc75074

ℹ️ 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".

[chatgpt-codex-connector]

Catch malformed settings errors before posting /model result

applyModelSelection() now writes via updatePiSettings(), which throws MalformedPiSettingsError when ~/.pi/agent/settings.json is malformed (including invalid packages types). This function does not catch that error, so /model invocations can bubble an unhandled rejection through command dispatch and return no user-facing response. This is a regression from the previous behavior (which tolerated parse failures via readSettings() fallback) and can break model switching for users with a corrupted settings file until they manually repair it.

Useful? React with 👍 / 👎.

[royosherove]

Codex Review — Round 1 (post-implementation)

PR #148 verified against PLAN.md v3 (which Codex already approved). Found 2 High, 3 Medium, 1 Low.

🔴 High

1. provisionExtensions() was NOT migrated — src/provisioning/bundle.ts:228,277 still does raw readFileSync/writeFileSync + generic JSON.parse. Plan items 2, 4, 9 not honored: "all 4 writers migrated" claim is false; shared module is not the single seam; this path bypasses MalformedPiSettingsError.

2. Setup writer is not concurrency-safe — src/cli/setup/runtime.ts:17,22 pre-reads settings.json into existing outside the lock, then makes overwrite decisions from that stale snapshot inside updatePiSettings(). A concurrent /model write between those two steps can be clobbered. Defeats the RMW guarantee.

🟡 Medium

3. Setup malformed-JSON handling missing — src/cli/setup/runtime.ts:17: outer JSON.parse is swallowed, then updatePiSettings() throws MalformedPiSettingsError with no boundary catch. Setup aborts instead of warn+skip per plan.

4. patchPiSettings swallows everything — src/cli/update.ts:120 empty catch {} hides malformed/permission/lock errors equally. Per plan, only ENOENT should be silent; malformed should warn+skip and surface to /doctor.

5. Concurrency test only covers in-process queue — test/pi-settings.test.ts:96 fires 10 concurrent calls through the same module in one process. Exercises the JS promise queue but not proper-lockfile's cross-process behavior or any cross-writer race (/model vs toggle vs setup). Big coverage gap given how much of the plan rests on RMW safety.

🔵 Low

6. package-lock.json still says 0.5.38 while package.json + CHANGELOG say 0.5.40. npm install would have updated it; missed.

---

Verdict: needs fixes before merge. All 6 issues are real and within scope of this PR.