fix Pi runtime startup compatibility

[Helmi]

Hey Jaymin,

I ran into this while using Overstory with the Pi runtime on a real project and put together a small fix.

After updating, Pi-based startup was failing for me for two reasons. Pi no longer seems to show the old pi v... header consistently, so readiness detection could time out even though it was already up. On top of that, the bash wrapping added for fish shell compatibility was breaking Pi startup in my setup.

This PR relaxes Pi readiness detection and adds a small config option for bash wrapping so fish users keep the existing behavior without forcing it for everyone else.

I also added test coverage for the Pi readiness case and the wrapper behavior.

With these changes, Pi-based coordinator startup worked again locally on my side.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7349ebb645

ℹ️ 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]

Handle non-fish shells in auto wrapper detection

createSession() now defaults to bashWrapper: "auto", but auto mode only enables wrapping when $SHELL contains "/fish"; every other shell path is treated as compatible. Because the startup command always prepends POSIX-style unset/export, users running tmux under other non-POSIX shells (or fish environments where $SHELL is not a full /.../fish path) will execute the unwrapped script and the new session can fail immediately, which is a regression from the previous always-wrapped behavior.

Useful? React with 👍 / 👎.

[lucabarak]

Verified on VM against v0.9.1: 146/146 Pi + tmux tests pass, biome + tsc clean. Code review of all 3 commits looks solid.

Pi detectReady fix — correct call. The status bar regex (\d+\.\d+%\/\d+k) is distinctive enough to be the sole readiness signal now that Pi >=0.55 dropped the header.

bashWrapper config — clean solution to the fish-vs-Pi quoting conflict. Auto-detecting $SHELL for fish and only wrapping when needed is the right trade-off. The Codex bot flagged that auto mode only catches fish (not csh/tcsh/nushell) — that's fair but practically low-risk, and "always" is available as a fallback for anyone hitting it.

One minor style note (not blocking): all 4 callers pass undefined for maxRetries to reach the bashWrapper parameter. If this pattern grows, an options object might be worth considering in a follow-up.

Heads up: this branch is based on v0.8.7 — it'll need a rebase onto current main (v0.9.1) before merge. The conflicting files (coordinator.ts, sling.ts) have changes in different functions so the rebase should be straightforward.

LGTM — approve pending rebase.

[jayminwest]

Review

Good fix for two real Pi runtime issues (readiness detection regression in Pi >=0.55 and bash wrapper breaking Pi's subshell quoting). The bashWrapper config approach is well-designed. Main blocker is the rebase.

Must fix

1. Rebase required onto main (v0.9.1) — The branch is based on v0.8.7. coordinator.ts on main now passes additional arguments (coordinatorName, profileFlag) to createSession. The diff will not apply cleanly. Should be a straightforward rebase since changes are in different functions.

Should fix

2. Missing config validation for bashWrapper — Other runtime config fields are validated in validateConfig(), but bashWrapper is not. A user setting bashWrapper: "yes" would silently fall through to the "auto" default. Add a quick validation block like other runtime fields:

   if (config.runtime?.bashWrapper !== undefined) {
     const valid = ["auto", "always", "never"];
     if (!valid.includes(config.runtime.bashWrapper)) {
       // warn and default to "auto"
     }
   }

3. No test for "always" mode — Tests cover "auto" (both shells) and "never", but not "always" forcing bash wrapping on a POSIX shell. Trivial addition alongside the existing "never" test.

Non-blocking suggestions

• Positional parameter sprawl in createSession (all 4 callers pass undefined for maxRetries to reach bashWrapper). An options object would be cleaner — could be a follow-up.
• The requiresBeaconVerification() JSDoc in pi.ts still mentions the header being present — should reflect Pi >=0.55 dropping it.

[RogerNavelsaker]

This looks superseded by #129 now. The newer branch carries the current Pi startup/readiness contract work on top of current main instead of the earlier compatibility-only fix path.