diff --git a/.changeset/ba4bc996.md b/.changeset/ba4bc996.md index c0459dfe..c2750c30 100644 --- a/.changeset/ba4bc996.md +++ b/.changeset/ba4bc996.md @@ -8,9 +8,10 @@ Add Pi (pi.dev) as a sixth CLI run mode (#206). - **New resolver** `src/utils/pi-cli-resolver.ts`. Unlike the sibling resolvers it sanity-probes `pi --version` and requires semver-shaped output, because `pi` is a short generic name that a stray binary on `$PATH` can shadow; the rejected path is logged. `GET /api/pi/status` returns `{ available, path, version }` so a misresolution is diagnosable. - **`PiConfig`** maps to `--model` (accepts `provider/id` and a `:thinking` suffix), `--provider`, `--thinking`, `--session`/`-c`, and the tri-state `--approve` / `--no-approve`. Every value is regex-allowlisted and dropped on failure. `--api-key` is deliberately never wired: it would put a provider secret on the spawn command line. -- **No bypass flag.** Pi has no permission prompts and no sandbox, so there is no `--dangerously-skip-permissions` analog. Its privilege-shaped knob is `approveProjectTrust`, which makes pi load and execute repo-local `.pi/extensions` TypeScript and install missing project packages. `clampExternalCliBypassForOwner()` therefore puts pi in the **materialize** branch: a non-granted multi-user owner gets `--no-approve` even when no config was sent, because pi's own default is an interactive prompt the session user could answer themselves. That helper had no test coverage at all; it now does, for all four CLIs. +- **No bypass flag.** Pi has no permission prompts and no sandbox, so there is no `--dangerously-skip-permissions` analog. Its privilege-shaped knob is `approveProjectTrust`, which makes pi load and execute repo-local `.pi/extensions` TypeScript and install missing project packages. `clampExternalCliBypassForOwner()` therefore puts pi in the **materialize** branch: a non-granted multi-user owner gets `--no-approve` even when no config was sent, because pi's own default is an interactive prompt the session user could answer themselves. The same materialization applies to cron-fired jobs (`clampCronExternalCliConfigs`), which carry no per-CLI config and would otherwise launch on pi's own default. Both helpers had no test coverage at all; they now do, for every CLI. - **Env allowlist gains only the `PI_*` prefix.** Pi's ~34 provider key vars share no prefix and `ALLOWED_ENV_PREFIXES` is one global list with no mode context, so admitting them would widen the allowlist for every mode at once. Users authenticate via pi's `/login` or the server process's own environment. - **Pi stays out of `isAltScreenStripMode()`.** Its default TUI renders into the main screen with terminal-owned scrollback, and since 0.84.0 the user can flip to a fullscreen TUI at runtime via `/settings` — verified to switch the pane into the alt screen, which the strip would have corrupted. - **Docker**: pi installs in its own `--ignore-scripts` step so that flag cannot affect the other four CLIs, and its credentials are seeded per-file (`auth.json`, `settings.json`, `trust.json`, `models.json`, `models-store.json`) rather than whole-dir, since `~/.pi/agent` also holds sessions, extensions and installed package trees. - **Local echo**: pi lands on the buffer overlay. Verified that codex's per-keystroke starvation does not reproduce — pi's slash picker re-filters on the whole composer content, so a one-shot flush behaves identically to per-keystroke typing. +- **Mode-list parity**: pi is excluded from the Ralph tracker auto-enable on `POST /api/sessions/:id/interactive` (like every other external CLI, whose output the tracker never parses), carries a `REMOTE_CLI_BIN` entry so a remote-SSH pi session reports its CLI version, and gets its own badge in the desktop home rail instead of rendering like Claude. - Installer detection, docs (`docs/pi-integration.md`), READMEs, and the architecture invariants are updated. Tests: `test/pi-mode.test.ts` and `test/routes/external-cli-bypass-clamp.test.ts`, plus extensions to the run-mode, mobile-overview, render-index-html, system-routes and local-echo suites. diff --git a/src/cron/cron-service.ts b/src/cron/cron-service.ts index 63c83064..93370d44 100644 --- a/src/cron/cron-service.ts +++ b/src/cron/cron-service.ts @@ -27,7 +27,7 @@ import { validateSessionFilePath } from '../web/route-helpers.js'; import { computeNextRunAt, dueKeyFor } from './cron-time.js'; import type { SessionPort, EventPort, ConfigPort, InfraPort } from '../web/ports/index.js'; import type { CronJob, CronJobRun, CronJobRunStatus, TriggerType } from '../types/cron.js'; -import type { GeminiConfig } from '../types/session.js'; +import type { GeminiConfig, PiConfig, SessionMode } from '../types/session.js'; import type { CronJobInput } from './cron-input.js'; /** The subset of the route context the cron depends on. */ @@ -35,6 +35,32 @@ export type CronDeps = SessionPort & EventPort & ConfigPort & InfraPort; const delay = (ms: number): Promise => new Promise((r) => setTimeout(r, ms)); +/** + * Section 6.3 clamp for a cron-launched external CLI, mirroring + * `clampExternalCliBypassForOwner()` in session-routes.ts. + * + * A cron job carries NO per-CLI config, so what a non-granted owner actually gets is + * each CLI's SPAWN DEFAULT, and for two of them that default is itself unsafe: + * - gemini: `buildGeminiCommand(undefined)` emits `--approval-mode yolo` (classifier-free), + * so `auto_edit` is materialized. + * - pi: pi's own `defaultProjectTrust` is an interactive prompt the session user can simply + * answer "yes" to, which then loads and EXECUTES repo-local `.pi/extensions` TypeScript, + * so `approveProjectTrust: false` (`--no-approve`) is materialized. Omitting `--approve` + * is NOT a clamp. + * Codex and antigravity need nothing here: their absent config already spawns safe. + * Granted/admin/single-user get undefined for both, i.e. upstream defaults untouched. + */ +export function clampCronExternalCliConfigs( + mode: SessionMode, + ownerGranted: boolean +): { geminiConfig: GeminiConfig | undefined; piConfig: PiConfig | undefined } { + if (ownerGranted) return { geminiConfig: undefined, piConfig: undefined }; + return { + geminiConfig: mode === 'gemini' ? { approvalMode: 'auto_edit' } : undefined, + piConfig: mode === 'pi' ? { approveProjectTrust: false } : undefined, + }; +} + /** Hard ceiling on a prompt-file read (defends against unbounded-read DoS). */ const MAX_PROMPT_FILE_BYTES = 1024 * 1024; @@ -371,13 +397,10 @@ export class CronService { const claudeModeConfig = await this.deps.getClaudeModeConfig(); const effectiveClaudeMode = await resolveClaudeModeForUsername(claudeModeConfig.claudeMode, job.owner); const model = mode !== 'shell' ? modelConfig?.defaultModel || undefined : undefined; - // Section 6.3: cron carries no per-CLI config, so buildGeminiCommand(undefined) - // would default a non-granted owner to `--approval-mode yolo` (classifier-free) — - // materialize auto_edit for a non-granted gemini owner, mirroring the route clamp - // (#15). Granted/admin/single-user leave it undefined → yolo parity. Codex's absent - // config already defaults to the safe sandbox, so no clamp is needed there. - const geminiConfig: GeminiConfig | undefined = - mode === 'gemini' && !ownerGranted ? { approvalMode: 'auto_edit' } : undefined; + // Section 6.3: materialize the safe default for a non-granted owner (see + // clampCronExternalCliConfigs — cron sends no per-CLI config, so the CLI's own + // spawn default is what would otherwise apply). + const { geminiConfig, piConfig } = clampCronExternalCliConfigs(mode, ownerGranted); session = new Session({ workingDir: job.workingDir, mode, @@ -389,6 +412,7 @@ export class CronService { claudeMode: effectiveClaudeMode, allowedTools: claudeModeConfig.allowedTools, geminiConfig, + piConfig, owner: job.owner, }); this.deps.addSession(session); diff --git a/src/remote-hosts.ts b/src/remote-hosts.ts index 6a01870d..681962ee 100644 --- a/src/remote-hosts.ts +++ b/src/remote-hosts.ts @@ -268,6 +268,7 @@ const REMOTE_CLI_BIN: Partial> = { codex: 'codex', gemini: 'gemini', antigravity: 'agy', + pi: 'pi', }; /** diff --git a/src/web/public/home-sessions.js b/src/web/public/home-sessions.js index a52d7a2a..a08dce23 100644 --- a/src/web/public/home-sessions.js +++ b/src/web/public/home-sessions.js @@ -69,6 +69,7 @@ const HOME_SESSIONS_MODE_BADGE = { codex: 'cx', gemini: 'gm', antigravity: 'ag', + pi: 'pi', }; Object.assign(CodemanApp.prototype, { diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 59c10686..d0b55fb8 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -1114,12 +1114,16 @@ export function registerSessionRoutes( try { // Auto-detect completion phrase from CLAUDE.md BEFORE starting (only if globally enabled and not explicitly disabled by user) - // Ralph tracker is not supported for opencode / codex / gemini / antigravity sessions + // Ralph tracker is not supported for opencode / codex / gemini / antigravity / pi sessions. + // Keep this list in step with isExternalCliMode(): _processExpensiveParsers() returns early + // for those modes, so a tracker enabled here would never be fed, and the session would + // still report ralphEnabled + Ralph UI state that no other external CLI shows. if ( session.mode !== 'opencode' && session.mode !== 'codex' && session.mode !== 'gemini' && session.mode !== 'antigravity' && + session.mode !== 'pi' && ctx.store.getConfig().ralphEnabled && !session.ralphTracker.autoEnableDisabled ) { diff --git a/test/cron-service.test.ts b/test/cron-service.test.ts index f8db095e..68ee369b 100644 --- a/test/cron-service.test.ts +++ b/test/cron-service.test.ts @@ -16,7 +16,7 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { existsSync, mkdtempSync, mkdirSync, writeFileSync, symlinkSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { CronService, type CronDeps } from '../src/cron/cron-service.js'; +import { CronService, clampCronExternalCliConfigs, type CronDeps } from '../src/cron/cron-service.js'; import { CronJobSchema } from '../src/web/schemas.js'; import { MAX_CRON_JOBS } from '../src/config/map-limits.js'; import type { CronJob, CronJobRun } from '../src/types/cron.js'; @@ -632,3 +632,36 @@ describe('CronService', () => { }); }); }); + +/** + * The §6.3 clamp cron applies at FIRE time. Cron sends no per-CLI config, so a + * missing clamp here is not "the default applies" but "the CLI's own unsafe default + * applies", which is the whole reason gemini and pi are materialized rather than + * left absent like codex/antigravity. + */ +describe('clampCronExternalCliConfigs', () => { + it('leaves everything undefined for a granted owner (upstream defaults)', () => { + expect(clampCronExternalCliConfigs('gemini', true)).toEqual({ geminiConfig: undefined, piConfig: undefined }); + expect(clampCronExternalCliConfigs('pi', true)).toEqual({ geminiConfig: undefined, piConfig: undefined }); + }); + + it('materializes gemini auto_edit for a non-granted owner (its default is yolo)', () => { + expect(clampCronExternalCliConfigs('gemini', false)).toEqual({ + geminiConfig: { approvalMode: 'auto_edit' }, + piConfig: undefined, + }); + }); + + it('materializes pi --no-approve for a non-granted owner (its default is an answerable prompt)', () => { + expect(clampCronExternalCliConfigs('pi', false)).toEqual({ + geminiConfig: undefined, + piConfig: { approveProjectTrust: false }, + }); + }); + + it('clamps nothing for modes whose absent config already spawns safe', () => { + for (const mode of ['claude', 'shell', 'opencode', 'codex', 'antigravity'] as const) { + expect(clampCronExternalCliConfigs(mode, false)).toEqual({ geminiConfig: undefined, piConfig: undefined }); + } + }); +}); diff --git a/test/home-sessions.test.ts b/test/home-sessions.test.ts index abae5884..719b3b38 100644 --- a/test/home-sessions.test.ts +++ b/test/home-sessions.test.ts @@ -143,6 +143,26 @@ describe('home sessions column: model', () => { }); expect(plain.buildHomeSessionRows()[0].modeBadge).toBe(''); }); + + it('badges every non-claude backend, so a new run mode cannot read as claude here', () => { + // The badge map is a per-mode lookup with a '' fallback, so a mode missing from it + // is indistinguishable from claude in this rail while the tab strip badges it fine. + for (const [mode, badge] of [ + ['shell', 'sh'], + ['opencode', 'oc'], + ['codex', 'cx'], + ['gemini', 'gm'], + ['antigravity', 'ag'], + ['pi', 'pi'], + ] as const) { + const app = loadHomeSessionsApp({ + sessions: sessionMap([{ id: 'a', mode }]), + sessionOrder: ['a'], + cases: CASES, + }); + expect(app.buildHomeSessionRows()[0].modeBadge).toBe(badge); + } + }); }); describe('home sessions column: gate', () => { diff --git a/test/pi-mode.test.ts b/test/pi-mode.test.ts index 1c5363ad..c1d63938 100644 --- a/test/pi-mode.test.ts +++ b/test/pi-mode.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest'; import { CreateSessionSchema, QuickStartSchema } from '../src/web/schemas.js'; import { buildSpawnCommand } from '../src/tmux-manager.js'; import { defaultDockerCommandForMode } from '../src/docker-hosts.js'; -import { defaultRemoteCommandForMode } from '../src/remote-hosts.js'; +import { defaultRemoteCommandForMode, buildRemoteCliVersionProbeCommand } from '../src/remote-hosts.js'; import { isExternalCliMode, isAltScreenStripMode } from '../src/session.js'; describe('Pi mode schemas', () => { @@ -188,4 +188,13 @@ describe('Pi mode gates', () => { // same fix as the other remote agent CLIs (see defaultRemoteCommandForMode). expect(defaultRemoteCommandForMode('pi')).toBe('exec "${SHELL:-/bin/sh}" -i -l -c \'pi\''); }); + + it('probes the CLI version on a remote host (REMOTE_CLI_BIN carries pi)', () => { + // Without the REMOTE_CLI_BIN entry this returns null and Session.cliVersion stays + // blank for every remote pi session, which is invisible until someone asks why the + // version column is empty on that host only. + const cmd = buildRemoteCliVersionProbeCommand({ username: 'dev', host: 'box.example', port: 22 }, 'pi'); + expect(cmd).not.toBeNull(); + expect(cmd).toContain('pi --version'); + }); });